Skip to content

Commit ac95d2d

Browse files
Removes TraitMap::insert_for_type. (#6867)
## Description Some use cases require the compiler to call insert_implementation_for_type on every call of find_method_for_type. This would double the compile time of a simple script. To avoid this we removed the TraitMap::insert_for_type. The TraitMap now only stores the original implementations and uses a less restrictive unify_check to get those that match the concrete implementations. This significantly reduces the TraitMap's size and speeds up the lookup times. On the other hand, it also introduces a type substitution on every find_method_for_type. A future PR will address the performance of doing type substitutions. Fixes #5002. Fixes #6858 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [ ] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Joshua Batty <joshpbatty@gmail.com>
1 parent 475fd78 commit ac95d2d

File tree

26 files changed

+781
-652
lines changed

26 files changed

+781
-652
lines changed

sway-core/src/language/ty/declaration/function.rs

+91
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,78 @@ impl DisplayWithEngines for TyFunctionDecl {
133133
}
134134
}
135135

136+
impl DeclRefFunction {
137+
/// Makes method with a copy of type_id.
138+
/// This avoids altering the type_id already in the type map.
139+
/// Without this it is possible to retrieve a method from the type map unify its types and
140+
/// the second time it won't be possible to retrieve the same method.
141+
pub fn get_method_safe_to_unify(&self, engines: &Engines, type_id: TypeId) -> Self {
142+
let decl_engine = engines.de();
143+
144+
let mut method = (*decl_engine.get_function(self)).clone();
145+
146+
if let Some(method_implementing_for_typeid) = method.implementing_for_typeid {
147+
let mut type_id_type_subst_map = TypeSubstMap::new();
148+
if let Some(TyDecl::ImplSelfOrTrait(t)) = &method.implementing_type {
149+
let impl_self_or_trait = &*engines.de().get(&t.decl_id);
150+
let mut type_id_type_parameters = vec![];
151+
type_id.extract_type_parameters(
152+
engines,
153+
0,
154+
&mut type_id_type_parameters,
155+
impl_self_or_trait.implementing_for.type_id,
156+
);
157+
158+
for impl_type_parameter in impl_self_or_trait.impl_type_parameters.clone() {
159+
let matches = type_id_type_parameters
160+
.iter()
161+
.filter(|(_, orig_tp)| {
162+
engines.te().get(orig_tp.type_id).eq(
163+
&*engines.te().get(impl_type_parameter.type_id),
164+
&PartialEqWithEnginesContext::new(engines),
165+
)
166+
})
167+
.collect::<Vec<_>>();
168+
if !matches.is_empty() {
169+
// Adds type substitution for first match only as we can apply only one.
170+
type_id_type_subst_map
171+
.insert(impl_type_parameter.type_id, matches[0].0.type_id);
172+
} else if engines
173+
.te()
174+
.get(impl_self_or_trait.implementing_for.initial_type_id)
175+
.eq(
176+
&*engines.te().get(impl_type_parameter.initial_type_id),
177+
&PartialEqWithEnginesContext::new(engines),
178+
)
179+
{
180+
type_id_type_subst_map.insert(impl_type_parameter.type_id, type_id);
181+
}
182+
}
183+
}
184+
185+
let mut method_type_subst_map = TypeSubstMap::new();
186+
method_type_subst_map.extend(&type_id_type_subst_map);
187+
method_type_subst_map.insert(method_implementing_for_typeid, type_id);
188+
189+
method.subst(&SubstTypesContext::new(
190+
engines,
191+
&method_type_subst_map,
192+
true,
193+
));
194+
195+
return engines
196+
.de()
197+
.insert(
198+
method.clone(),
199+
engines.de().get_parsed_decl_id(self.id()).as_ref(),
200+
)
201+
.with_parent(decl_engine, self.id().into());
202+
}
203+
204+
self.clone()
205+
}
206+
}
207+
136208
impl Named for TyFunctionDecl {
137209
fn name(&self) -> &Ident {
138210
&self.name
@@ -481,6 +553,25 @@ impl TyFunctionDecl {
481553
_ => Some(false),
482554
}
483555
}
556+
557+
pub fn is_from_blanket_impl(&self, engines: &Engines) -> bool {
558+
if let Some(TyDecl::ImplSelfOrTrait(existing_impl_trait)) = self.implementing_type.clone() {
559+
let existing_trait_decl = engines
560+
.de()
561+
.get_impl_self_or_trait(&existing_impl_trait.decl_id);
562+
if !existing_trait_decl.impl_type_parameters.is_empty()
563+
&& matches!(
564+
*engines
565+
.te()
566+
.get(existing_trait_decl.implementing_for.type_id),
567+
TypeInfo::UnknownGeneric { .. }
568+
)
569+
{
570+
return true;
571+
}
572+
}
573+
false
574+
}
484575
}
485576

486577
#[derive(Debug, Clone, Serialize, Deserialize)]

sway-core/src/language/ty/expression/expression_variant.rs

