Skip to content

Commit

Permalink
Fixes mismatched types in ABI cast. (#6489)
Browse files Browse the repository at this point in the history
## Description

ABI cast generates TypeInfo::ContractCaller which contains an address
that is an expression. While doing the first and second code block
passes we would obtain different expressions because of first passes
optimizations.

This PR disables first pass optimizations while type checking address
expression so there aren't any mismatched types.

## 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]>
Co-authored-by: Sophie Dankel <[email protected]>
  • Loading branch information
3 people authored Sep 6, 2024
1 parent 2538442 commit 9132ca8
Show file tree
Hide file tree
Showing 19 changed files with 135 additions and 73 deletions.
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/ast_node/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl ty::TyCodeBlock {
// This is required to fix the test case numeric_type_propagation and issue #6371
ctx.by_ref()
.with_collecting_unifications()
.with_code_block_first_pass(true)
.scoped(|mut ctx| {
code_block.contents.iter().for_each(|node| {
ty::TyAstNode::type_check(&Handler::default(), ctx.by_ref(), node).ok();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl ty::TyConfigurableDecl {
// this subst is required to replace associated types, namely TypeInfo::TraitType.
type_ascription.type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

if !is_screaming_snake_case(name.as_str()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl ty::TyConstantDecl {
// this subst is required to replace associated types, namely TypeInfo::TraitType.
type_ascription.type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

if !is_screaming_snake_case(name.as_str()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl TyDecl {
},
};

if !ctx.collecting_unifications() {
if !ctx.code_block_first_pass() {
let previous_symbol = ctx
.namespace()
.module(engines)
Expand Down
28 changes: 14 additions & 14 deletions sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ fn type_check_trait_implementation(
let type_engine = ctx.engines.te();
let decl_engine = ctx.engines.de();
let engines = ctx.engines();
let collecting_unifications = ctx.collecting_unifications();
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 @@ -665,7 +665,7 @@ fn type_check_trait_implementation(
block_span,
engines,
TryInsertingTraitImplOnFailure::Yes,
collecting_unifications.into(),
code_block_first_pass.into(),
)
})?;

Expand Down Expand Up @@ -807,7 +807,7 @@ fn type_check_trait_implementation(

type_decl.subst(
&trait_type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

// Remove this type from the checklist.
Expand Down Expand Up @@ -881,7 +881,7 @@ fn type_check_trait_implementation(

impl_method.subst(
&trait_type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

// Remove this method from the checklist.
Expand All @@ -907,7 +907,7 @@ fn type_check_trait_implementation(

const_decl.subst(
&trait_type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

// Remove this constant from the checklist.
Expand Down Expand Up @@ -980,7 +980,7 @@ fn type_check_trait_implementation(
method.replace_decls(&decl_mapping, handler, &mut ctx)?;
method.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items_refs.push(TyImplItem::Fn(
decl_engine
Expand All @@ -996,7 +996,7 @@ fn type_check_trait_implementation(
const_decl.replace_decls(&decl_mapping, handler, &mut ctx)?;
const_decl.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items_refs.push(TyImplItem::Constant(decl_engine.insert(
const_decl,
Expand All @@ -1007,7 +1007,7 @@ fn type_check_trait_implementation(
let mut type_decl = (*decl_engine.get_type(decl_ref)).clone();
type_decl.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items_refs.push(TyImplItem::Type(decl_engine.insert(
type_decl.clone(),
Expand Down Expand Up @@ -1153,14 +1153,14 @@ fn type_check_impl_method(
let mut impl_method_param_type_id = impl_method_param.type_argument.type_id;
impl_method_param_type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

let mut impl_method_signature_param_type_id =
impl_method_signature_param.type_argument.type_id;
impl_method_signature_param_type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

if !UnifyCheck::non_dynamic_equality(engines).check(
Expand Down Expand Up @@ -1233,14 +1233,14 @@ fn type_check_impl_method(
let mut impl_method_return_type_id = impl_method.return_type.type_id;
impl_method_return_type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

let mut impl_method_signature_return_type_type_id =
impl_method_signature.return_type.type_id;
impl_method_signature_return_type_type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

if !UnifyCheck::non_dynamic_equality(engines).check(
Expand Down Expand Up @@ -1349,13 +1349,13 @@ fn type_check_const_decl(
let mut const_decl_type_id = const_decl.type_ascription.type_id;
const_decl_type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

let mut const_decl_signature_type_id = const_decl_signature.type_ascription.type_id;
const_decl_signature_type_id.subst(
&ctx.type_subst(),
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);

// unify the types from the constant with the constant signature
Expand Down
14 changes: 7 additions & 7 deletions sway-core/src/semantic_analysis/ast_node/declaration/trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ impl TyTraitDecl {
let r = if method
.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
)
.has_changes()
{
Expand All @@ -380,7 +380,7 @@ impl TyTraitDecl {
let r = if const_decl
.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
)
.has_changes()
{
Expand All @@ -399,7 +399,7 @@ impl TyTraitDecl {
let r = if t
.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
)
.has_changes()
{
Expand Down Expand Up @@ -456,7 +456,7 @@ impl TyTraitDecl {
let mut method = (*decl_engine.get_trait_fn(decl_ref)).clone();
method.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items.push(TyImplItem::Fn(
decl_engine
Expand Down Expand Up @@ -487,7 +487,7 @@ impl TyTraitDecl {
let mut method = (*decl_engine.get_function(decl_ref)).clone();
method.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items.push(TyImplItem::Fn(
ctx.engines
Expand All @@ -503,7 +503,7 @@ impl TyTraitDecl {
let mut const_decl = (*decl_engine.get_constant(decl_ref)).clone();
const_decl.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items.push(TyImplItem::Constant(decl_engine.insert(
const_decl,
Expand All @@ -514,7 +514,7 @@ impl TyTraitDecl {
let mut type_decl = (*decl_engine.get_type(decl_ref)).clone();
type_decl.subst(
&type_mapping,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
all_items.push(TyImplItem::Type(decl_engine.insert(
type_decl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1643,10 +1643,14 @@ impl ty::TyExpression {
// type check the address and make sure it is
let err_span = address.span().clone();
let address_expr = {
// We want to type check the address expression as we do in the second pass, otherwise we get
// mismatched types while comparing TypeInfo::ContractCaller which is the return type of
// TyExpressionVariant::AbiCast.
let ctx = ctx
.by_ref()
.with_help_text("An address that is being ABI cast must be of type b256")
.with_type_annotation(type_engine.insert(engines, TypeInfo::B256, None));
.with_type_annotation(type_engine.insert(engines, TypeInfo::B256, None))
.with_code_block_first_pass(false);
ty::TyExpression::type_check(handler, ctx, address)
.unwrap_or_else(|err| ty::TyExpression::error(err, err_span, engines))
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub(crate) fn instantiate_function_application(
{
cached_fn_ref
} else {
if !ctx.collecting_unifications() {
if !ctx.code_block_first_pass() {
// Handle the trait constraints. This includes checking to see if the trait
// constraints are satisfied and replacing old decl ids based on the
// constraint with new decl ids based on the new type.
Expand Down Expand Up @@ -113,7 +113,7 @@ pub(crate) fn instantiate_function_application(
)
.with_parent(decl_engine, (*function_decl_ref.id()).into());

if !ctx.collecting_unifications()
if !ctx.code_block_first_pass()
&& method_sig.is_concrete(engines)
&& function_is_type_check_finalized
&& !function_is_trait_method_dummy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,14 +672,14 @@ pub(crate) fn type_check_method_application(
);
method.subst(
&type_subst,
&SubstTypesContext::new(engines, !ctx.collecting_unifications()),
&SubstTypesContext::new(engines, !ctx.code_block_first_pass()),
);
}
}
}
}

if !ctx.collecting_unifications() {
if !ctx.code_block_first_pass() {
// Handle the trait constraints. This includes checking to see if the trait
// constraints are satisfied and replacing old decl ids based on the
// constraint with new decl ids based on the new type.
Expand All @@ -702,7 +702,7 @@ pub(crate) fn type_check_method_application(
method_return_type_id = method.return_type.type_id;
decl_engine.replace(*fn_ref.id(), method.clone());

if !ctx.collecting_unifications()
if !ctx.code_block_first_pass()
&& method_sig.is_concrete(engines)
&& method.is_type_check_finalized
&& !method.is_trait_method_dummy
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/namespace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use namespace::Namespace;
pub use namespace::TryInsertingTraitImplOnFailure;
pub use root::ResolvedDeclaration;
pub use root::Root;
pub(super) use trait_map::CollectingUnification;
pub(super) use trait_map::CodeBlockFirstPass;
pub(super) use trait_map::IsExtendingExistingImpl;
pub(super) use trait_map::IsImplSelf;
pub(super) use trait_map::ResolvedTraitImplItem;
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/namespace/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ impl Root {
.filter_by_type_item_import(
type_id,
engines,
super::CollectingUnification::No,
super::CodeBlockFirstPass::No,
),
engines,
);
Expand Down
Loading

0 comments on commit 9132ca8

Please sign in to comment.