Skip to content

Commit

Permalink
Fixes Numeric type propagation (#6461)
Browse files Browse the repository at this point in the history
## Description

To fix the Numeric type propagation we now do the type checking of code
blocks twice, first to collect all the unification and second to unify
the types of a previous namespace with the variable declarations types
of the current namespace.

This change incurs a performance drop in the compilation time. The
benchmark `compile` takes 25% more time to run.

This can be improved significantly by using LexicalScopes instead of
cloning the whole namespace on each scope.

Fixes #6371


## 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)
- [x] 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: IGI-111 <[email protected]>
  • Loading branch information
esdrubal and IGI-111 authored Aug 28, 2024
1 parent 4fc554c commit 4698d37
Show file tree
Hide file tree
Showing 58 changed files with 1,029 additions and 380 deletions.
54 changes: 27 additions & 27 deletions sway-core/src/decl_engine/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ where
}

impl SubstTypes for DeclId<TyFunctionDecl> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -155,10 +155,10 @@ impl SubstTypes for DeclId<TyFunctionDecl> {
}
}
impl SubstTypes for DeclId<TyTraitDecl> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -167,10 +167,10 @@ impl SubstTypes for DeclId<TyTraitDecl> {
}
}
impl SubstTypes for DeclId<TyTraitFn> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -179,10 +179,10 @@ impl SubstTypes for DeclId<TyTraitFn> {
}
}
impl SubstTypes for DeclId<TyImplSelfOrTrait> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -191,10 +191,10 @@ impl SubstTypes for DeclId<TyImplSelfOrTrait> {
}
}
impl SubstTypes for DeclId<TyStructDecl> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -203,10 +203,10 @@ impl SubstTypes for DeclId<TyStructDecl> {
}
}
impl SubstTypes for DeclId<TyEnumDecl> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -215,10 +215,10 @@ impl SubstTypes for DeclId<TyEnumDecl> {
}
}
impl SubstTypes for DeclId<TyTypeAliasDecl> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -228,10 +228,10 @@ impl SubstTypes for DeclId<TyTypeAliasDecl> {
}

