Skip to content

Commit

Permalink
Removes TraitMap::insert_for_type.
Browse files Browse the repository at this point in the history
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 reduces significantly the size of the TraitMap 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.
  • Loading branch information
esdrubal committed Feb 4, 2025
1 parent 8c5afa3 commit 057d1ea
Show file tree
Hide file tree
Showing 22 changed files with 745 additions and 652 deletions.
94 changes: 94 additions & 0 deletions sway-core/src/language/ty/declaration/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,81 @@ impl DisplayWithEngines for TyFunctionDecl {
}
}

impl DeclRefFunction {
/// Makes method with a copy of type_id.
/// This avoids altering the type_id already in the type map.
/// Without this it is possible to retrieve a method from the type map unify its types and
/// the second time it won't be possible to retrieve the same method.
pub fn get_method_safe_to_unify(&self, engines: &Engines, type_id: TypeId) -> Self {
let decl_engine = engines.de();

let mut method = (*decl_engine.get_function(self)).clone();

if let Some(method_implementing_for_typeid) = method.implementing_for_typeid {
let mut type_id_type_subst_map = TypeSubstMap::new();
match &method.implementing_type {
Some(TyDecl::ImplSelfOrTrait(t)) => {
let impl_self_or_trait = &*engines.de().get(&t.decl_id);
let mut type_id_type_parameters = vec![];
type_id.extract_type_parameters(
engines,
0,
&mut type_id_type_parameters,
impl_self_or_trait.implementing_for.type_id,
);

for impl_type_parameter in impl_self_or_trait.impl_type_parameters.clone() {
let matches = type_id_type_parameters
.iter()
.filter(|(_, orig_tp)| {
engines.te().get(orig_tp.type_id).eq(
&*engines.te().get(impl_type_parameter.type_id),
&PartialEqWithEnginesContext::new(engines),
)
})
.collect::<Vec<_>>();
if matches.len() > 0 {
type_id_type_subst_map
.insert(impl_type_parameter.type_id, matches[0].0.type_id);
}
if engines
.te()
.get(impl_self_or_trait.implementing_for.initial_type_id)
.eq(
&*engines.te().get(impl_type_parameter.initial_type_id),
&PartialEqWithEnginesContext::new(engines),
)
{
type_id_type_subst_map.insert(impl_type_parameter.type_id, type_id);
}
}
}
_ => {}
}

let mut method_type_subst_map = TypeSubstMap::new();
method_type_subst_map.extend(&type_id_type_subst_map);
method_type_subst_map.insert(method_implementing_for_typeid, type_id);

method.subst(&SubstTypesContext::new(
engines,
&method_type_subst_map,
true,
));

return engines
.de()
.insert(
method.clone(),
engines.de().get_parsed_decl_id(self.id()).as_ref(),
)
.with_parent(decl_engine, self.id().into());
}

self.clone()
}
}

