Skip to content

Commit 33f853a

Browse files
esdrubalesdrubal
committed
Fixes find_method_for_type bail out with 1st match
When find_method_for_type could not match the parameters and return type with the available methods it would return the first match by name. This was not a deterministic and secure behavior so now we strictly match the parameter types and return type. Fixes #5086
1 parent 2156bfb commit 33f853a

File tree

39 files changed

+311
-121
lines changed

39 files changed

+311
-121
lines changed

‎sway-core/src/semantic_analysis/ast_node/declaration/function.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,12 @@ impl ty::TyFunctionDecl {
216216
} = ty_fn_decl;
217217

218218
// Insert the previously type checked type parameters into the current namespace.
219+
// We insert all type parameter before the constraints because some constraints may depend on the parameters.
220+
for p in type_parameters.iter() {
221+
p.insert_into_namespace_self(handler, ctx.by_ref())?;
222+
}
219223
for p in type_parameters {
220-
p.insert_into_namespace(handler, ctx.by_ref())?;
224+
p.insert_into_namespace_constraints(handler, ctx.by_ref())?;
221225
}
222226

223227
// Insert the previously type checked function parameters into the current namespace.

‎sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ impl ty::TyExpression {
5858
arguments: Vec<ty::TyExpression>,
5959
span: Span,
6060
) -> Result<ty::TyExpression, ErrorEmitted> {
61+
let engines = ctx.engines;
62+
let ctx = ctx.with_type_annotation(engines.te().insert(
63+
engines,
64+
TypeInfo::Boolean,
65+
span.source_id(),
66+
));
6167
Self::core_ops(handler, ctx, OpVariant::Equals, arguments, span)
6268
}
6369

@@ -67,6 +73,12 @@ impl ty::TyExpression {
6773
arguments: Vec<ty::TyExpression>,
6874
span: Span,
6975
) -> Result<ty::TyExpression, ErrorEmitted> {
76+
let engines = ctx.engines;
77+
let ctx = ctx.with_type_annotation(engines.te().insert(
78+
engines,
79+
TypeInfo::Boolean,
80+
span.source_id(),
81+
));
7082
Self::core_ops(handler, ctx, OpVariant::NotEquals, arguments, span)
7183
}
7284