impl SubstTypes for DeclId<TyTraitType> {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(*self, decl);
HasChanges::Yes
} else {
Expand All @@ -248,11 +248,11 @@ where
pub(crate) fn subst_types_and_insert_new(
&self,
type_mapping: &TypeSubstMap,
engines: &Engines,
ctx: &SubstTypesContext,
) -> Option<DeclRef<Self>> {
let decl_engine = engines.de();
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(self)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
Some(decl_engine.insert(decl, decl_engine.get_parsed_decl_id(self).as_ref()))
} else {
None
Expand Down
50 changes: 31 additions & 19 deletions sway-core/src/decl_engine/ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,23 @@ impl<T> DeclRef<DeclId<T>> {
impl<T> DeclRef<DeclId<T>>
where
DeclEngine: DeclEngineIndex<T> + DeclEngineInsert<T> + DeclEngineGetParsedDeclId<T>,
T: Named + Spanned + SubstTypes + Clone + TyDeclParsedType,
T: Named + Spanned + IsConcrete + SubstTypes + Clone + TyDeclParsedType,
{
pub(crate) fn subst_types_and_insert_new(
&self,
type_mapping: &TypeSubstMap,
engines: &Engines,
ctx: &SubstTypesContext,
) -> Option<Self> {
let decl_engine = engines.de();
let mut decl = (*decl_engine.get(&self.id)).clone();
if decl.subst(type_mapping, engines).has_changes() {
Some(decl_engine.insert(decl, decl_engine.get_parsed_decl_id(&self.id).as_ref()))
let decl_engine = ctx.engines.de();
if type_mapping.source_ids_contains_concrete_type(ctx.engines)
|| !decl_engine.get(&self.id).is_concrete(ctx.engines)
{
let mut decl = (*decl_engine.get(&self.id)).clone();
if decl.subst(type_mapping, ctx).has_changes() {
Some(decl_engine.insert(decl, decl_engine.get_parsed_decl_id(&self.id).as_ref()))
} else {
None
}
} else {
None
}
Expand All @@ -133,21 +139,27 @@ impl<T> DeclRef<DeclId<T>>
where
AssociatedItemDeclId: From<DeclId<T>>,
DeclEngine: DeclEngineIndex<T> + DeclEngineInsert<T> + DeclEngineGetParsedDeclId<T>,
T: Named + Spanned + SubstTypes + Clone + TyDeclParsedType,
T: Named + Spanned + IsConcrete + SubstTypes + Clone + TyDeclParsedType,
{
pub(crate) fn subst_types_and_insert_new_with_parent(
&self,
type_mapping: &TypeSubstMap,
engines: &Engines,
ctx: &SubstTypesContext,
) -> Option<Self> {
let decl_engine = engines.de();
let mut decl = (*decl_engine.get(&self.id)).clone();
if decl.subst(type_mapping, engines).has_changes() {
Some(
decl_engine
.insert(decl, decl_engine.get_parsed_decl_id(&self.id).as_ref())
.with_parent(decl_engine, self.id.into()),
)
let decl_engine = ctx.engines.de();
if type_mapping.source_ids_contains_concrete_type(ctx.engines)
|| !decl_engine.get(&self.id).is_concrete(ctx.engines)
{
let mut decl = (*decl_engine.get(&self.id)).clone();
if decl.subst(type_mapping, ctx).has_changes() {
Some(
decl_engine
.insert(decl, decl_engine.get_parsed_decl_id(&self.id).as_ref())
.with_parent(decl_engine, self.id.into()),
)
} else {
None
}
} else {
None
}
Expand Down Expand Up @@ -257,10 +269,10 @@ where
DeclEngine: DeclEngineIndex<T>,
T: Named + Spanned + SubstTypes + Clone,
{
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
let decl_engine = engines.de();
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
let decl_engine = ctx.engines.de();
let mut decl = (*decl_engine.get(&self.id)).clone();
if decl.subst(type_mapping, engines).has_changes() {
if decl.subst(type_mapping, ctx).has_changes() {
decl_engine.replace(self.id, decl);
HasChanges::Yes
} else {
Expand Down
6 changes: 3 additions & 3 deletions sway-core/src/language/ty/ast_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ impl DebugWithEngines for TyAstNode {
}

impl SubstTypes for TyAstNode {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
match self.content {
TyAstNodeContent::Declaration(ref mut decl) => decl.subst(type_mapping, engines),
TyAstNodeContent::Expression(ref mut expr) => expr.subst(type_mapping, engines),
TyAstNodeContent::Declaration(ref mut decl) => decl.subst(type_mapping, ctx),
TyAstNodeContent::Expression(ref mut expr) => expr.subst(type_mapping, ctx),
TyAstNodeContent::SideEffect(_) | TyAstNodeContent::Error(_, _) => HasChanges::No,
}
}
Expand Down
4 changes: 2 additions & 2 deletions sway-core/src/language/ty/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ impl HashWithEngines for TyCodeBlock {
}

impl SubstTypes for TyCodeBlock {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
self.contents.subst(type_mapping, engines)
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
self.contents.subst(type_mapping, ctx)
}
}

Expand Down
8 changes: 4 additions & 4 deletions sway-core/src/language/ty/declaration/configurable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ impl Spanned for TyConfigurableDecl {
}

impl SubstTypes for TyConfigurableDecl {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
has_changes! {
self.return_type.subst(type_mapping, engines);
self.type_ascription.subst(type_mapping, engines);
self.value.subst(type_mapping, engines);
self.return_type.subst(type_mapping, ctx);
self.type_ascription.subst(type_mapping, ctx);
self.value.subst(type_mapping, ctx);
}
}
}
Expand Down
15 changes: 11 additions & 4 deletions sway-core/src/language/ty/declaration/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,19 @@ impl Spanned for TyConstantDecl {
}
}

impl IsConcrete for TyConstantDecl {
fn is_concrete(&self, engines: &Engines) -> bool {
self.return_type
.is_concrete(engines, TreatNumericAs::Concrete)
}
}

impl SubstTypes for TyConstantDecl {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
has_changes! {
self.return_type.subst(type_mapping, engines);
self.type_ascription.subst(type_mapping, engines);
self.value.subst(type_mapping, engines);
self.return_type.subst(type_mapping, ctx);
self.type_ascription.subst(type_mapping, ctx);
self.value.subst(type_mapping, ctx);
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions sway-core/src/language/ty/declaration/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,33 +250,33 @@ impl HashWithEngines for TyDecl {
}

impl SubstTypes for TyDecl {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
match self {
TyDecl::VariableDecl(ref mut var_decl) => var_decl.subst(type_mapping, engines),
TyDecl::VariableDecl(ref mut var_decl) => var_decl.subst(type_mapping, ctx),
TyDecl::FunctionDecl(FunctionDecl {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
TyDecl::TraitDecl(TraitDecl {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
TyDecl::StructDecl(StructDecl {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
TyDecl::EnumDecl(EnumDecl {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
TyDecl::EnumVariantDecl(EnumVariantDecl {
ref mut enum_ref, ..
}) => enum_ref.subst(type_mapping, engines),
}) => enum_ref.subst(type_mapping, ctx),
TyDecl::ImplSelfOrTrait(ImplSelfOrTrait {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
TyDecl::TypeAliasDecl(TypeAliasDecl {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
TyDecl::TraitTypeDecl(TraitTypeDecl {
ref mut decl_id, ..
}) => decl_id.subst(type_mapping, engines),
}) => decl_id.subst(type_mapping, ctx),
// generics in an ABI is unsupported by design
TyDecl::AbiDecl(_)
| TyDecl::ConstantDecl(_)
Expand Down
18 changes: 13 additions & 5 deletions sway-core/src/language/ty/declaration/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ impl HashWithEngines for TyEnumDecl {
}

impl SubstTypes for TyEnumDecl {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
has_changes! {
self.variants.subst(type_mapping, engines);
self.type_parameters.subst(type_mapping, engines);
self.variants.subst(type_mapping, ctx);
self.type_parameters.subst(type_mapping, ctx);
}
}
}
Expand All @@ -84,6 +84,14 @@ impl Spanned for TyEnumDecl {
}
}

impl IsConcrete for TyEnumDecl {
fn is_concrete(&self, engines: &Engines) -> bool {
self.type_parameters
.iter()
.all(|tp| tp.is_concrete(engines))
}
}

impl MonomorphizeHelper for TyEnumDecl {
fn type_parameters(&self) -> &[TypeParameter] {
&self.type_parameters
Expand Down Expand Up @@ -178,7 +186,7 @@ impl OrdWithEngines for TyEnumVariant {
}

impl SubstTypes for TyEnumVariant {
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) -> HasChanges {
self.type_argument.subst_inner(type_mapping, engines)
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, ctx: &SubstTypesContext) -> HasChanges {
self.type_argument.subst_inner(type_mapping, ctx)
}
}
Loading

0 comments on commit 4698d37

Please sign in to comment.