Skip to content

Commit

Permalink
Fixes assert_eq always passing for U256 (#5054)
Browse files Browse the repository at this point in the history
## Description

The issue was occurring because `get_items_for_type_and_trait_name` was
comparing an absolute call path to an external relative call path, when
the second should also be absolute.

`compile_fn` now checks if method is trait dummy.

These changes should avoid the silent failures where a dummy method is
used,
instead of the proper implementation.

Closes #4989

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [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.
  • Loading branch information
esdrubal authored Aug 30, 2023
1 parent 2d10293 commit 9b986af
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 3 deletions.
8 changes: 8 additions & 0 deletions sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,17 @@ fn compile_fn(
visibility,
purity,
span,
is_trait_method_dummy,
..
} = ast_fn_decl;

if *is_trait_method_dummy {
return Err(vec![CompileError::InternalOwned(
format!("Method {name} is a trait method dummy and was not properly replaced."),
span.clone(),
)]);
}

let args = ast_fn_decl
.parameters
.iter()
Expand Down
12 changes: 10 additions & 2 deletions sway-core/src/language/call_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,11 @@ impl CallPath {
// If the path starts with an external module (i.e. a module that is imported in
// `Forc.toml`, then do not change it since it's a complete path already.
if m.is_external {
self.clone()
CallPath {
prefixes: self.prefixes.clone(),
suffix: self.suffix.clone(),
is_absolute: true,
}
} else {
let mut prefixes: Vec<Ident> = vec![];
if let Some(pkg_name) = &namespace.root().module.name {
Expand All @@ -166,7 +170,11 @@ impl CallPath {
}
}
} else {
self.clone()
CallPath {
prefixes: self.prefixes.clone(),
suffix: self.suffix.clone(),
is_absolute: true,
}
}
}
}
3 changes: 3 additions & 0 deletions sway-core/src/language/ty/declaration/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub struct TyFunctionDecl {
pub is_contract_call: bool,
pub purity: Purity,
pub where_clause: Vec<(Ident, Vec<TraitConstraint>)>,
pub is_trait_method_dummy: bool,
}

impl Named for TyFunctionDecl {
Expand Down Expand Up @@ -85,6 +86,7 @@ impl HashWithEngines for TyFunctionDecl {
attributes: _,
implementing_type: _,
where_clause: _,
is_trait_method_dummy: _,
} = self;
name.hash(state);
body.hash(state, engines);
Expand Down Expand Up @@ -245,6 +247,7 @@ impl TyFunctionDecl {
return_type,
type_parameters: Default::default(),
where_clause,
is_trait_method_dummy: false,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ impl ty::TyFunctionDecl {
is_contract_call,
purity,
where_clause,
is_trait_method_dummy: false,
};

Ok(function_decl)
Expand Down Expand Up @@ -281,6 +282,7 @@ fn test_function_selector_behavior() {
visibility: Visibility::Public,
is_contract_call: false,
where_clause: vec![],
is_trait_method_dummy: false,
};

let selector_text = decl
Expand Down Expand Up @@ -329,6 +331,7 @@ fn test_function_selector_behavior() {
visibility: Visibility::Public,
is_contract_call: false,
where_clause: vec![],
is_trait_method_dummy: false,
};

let selector_text = decl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ impl ty::TyTraitFn {
type_parameters: vec![],
is_contract_call: matches!(abi_mode, AbiMode::ImplAbiFn(..)),
where_clause: vec![],
is_trait_method_dummy: true,
}
}
}
6 changes: 5 additions & 1 deletion sway-core/src/semantic_analysis/ast_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ impl ty::TyAstNode {
let node = ty::TyAstNode {
content: match node.content.clone() {
AstNodeContent::UseStatement(a) => {
let path = if a.is_absolute {
let mut is_external = false;
if let Some(submodule) = ctx.namespace.submodule(&[a.call_path[0].clone()]) {
is_external = submodule.is_external;
}
let path = if is_external || a.is_absolute {
a.call_path.clone()
} else {
ctx.namespace.find_module_path(&a.call_path)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
out
target
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'assert_eq_u256_revert'
source = 'member'
dependencies = ['std']

[[package]]
name = 'core'
source = 'path+from-root-F5B942D6B862CF72'

[[package]]
name = 'std'
source = 'path+from-root-F5B942D6B862CF72'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "assert_eq_u256_revert"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
{
"configurables": [],
"functions": [
{
"attributes": null,
"inputs": [],
"name": "main",
"output": {
"name": "",
"type": 0,
"typeArguments": null
}
}
],
"loggedTypes": [
{
"logId": 0,
"loggedType": {
"name": "",
"type": 1,
"typeArguments": []
}
},
{
"logId": 1,
"loggedType": {
"name": "",
"type": 1,
"typeArguments": []
}
}
],
"messagesTypes": [],
"types": [
{
"components": [],
"type": "()",
"typeId": 0,
"typeParameters": null
},
{
"components": [
{
"name": "a",
"type": 2,
"typeArguments": null
},
{
"name": "b",
"type": 2,
"typeArguments": null
},
{
"name": "c",
"type": 2,
"typeArguments": null
},
{
"name": "d",
"type": 2,
"typeArguments": null
}
],
"type": "struct std::u256::U256",
"typeId": 1,
"typeParameters": null
},
{
"components": null,
"type": "u64",
"typeId": 2,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
script;

use std::u256::*;

fn main() {
let five = U256::from((0, 0, 0, 5));
let two = U256::from((0, 0, 0, 2));
assert_eq(five, two);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "run"
expected_result = { action = "revert", value = -65533 } # 0xffffffffffff0003 as i64
validate_abi = true

0 comments on commit 9b986af

Please sign in to comment.