-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use crate::{
33
engine_threading::*,
44
has_changes,
55
language::{ty::*, *},
6-
namespace::TryInsertingTraitImplOnFailure,
76
semantic_analysis::{
87
TyNodeDepGraphEdge, TyNodeDepGraphEdgeInfo, TypeCheckAnalysis, TypeCheckAnalysisContext,
98
TypeCheckContext, TypeCheckFinalization, TypeCheckFinalizationContext,
@@ -855,7 +854,6 @@ impl ReplaceDecls for TyExpressionVariant {
855854
.map(|a| a.1.return_type)
856855
.collect::<VecDeque<_>>(),
857856
None,
858-
TryInsertingTraitImplOnFailure::Yes,
859857
)?;
860858
method = (*decl_engine.get(&implementing_type_method_ref)).clone();
861859
}

sway-core/src/semantic_analysis/ast_node/declaration/abi.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
DeclId,
1010
},
1111
language::ty::{TyAbiDecl, TyFunctionDecl},
12-
namespace::{IsExtendingExistingImpl, IsImplSelf, TryInsertingTraitImplOnFailure},
12+
namespace::{IsExtendingExistingImpl, IsImplSelf},
1313
semantic_analysis::{
1414
symbol_collection_context::SymbolCollectionContext, TypeCheckAnalysis,
1515
TypeCheckAnalysisContext, TypeCheckFinalization, TypeCheckFinalizationContext,
@@ -113,7 +113,6 @@ impl ty::TyAbiDecl {
113113
ctx.type_annotation(),
114114
&Default::default(),
115115
None,
116-
TryInsertingTraitImplOnFailure::No,
117116
) {
118117
let superabi_impl_method =
119118
ctx.engines.de().get_function(&superabi_impl_method_ref);
@@ -280,7 +279,6 @@ impl ty::TyAbiDecl {
280279
ctx.type_annotation(),
281280
&Default::default(),
282281
None,
283-
TryInsertingTraitImplOnFailure::No,
284282
) {
285283
let superabi_method =
286284
ctx.engines.de().get_function(&superabi_method_ref);
@@ -355,7 +353,6 @@ impl ty::TyAbiDecl {
355353
ctx.type_annotation(),
356354
&Default::default(),
357355
None,
358-
TryInsertingTraitImplOnFailure::No,
359356
) {
360357
let superabi_impl_method =
361358
ctx.engines.de().get_function(&superabi_impl_method_ref);

sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::{
2222
},
2323
*,
2424
},
25-
namespace::{IsExtendingExistingImpl, IsImplSelf, TraitMap, TryInsertingTraitImplOnFailure},
25+
namespace::{IsExtendingExistingImpl, IsImplSelf, TraitMap},
2626
semantic_analysis::{
2727
symbol_collection_context::SymbolCollectionContext, AbiMode, ConstShadowingMode,
2828
TyNodeDepGraphNodeId, TypeCheckAnalysis, TypeCheckAnalysisContext, TypeCheckContext,
@@ -739,7 +739,6 @@ fn type_check_trait_implementation(
739739
let type_engine = ctx.engines.te();
740740
let decl_engine = ctx.engines.de();
741741
let engines = ctx.engines();
742-
let code_block_first_pass = ctx.code_block_first_pass();
743742

744743
// Check to see if the type that we are implementing for implements the
745744
// supertraits of this trait.
@@ -756,8 +755,6 @@ fn type_check_trait_implementation(
756755
.collect::<Vec<_>>(),
757756
block_span,
758757
engines,
759-
TryInsertingTraitImplOnFailure::Yes,
760-
code_block_first_pass.into(),
761758
)
762759
})?;
763760

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

+40-29
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use crate::{
88
ty::{self, TyDecl, TyExpression, TyFunctionSig},
99
*,
1010
},
11-
namespace::TryInsertingTraitImplOnFailure,
1211
semantic_analysis::*,
1312
type_system::*,
1413
};
@@ -36,6 +35,7 @@ pub(crate) fn type_check_method_application(
3635
let type_engine = ctx.engines.te();
3736
let decl_engine = ctx.engines.de();
3837
let engines = ctx.engines();
38+
let coercion_check = UnifyCheck::coercion(engines);
3939

4040
// type check the function arguments (1st pass)
4141
// Some arguments may fail on this first pass because they may require the type_annotation to the parameter type.
@@ -130,34 +130,49 @@ pub(crate) fn type_check_method_application(
130130
// type check the function arguments (2nd pass)
131131
let mut args_buf = VecDeque::new();
132132
for (arg, index, arg_opt) in izip!(arguments.iter(), 0.., args_opt_buf.iter().cloned()) {
133-
if let (Some(arg), _, false) = arg_opt {
134-
args_buf.push_back(arg);
133+
let param_index = if method.is_contract_call {
134+
if index == 0 {
135+
if let (Some(arg), _, _) = arg_opt {
136+
args_buf.push_back(arg);
137+
}
138+
continue;
139+
}
140+
index - 1 //contract call methods don't have self parameter.
135141
} else {
136-
// We type check the argument expression again this time throwing out the error.
137-
let param_index = if method.is_contract_call {
138-
index - 1 //contract call methods don't have self parameter.
139-
} else {
140-
index
141-
};
142+
index
143+
};
142144

143-
let ctx = if let Some(param) = method.parameters.get(param_index) {
144-
// We now try to type check it again, this time with the type annotation.
145-
ctx.by_ref()
146-
.with_help_text(
147-
"Function application argument type must match function parameter type.",
148-
)
149-
.with_type_annotation(param.type_argument.type_id)
145+
if let (Some(arg), _, false) = arg_opt {
146+
if let Some(param) = method.parameters.get(param_index) {
147+
// If argument type is compcoerces to resolved method parameter type skip second type_check.
148+
if coercion_check.check(arg.return_type, param.type_argument.type_id) {
149+
args_buf.push_back(arg);
150+
continue;
151+
}
150152
} else {
151-
ctx.by_ref()
152-
.with_help_text("")
153-
.with_type_annotation(type_engine.new_unknown())
154-
};
155-
156-
args_buf.push_back(
157-
ty::TyExpression::type_check(handler, ctx, arg)
158-
.unwrap_or_else(|err| ty::TyExpression::error(err, span.clone(), engines)),
159-
);
153+
args_buf.push_back(arg);
154+
continue;
155+
}
160156
}
157+
158+
// We type check the argument expression again this time throwing out the error.
159+
let ctx = if let Some(param) = method.parameters.get(param_index) {
160+
// We now try to type check it again, this time with the type annotation.
161+
ctx.by_ref()
162+
.with_help_text(
163+
"Function application argument type must match function parameter type.",
164+
)
165+
.with_type_annotation(param.type_argument.type_id)
166+
} else {
167+
ctx.by_ref()
168+
.with_help_text("")
169+
.with_type_annotation(type_engine.new_unknown())
170+
};
171+
172+
args_buf.push_back(
173+
ty::TyExpression::type_check(handler, ctx, arg)
174+
.unwrap_or_else(|err| ty::TyExpression::error(err, span.clone(), engines)),
175+
);
161176
}
162177

163178
// check the method visibility
@@ -831,7 +846,6 @@ pub(crate) fn resolve_method_name(
831846
ctx.type_annotation(),
832847
&arguments_types,
833848
None,
834-
TryInsertingTraitImplOnFailure::Yes,
835849
)?;
836850

837851
(decl_ref, type_id)
@@ -876,7 +890,6 @@ pub(crate) fn resolve_method_name(
876890
ctx.type_annotation(),
877891
&arguments_types,
878892
None,
879-
TryInsertingTraitImplOnFailure::Yes,
880893
)?;
881894

882895
(decl_ref, type_id)
@@ -900,7 +913,6 @@ pub(crate) fn resolve_method_name(
900913
ctx.type_annotation(),
901914
&arguments_types,
902915
None,
903-
TryInsertingTraitImplOnFailure::Yes,
904916
)?;
905917

906918
(decl_ref, type_id)
@@ -925,7 +937,6 @@ pub(crate) fn resolve_method_name(
925937
ctx.type_annotation(),
926938
&arguments_types,
927939
Some(*as_trait),
928-
TryInsertingTraitImplOnFailure::Yes,
929940
)?;
930941

931942
(decl_ref, type_id)

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,13 @@ fn collect_struct_constructors(
339339
.filter_map(|item| match item {
340340
ResolvedTraitImplItem::Parsed(_) => unreachable!(),
341341
ResolvedTraitImplItem::Typed(item) => match item {
342-
ty::TyTraitItem::Fn(fn_decl_id) => Some(fn_decl_id),
342+
ty::TyTraitItem::Fn(fn_decl_id) => {
343+
Some(fn_decl_id.get_method_safe_to_unify(engines, struct_type_id))
344+
}
343345
_ => None,
344346
},
345347
})
346-
.map(|fn_decl_id| engines.de().get_function(fn_decl_id))
348+
.map(|fn_decl_id| engines.de().get_function(&fn_decl_id))
347349
.filter(|fn_decl| {
348350
matches!(fn_decl.visibility, Visibility::Public)
349351
&& fn_decl

sway-core/src/semantic_analysis/namespace/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ pub use module::Module;
1313
pub use namespace::Namespace;
1414
pub use root::ResolvedDeclaration;
1515
pub use root::Root;
16-
pub(super) use trait_map::CodeBlockFirstPass;
1716
pub(crate) use trait_map::IsExtendingExistingImpl;
1817
pub(crate) use trait_map::IsImplSelf;
1918
pub(super) use trait_map::ResolvedTraitImplItem;

sway-core/src/semantic_analysis/namespace/root.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -560,11 +560,7 @@ impl Root {
560560
src_mod
561561
.root_items()
562562
.implemented_traits
563-
.filter_by_type_item_import(
564-
type_id,
565-
engines,
566-
super::CodeBlockFirstPass::No,
567-
),
563+
.filter_by_type_item_import(type_id, engines),
568564
engines,
569565
);
570566
}

0 commit comments

Comments
 (0)