Skip to content

Commit

Permalink
Implicit accessor function should return struct members, not struct
Browse files Browse the repository at this point in the history 
If the acessor function returns a single struct, then the members should be returned. If
any of those members are structs, then those members will remain structs.

	contract C {
		struct S { int a; bool b; }

		S public s;

		function foo() public {
			(int a, bool b) = this.s();
		}
	}

Also, the accesor function should be declared `external` and therefore not accessible
as an internal function call.

Signed-off-by: Sean Young <sean@mess.org>
  • Loading branch information
@seanyoung
seanyoung committed Jun 20, 2023
1 parent ec84cab commit 8b8dc30
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 23 deletions.
17 changes: 11 additions & 6 deletions src/abi/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,15 @@ fn constants_and_types() {

assert_eq!(idl.types.len(), 2);

assert_eq!(idl.types[0].name, "MyStruct");
assert!(idl.types[0].docs.is_none());
assert_eq!(idl.types[1].name, "cte5_returns");
assert_eq!(
idl.types[0].ty,
idl.types[1].docs,
Some(vec![
"Data structure to hold the multiple returns of function cte5".into()
])
);
assert_eq!(
idl.types[1].ty,
IdlTypeDefinitionTy::Struct {
fields: vec![
IdlField {
Expand All @@ -104,10 +109,10 @@ fn constants_and_types() {
}
);

assert_eq!(idl.types[1].name, "Week");
assert!(idl.types[1].docs.is_none());
assert_eq!(idl.types[0].name, "Week");
assert!(idl.types[0].docs.is_none());
assert_eq!(
idl.types[1].ty,
idl.types[0].ty,
IdlTypeDefinitionTy::Enum {
variants: vec![
IdlEnumVariant {
Expand Down
155 changes: 141 additions & 14 deletions src/sema/variables.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use solang_parser::{
doccomment::DocComment,
pt::{self, CodeLocation, OptionalCodeLocation},
};
use std::sync::Arc;

pub struct DelayedResolveInitializer<'a> {
var_no: usize,
Expand Down Expand Up @@ -460,6 +461,143 @@ pub fn variable_decl<'a>(
));
}

let body;
let returns;

// If the return value of an accessor function is a struct, return the members rather than
// the struct. This is a particular inconsistency in Solidity which does not apply recursively,
// i.e. if the struct contains structs, then those values are returned as structs.
if let Type::Struct(str) = &ty {
let expr = Arc::new(expr);

if constant {
let var_no = symtable
.add(
&pt::Identifier {
loc: pt::Loc::Implicit,
name: "_".into(),
},
ty.clone(),
ns,
VariableInitializer::Solidity(Some(expr.clone())),
VariableUsage::LocalVariable,
Some(pt::StorageLocation::Storage(pt::Loc::Implicit)),
)
.unwrap();

symtable.vars[&var_no].read = true;
symtable.vars[&var_no].assigned = true;

let def = str.definition(ns);

let list = def
.fields
.iter()
.enumerate()
.map(|(i, param)| Expression::Load {
loc: pt::Loc::Implicit,
ty: param.ty.clone(),
expr: Expression::StructMember {
loc: pt::Loc::Implicit,
ty: Type::Ref(def.fields[i].ty.clone().into()),
expr: Expression::Variable {
loc: pt::Loc::Implicit,
ty: ty.clone(),
var_no,
}
.into(),
field: i,
}
.into(),
})
.collect();

body = vec![
Statement::VariableDecl(pt::Loc::Implicit, var_no, param, Some(expr)),
Statement::Return(
pt::Loc::Implicit,
Some(Expression::List {
loc: pt::Loc::Implicit,
list,
}),
),
];

returns = def.fields.clone();
} else {
let ty = Type::StorageRef(false, ty.clone().into());

let var_no = symtable
.add(
&pt::Identifier {
loc: pt::Loc::Implicit,
name: "_".into(),
},
ty.clone(),
ns,
VariableInitializer::Solidity(Some(expr.clone())),
VariableUsage::LocalVariable,
Some(pt::StorageLocation::Storage(pt::Loc::Implicit)),
)
.unwrap();

symtable.vars[&var_no].read = true;
symtable.vars[&var_no].assigned = true;

let def = str.definition(ns);

let list = def
.fields
.iter()
.enumerate()
.map(|(i, param)| Expression::StorageLoad {
loc: pt::Loc::Implicit,
ty: param.ty.clone(),
expr: Expression::StructMember {
loc: pt::Loc::Implicit,
ty: Type::StorageRef(false, def.fields[i].ty.clone().into()),
expr: Expression::Variable {
loc: pt::Loc::Implicit,
ty: ty.clone(),
var_no,
}
.into(),
field: i,
}
.into(),
})
.collect();

body = vec![
Statement::VariableDecl(pt::Loc::Implicit, var_no, param, Some(expr)),
Statement::Return(
pt::Loc::Implicit,
Some(Expression::List {
loc: pt::Loc::Implicit,
list,
}),
),
];

returns = def.fields.clone();
}
} else {
body = vec![Statement::Return(
pt::Loc::Implicit,
Some(if constant {
expr
} else {
Expression::StorageLoad {
loc: pt::Loc::Implicit,
ty,
expr: Box::new(expr),
}
}),
)];

returns = vec![param];
}

let mut func = Function::new(
def.name.as_ref().unwrap().loc,
def.name.as_ref().unwrap().name.to_owned(),
Expand All @@ -468,25 +606,14 @@ pub fn variable_decl<'a>(
pt::FunctionTy::Function,
// accessors for constant variables have view mutability
Some(pt::Mutability::View(def.name.as_ref().unwrap().loc)),
visibility,
pt::Visibility::External(None),
params,
vec![param],
returns,
ns,
);

// Create the implicit body - just return the value
func.body = vec![Statement::Return(
pt::Loc::Implicit,
Some(if constant {
expr
} else {
Expression::StorageLoad {
loc: pt::Loc::Implicit,
ty,
expr: Box::new(expr),
}
}),
)];
func.body = body;
func.is_accessor = true;
func.has_body = true;
func.is_override = is_override;
Expand Down
10 changes: 10 additions & 0 deletions tests/contract_testcases/solana/accessor/accessor_is_external.sol
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
contract C {
int constant public c = 102;

function f() public {
int x = c();
}
}
// ---- Expect: diagnostics ----
// error: 5:11-14: functions declared external cannot be called via an internal function call
// note 2:22-23: declaration of function 'c'
2 changes: 1 addition & 1 deletion tests/evm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ fn ethereum_solidity_tests() {
})
.sum();

assert_eq!(errors, 1042);
assert_eq!(errors, 1039);
}

fn set_file_contents(source: &str, path: &Path) -> (FileResolver, Vec<String>) {
Expand Down
41 changes: 41 additions & 0 deletions tests/solana_tests/accessor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,44 @@ fn constant() {
])
);
}

#[test]
fn struct_accessor() {
let mut vm = build_solidity(
r#"
contract C {
struct E {
bytes4 b4;
}
struct S {
int64 f1;
bool f2;
E f3;
}
S public a = S({f1: -63, f2: false, f3: E("nuff")});
S[100] public s;
mapping(int => S) public m;
E public constant e = E("cons");
constructor() {
s[99] = S({f1: 65535, f2: true, f3: E("naff")});
m[1023413412] = S({f1: 414243, f2: true, f3: E("niff")});
}
function f() public view {
(int64 a1, bool b, E memory c) = this.a();
require(a1 == -63 && !b && c.b4 == "nuff", "a");
(a1, b, c) = this.s(99);
require(a1 == 65535 && b && c.b4 == "naff", "b");
(a1, b, c) = this.m(1023413412);
require(a1 == 414243 && b && c.b4 == "niff", "c");
c.b4 = this.e();
require(a1 == 414243 && b && c.b4 == "cons", "E");
}
}"#,
);

vm.constructor(&[]);

vm.function("f", &[]);
}
4 changes: 2 additions & 2 deletions tests/substrate_tests/inheritance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,11 +666,11 @@ fn var_or_function() {
r##"
contract x is c {
function f1() public returns (int64) {
return c.selector();
return selector;
}
function f2() public returns (int64) {
function() internal returns (int64) a = c.selector;
function() external returns (int64) a = this.selector;
return a();
}
}
Expand Down

0 comments on commit 8b8dc30

Please sign in to comment.