Implicit accessor function should return struct members, not struct #1374
Conversation
seanyoung
force-pushed
the
destructure
branch
6 times, most recently
from
8b8dc30
to
7ea2dd3
Compare
src/sema/variables.rs
Outdated
| 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, | ||
| }), | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
What do you think of building the function body during codegen?
There was a problem hiding this comment.
I can see the argument for keeping the ast more "pure", but on the other hand I don't think it is that ugly. However, I am not sure what this would look like, code wise.
We need to build the function prototype in sema, that includes the parameters and return values. Once we've calculated all of that, we've already done almost all the work to build up the body. We would need to do the same work again in codegen to build the body, or somehow share this code.
Maybe something for another day.
There was a problem hiding this comment.
In principle I agree with Lucas, it seems odd to sort of generate code not in codegen but in sema. However there might be more such instances. I also thought a bit about it and didn't come to an immediate conclusion or solution to that question.
src/sema/variables.rs
Outdated
| ]; | ||
|
|
||
| returns = def.fields.clone(); | ||
| } |
There was a problem hiding this comment.
The two branches of if constant { .. } else {...} have nearly identical code. You can simplify most of it.
There was a problem hiding this comment.
I've simplified it, not sure it can be simplified more
There was a problem hiding this comment.
Your new version looks much cleaner. Thanks!
| // error: 5:11-14: functions declared external cannot be called via an internal function call | ||
| // note 2:22-23: declaration of function 'c' |
There was a problem hiding this comment.
The note points to a public variable, while the error says functions declared external. Don't we need a clearer error message for this case?
There was a problem hiding this comment.
Good idea, I've improved this.
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 <[email protected]>
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.
Also, the accesor function should be declared
externaland therefore not accessible as an internal function call.