‎sway-core/src/semantic_analysis/namespace/trait_map.rs‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,7 @@ impl TraitMap {
10251025
type_id: TypeId,
10261026
) -> Vec<(ResolvedTraitImplItem, TraitKey)> {
10271027
let type_engine = engines.te();
1028-
let unify_check = UnifyCheck::non_dynamic_equality(engines);
1028+
let unify_check = UnifyCheck::non_dynamic_equality(engines).with_unify_ref_mut(false);
10291029

10301030
let mut items = vec![];
10311031
// small performance gain in bad case
@@ -1303,16 +1303,18 @@ impl TraitMap {
13031303
CompileError::MultipleApplicableItemsInScope {
13041304
item_name: symbol.as_str().to_string(),
13051305
item_kind: "item".to_string(),
1306-
type_name: engines.help_out(type_id).to_string(),
13071306
as_traits: candidates
13081307
.keys()
13091308
.map(|k| {
1310-
k.clone()
1311-
.split("::")
1312-
.collect::<Vec<_>>()
1313-
.last()
1314-
.unwrap()
1315-
.to_string()
1309+
(
1310+
k.clone()
1311+
.split("::")
1312+
.collect::<Vec<_>>()
1313+
.last()
1314+
.unwrap()
1315+
.to_string(),
1316+
engines.help_out(type_id).to_string(),
1317+
)
13161318
})
13171319
.collect::<Vec<_>>(),
13181320
span: symbol.span(),

‎sway-core/src/semantic_analysis/type_check_context.rs‎

Lines changed: 123 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{HashMap, VecDeque};
1+
use std::collections::{HashMap, HashSet, VecDeque};
22

33
use crate::{
44
build_config::ExperimentalFlags,
@@ -1256,17 +1256,40 @@ impl<'a> TypeCheckContext<'a> {
12561256
// While collecting unification we don't decay numeric and will ignore this error.
12571257
if self.collecting_unifications {
12581258
return Err(handler.emit_err(CompileError::MethodNotFound {
1259-
method_name: method_name.clone(),
1259+
method: method_name.clone().as_str().to_string(),
12601260
type_name: self.engines.help_out(type_id).to_string(),
1261+
matching_method_strings: vec![],
12611262
span: method_name.span(),
12621263
}));
12631264
}
12641265
type_engine.decay_numeric(handler, self.engines, type_id, &method_name.span())?;
12651266
}
12661267

1267-
let matching_item_decl_refs =
1268+
let mut matching_item_decl_refs =
12681269
self.find_items_for_type(handler, type_id, method_prefix, method_name)?;
12691270

1271+
// This code path tries to get items of specific implementation of the annotation type that is a superset of type id.
1272+
// This allows the following associated method to be found:
1273+
// let _: Option<MyStruct<u64>> = MyStruct::try_from(my_u64);
1274+
if !matches!(&*type_engine.get(annotation_type), TypeInfo::Unknown)
1275+
&& !type_id.is_concrete(self.engines, crate::TreatNumericAs::Concrete)
1276+
{
1277+
let inner_types =
1278+
annotation_type.extract_inner_types(self.engines, crate::IncludeSelf::Yes);
1279+
for inner_type_id in inner_types {
1280+
if coercion_check.check(inner_type_id, type_id) {
1281+
matching_item_decl_refs.extend(self.find_items_for_type(
1282+
handler,
1283+
inner_type_id,
1284+
method_prefix,
1285+
method_name,
1286+
)?);
1287+
}
1288+
}
1289+
}
1290+
1291+
let mut matching_method_strings = HashSet::<String>::new();
1292+
12701293
let matching_method_decl_refs = matching_item_decl_refs
12711294
.into_iter()
12721295
.flat_map(|item| match item {
@@ -1284,22 +1307,36 @@ impl<'a> TypeCheckContext<'a> {
12841307
let mut maybe_method_decl_refs: Vec<DeclRefFunction> = vec![];
12851308
for decl_ref in matching_method_decl_refs.clone().into_iter() {
12861309
let method = decl_engine.get_function(&decl_ref);
1287-
if method.parameters.len() == arguments_types.len()
1310+
// Contract call methods don't have self parameter.
1311+
let args_len_diff = if method.is_contract_call && arguments_types.len() > 0 {
1312+
1
1313+
} else {
1314+
0
1315+
};
1316+
if method.parameters.len() == arguments_types.len() - args_len_diff
12881317
&& method
12891318
.parameters
12901319
.iter()
1291-
.zip(arguments_types.iter())
1292-
.all(|(p, a)| coercion_check.check(p.type_argument.type_id, *a))
1320+
.zip(arguments_types.iter().skip(args_len_diff))
1321+
.all(|(p, a)| coercion_check.check(*a, p.type_argument.type_id))
12931322
&& (matches!(&*type_engine.get(annotation_type), TypeInfo::Unknown)
1294-
|| coercion_check.check(annotation_type, method.return_type.type_id))
1323+
|| matches!(
1324+
&*type_engine.get(method.return_type.type_id),
1325+
TypeInfo::Never
1326+
)
1327+
|| coercion_check
1328+
.with_ignore_generic_names(true)
1329+
.check(annotation_type, method.return_type.type_id))
12951330
{
12961331
maybe_method_decl_refs.push(decl_ref);
12971332
}
12981333
}
12991334

13001335
if !maybe_method_decl_refs.is_empty() {
1301-
let mut trait_methods =
1302-
HashMap::<(CallPath, Vec<WithEngines<TypeArgument>>), DeclRefFunction>::new();
1336+
let mut trait_methods = HashMap::<
1337+
(CallPath, Vec<WithEngines<TypeArgument>>, TypeId),
1338+
DeclRefFunction,
1339+
>::new();
13031340
let mut impl_self_method = None;
13041341
for method_ref in maybe_method_decl_refs.clone() {
13051342
let method = decl_engine.get_function(&method_ref);
@@ -1353,6 +1390,7 @@ impl<'a> TypeCheckContext<'a> {
13531390
.cloned()
13541391
.map(|a| self.engines.help_out(a))
13551392
.collect::<Vec<_>>(),
1393+
method.implementing_for_typeid.unwrap(),
13561394
),
13571395
method_ref.clone(),
13581396
);
@@ -1372,6 +1410,7 @@ impl<'a> TypeCheckContext<'a> {
13721410
.cloned()
13731411
.map(|a| self.engines.help_out(a))
13741412
.collect::<Vec<_>>(),
1413+
method.implementing_for_typeid.unwrap(),
13751414
),
13761415
method_ref.clone(),
13771416
);
@@ -1407,20 +1446,24 @@ impl<'a> TypeCheckContext<'a> {
14071446
.collect::<Vec<_>>()
14081447
.join(", ")
14091448
)
1410-
}
1449+
},
14111450
)
14121451
}
14131452
let mut trait_strings = trait_methods
14141453
.keys()
1415-
.map(|t| to_string(t.0.clone(), t.1.clone()))
1416-
.collect::<Vec<String>>();
1454+
.map(|t| {
1455+
(
1456+
to_string(t.0.clone(), t.1.clone()),
1457+
self.engines.help_out(t.2.clone()).to_string(),
1458+
)
1459+
})
1460+
.collect::<Vec<(String, String)>>();
14171461
// Sort so the output of the error is always the same.
14181462
trait_strings.sort();
14191463
return Err(handler.emit_err(
14201464
CompileError::MultipleApplicableItemsInScope {
14211465
item_name: method_name.as_str().to_string(),
14221466
item_kind: "function".to_string(),
1423-
type_name: self.engines.help_out(type_id).to_string(),
14241467
as_traits: trait_strings,
14251468
span: method_name.span(),
14261469
},
@@ -1435,7 +1478,48 @@ impl<'a> TypeCheckContext<'a> {
14351478
} else {
14361479
// When we can't match any method with parameter types we still return the first method found
14371480
// This was the behavior before introducing the parameter type matching
1438-
matching_method_decl_refs.first().cloned()
1481+
//matching_method_decl_refs.first().cloned();
1482+
1483+
for decl_ref in matching_method_decl_refs.clone().into_iter() {
1484+
let method = decl_engine.get_function(&decl_ref);
1485+
println!(
1486+
" {} {} {:?} // {:?} Return:{:?} // {:?}",
1487+
method.is_contract_call,
1488+
method.name.as_str(),
1489+
self.engines.help_out(
1490+
method
1491+
.parameters
1492+
.iter()
1493+
.map(|p| p.type_argument.type_id)
1494+
.collect::<Vec<_>>()
1495+
),
1496+
self.engines
1497+
.help_out(arguments_types.iter().collect::<Vec<_>>()),
1498+
self.engines.help_out(method.return_type.type_id),
1499+
self.engines.help_out(self.type_annotation())
1500+
);
1501+
1502+
matching_method_strings.insert(format!(
1503+
"{}({}) -> {}{}",
1504+
method.name.as_str(),
1505+
method
1506+
.parameters
1507+
.iter()
1508+
.map(|p| self.engines.help_out(p.type_argument.type_id).to_string())
1509+
.collect::<Vec<_>>()
1510+
.join(", "),
1511+
self.engines
1512+
.help_out(method.return_type.type_id)
1513+
.to_string(),
1514+
if let Some(implementing_for_type_id) = method.implementing_for_typeid {
1515+
format!(" in {}", self.engines.help_out(implementing_for_type_id))
1516+
} else {
1517+
"".to_string()
1518+
}
1519+
));
1520+
}
1521+
1522+
None
14391523
}
14401524
};
14411525

@@ -1478,9 +1562,33 @@ impl<'a> TypeCheckContext<'a> {
14781562
} else {
14791563
self.engines.help_out(type_id).to_string()
14801564
};
1565+
14811566
Err(handler.emit_err(CompileError::MethodNotFound {
1482-
method_name: method_name.clone(),
1567+
method: format!(
1568+
"{}({}){}",
1569+
method_name.clone(),
1570+
arguments_types
1571+
.iter()
1572+
.map(|a| self.engines.help_out(a).to_string())
1573+
.collect::<Vec<_>>()
1574+
.join(", "),
1575+
if matches!(
1576+
*self.engines.te().get(self.type_annotation),
1577+
TypeInfo::Unknown
1578+
) {
1579+
"".to_string()
1580+
} else {
1581+
format!(
1582+
" -> {}",
1583+
self.engines.help_out(self.type_annotation).to_string()
1584+
)
1585+
}
1586+
),
14831587
type_name,
1588+
matching_method_strings: matching_method_strings
1589+
.iter()
1590+
.cloned()
1591+
.collect::<Vec<_>>(),
14841592
span: method_name.span(),
14851593
}))
14861594
}

‎sway-core/src/type_system/ast_elements/type_parameter.rs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -378,19 +378,7 @@ impl TypeParameter {
378378
Ok(())
379379
}
380380

381-
pub fn insert_into_namespace(
382-
&self,
383-
handler: &Handler,
384-
mut ctx: TypeCheckContext,
385-
) -> Result<(), ErrorEmitted> {
386-
self.insert_into_namespace_constraints(handler, ctx.by_ref())?;
387-
388-
self.insert_into_namespace_self(handler, ctx.by_ref())?;
389-
390-
Ok(())
391-
}
392-
393-
fn insert_into_namespace_constraints(
381+
pub(crate) fn insert_into_namespace_constraints(
394382
&self,
395383
handler: &Handler,
396384
mut ctx: TypeCheckContext,

‎sway-core/src/type_system/id.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ use crate::{
99
decl_engine::DeclEngineGet,
1010
engine_threading::{DebugWithEngines, DisplayWithEngines, Engines, WithEngines},
1111
language::CallPath,
12-
semantic_analysis::type_check_context::EnforceTypeArguments,
13-
semantic_analysis::TypeCheckContext,
12+
semantic_analysis::{type_check_context::EnforceTypeArguments, TypeCheckContext},
1413
type_system::priv_prelude::*,
1514
types::{CollectTypesMetadata, CollectTypesMetadataContext, TypeMetadata},
1615
};

0 commit comments

Comments
 (0)