impl Named for TyFunctionDecl {
fn name(&self) -> &Ident {
&self.name
Expand Down Expand Up @@ -481,6 +556,25 @@ impl TyFunctionDecl {
_ => Some(false),
}
}

pub fn is_from_blanket_impl(&self, engines: &Engines) -> bool {
if let Some(TyDecl::ImplSelfOrTrait(existing_impl_trait)) = self.implementing_type.clone() {
let existing_trait_decl = engines
.de()
.get_impl_self_or_trait(&existing_impl_trait.decl_id);
if !existing_trait_decl.impl_type_parameters.is_empty()
&& matches!(
*engines
.te()
.get(existing_trait_decl.implementing_for.type_id),
TypeInfo::UnknownGeneric { .. }
)
{
return true;
}
}
false
}
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down
2 changes: 0 additions & 2 deletions sway-core/src/language/ty/expression/expression_variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::{
engine_threading::*,
has_changes,
language::{ty::*, *},
namespace::TryInsertingTraitImplOnFailure,
semantic_analysis::{
TyNodeDepGraphEdge, TyNodeDepGraphEdgeInfo, TypeCheckAnalysis, TypeCheckAnalysisContext,
TypeCheckContext, TypeCheckFinalization, TypeCheckFinalizationContext,
Expand Down Expand Up @@ -833,7 +832,6 @@ impl ReplaceDecls for TyExpressionVariant {
.map(|a| a.1.return_type)
.collect::<VecDeque<_>>(),
None,
TryInsertingTraitImplOnFailure::Yes,
)?;
method = (*decl_engine.get(&implementing_type_method_ref)).clone();
}
Expand Down
5 changes: 1 addition & 4 deletions sway-core/src/semantic_analysis/ast_node/declaration/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
DeclId,
},
language::ty::{TyAbiDecl, TyFunctionDecl},
namespace::{IsExtendingExistingImpl, IsImplSelf, TryInsertingTraitImplOnFailure},
namespace::{IsExtendingExistingImpl, IsImplSelf},
semantic_analysis::{
symbol_collection_context::SymbolCollectionContext, TypeCheckAnalysis,
TypeCheckAnalysisContext, TypeCheckFinalization, TypeCheckFinalizationContext,
Expand Down Expand Up @@ -113,7 +113,6 @@ impl ty::TyAbiDecl {
ctx.type_annotation(),
&Default::default(),
None,
TryInsertingTraitImplOnFailure::No,
) {
let superabi_impl_method =
ctx.engines.de().get_function(&superabi_impl_method_ref);
Expand Down Expand Up @@ -279,7 +278,6 @@ impl ty::TyAbiDecl {
ctx.type_annotation(),
&Default::default(),
None,
TryInsertingTraitImplOnFailure::No,
) {
let superabi_method =
ctx.engines.de().get_function(&superabi_method_ref);
Expand Down Expand Up @@ -354,7 +352,6 @@ impl ty::TyAbiDecl {
ctx.type_annotation(),
&Default::default(),
None,
TryInsertingTraitImplOnFailure::No,
) {
let superabi_impl_method =
ctx.engines.de().get_function(&superabi_impl_method_ref);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
},
*,
},
namespace::{IsExtendingExistingImpl, IsImplSelf, TraitMap, TryInsertingTraitImplOnFailure},
namespace::{IsExtendingExistingImpl, IsImplSelf, TraitMap},
semantic_analysis::{
symbol_collection_context::SymbolCollectionContext, AbiMode, ConstShadowingMode,
TyNodeDepGraphNodeId, TypeCheckAnalysis, TypeCheckAnalysisContext, TypeCheckContext,
Expand Down Expand Up @@ -692,7 +692,6 @@ fn type_check_trait_implementation(
let type_engine = ctx.engines.te();
let decl_engine = ctx.engines.de();
let engines = ctx.engines();
let code_block_first_pass = ctx.code_block_first_pass();

// Check to see if the type that we are implementing for implements the
// supertraits of this trait.
Expand All @@ -709,8 +708,6 @@ fn type_check_trait_implementation(
.collect::<Vec<_>>(),
block_span,
engines,
TryInsertingTraitImplOnFailure::Yes,
code_block_first_pass.into(),
)
})?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::{
ty::{self, TyDecl, TyExpression, TyFunctionSig},
*,
},
namespace::TryInsertingTraitImplOnFailure,
semantic_analysis::*,
type_system::*,
};
Expand Down Expand Up @@ -36,6 +35,7 @@ pub(crate) fn type_check_method_application(
let type_engine = ctx.engines.te();
let decl_engine = ctx.engines.de();
let engines = ctx.engines();
let coercion_check = UnifyCheck::coercion(engines);

// type check the function arguments (1st pass)
// Some arguments may fail on this first pass because they may require the type_annotation to the parameter type.
Expand Down Expand Up @@ -120,34 +120,46 @@ pub(crate) fn type_check_method_application(
// type check the function arguments (2nd pass)
let mut args_buf = VecDeque::new();
for (arg, index, arg_opt) in izip!(arguments.iter(), 0.., args_opt_buf.iter().cloned()) {
if let (Some(arg), false) = arg_opt {
args_buf.push_back(arg);
let param_index = if method.is_contract_call {
if index == 0 {
continue;
}
index - 1 //contract call methods don't have self parameter.
} else {
// We type check the argument expression again this time throwing out the error.
let param_index = if method.is_contract_call {
index - 1 //contract call methods don't have self parameter.
} else {
index
};
index
};

let ctx = if let Some(param) = method.parameters.get(param_index) {
// We now try to type check it again, this time with the type annotation.
ctx.by_ref()
.with_help_text(
"Function application argument type must match function parameter type.",
)
.with_type_annotation(param.type_argument.type_id)
if let (Some(arg), false) = arg_opt {
if let Some(param) = method.parameters.get(param_index) {
// If argument type is compcoerces to resolved method parameter type skip second type_check.
if coercion_check.check(arg.return_type, param.type_argument.type_id) {
args_buf.push_back(arg);
continue;
}
} else {
ctx.by_ref()
.with_help_text("")
.with_type_annotation(type_engine.new_unknown())
};

args_buf.push_back(
ty::TyExpression::type_check(handler, ctx, arg)
.unwrap_or_else(|err| ty::TyExpression::error(err, span.clone(), engines)),
);
args_buf.push_back(arg);
continue;
}
}

// We type check the argument expression again this time throwing out the error.
let ctx = if let Some(param) = method.parameters.get(param_index) {
// We now try to type check it again, this time with the type annotation.
ctx.by_ref()
.with_help_text(
"Function application argument type must match function parameter type.",
)
.with_type_annotation(param.type_argument.type_id)
} else {
ctx.by_ref()
.with_help_text("")
.with_type_annotation(type_engine.new_unknown())
};

args_buf.push_back(
ty::TyExpression::type_check(handler, ctx, arg)
.unwrap_or_else(|err| ty::TyExpression::error(err, span.clone(), engines)),
);
}

// check the method visibility
Expand Down Expand Up @@ -821,7 +833,6 @@ pub(crate) fn resolve_method_name(
ctx.type_annotation(),
&arguments_types,
None,
TryInsertingTraitImplOnFailure::Yes,
)?;

(decl_ref, type_id)
Expand Down Expand Up @@ -866,7 +877,6 @@ pub(crate) fn resolve_method_name(
ctx.type_annotation(),
&arguments_types,
None,
TryInsertingTraitImplOnFailure::Yes,
)?;

(decl_ref, type_id)
Expand All @@ -890,7 +900,6 @@ pub(crate) fn resolve_method_name(
ctx.type_annotation(),
&arguments_types,
None,
TryInsertingTraitImplOnFailure::Yes,
)?;

(decl_ref, type_id)
Expand All @@ -915,7 +924,6 @@ pub(crate) fn resolve_method_name(
ctx.type_annotation(),
&arguments_types,
Some(*as_trait),
TryInsertingTraitImplOnFailure::Yes,
)?;

(decl_ref, type_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,13 @@ fn collect_struct_constructors(
.filter_map(|item| match item {
ResolvedTraitImplItem::Parsed(_) => unreachable!(),
ResolvedTraitImplItem::Typed(item) => match item {
ty::TyTraitItem::Fn(fn_decl_id) => Some(fn_decl_id),
ty::TyTraitItem::Fn(fn_decl_id) => {
Some(fn_decl_id.get_method_safe_to_unify(engines, struct_type_id))
}
_ => None,
},
})
.map(|fn_decl_id| engines.de().get_function(fn_decl_id))
.map(|fn_decl_id| engines.de().get_function(&fn_decl_id))
.filter(|fn_decl| {
matches!(fn_decl.visibility, Visibility::Public)
&& fn_decl
Expand Down
1 change: 0 additions & 1 deletion sway-core/src/semantic_analysis/namespace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ pub use module::Module;
pub use namespace::Namespace;
pub use root::ResolvedDeclaration;
pub use root::Root;
pub(super) use trait_map::CodeBlockFirstPass;
pub(crate) use trait_map::IsExtendingExistingImpl;
pub(crate) use trait_map::IsImplSelf;
pub(super) use trait_map::ResolvedTraitImplItem;
Expand Down
6 changes: 1 addition & 5 deletions sway-core/src/semantic_analysis/namespace/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,7 @@ impl Root {
src_mod
.root_items()
.implemented_traits
.filter_by_type_item_import(
type_id,
engines,
super::CodeBlockFirstPass::No,
),
.filter_by_type_item_import(type_id, engines),
engines,
);
}
Expand Down
Loading

0 comments on commit 057d1ea

Please sign in to comment.