-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lower list expressions #388
Conversation
BTW, the parser doesn't allow newlines inside of square brackets at the moment. I made an issue for it #389. |
1f05ba2
to
f789cba
Compare
.contract_scope() | ||
.borrow() | ||
.function_def(func_name) | ||
}) = called_func |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this out of the if let
was needed to mitigate a borrow_mut()
violation...this took me way to long to find 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, Ive ran into this too
// Turn List Expression into a function call | ||
return fe::Expr::Call { | ||
func: fe::Expr::Attribute { | ||
value: fe::Expr::Name(_self.to_string()).into_boxed_node(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure there's a nicer way to get the string of the Object::Self
that would allow me to move it back inline here but I wasn't able to find it. /cc @sbillig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the best way is probably to derive strum::ToString
instead of (or in addition to) IntoStaticStr
, and do fe::Expr::Name(Object::Self_.to_string())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, it's that simple, thanks!
Codecov Report
@@ Coverage Diff @@
## master #388 +/- ##
==========================================
+ Coverage 86.33% 86.51% +0.17%
==========================================
Files 66 67 +1
Lines 4011 4048 +37
==========================================
+ Hits 3463 3502 +39
+ Misses 548 546 -2
Continue to review full report at Codecov.
|
contract Foo: | ||
|
||
pub def bar() -> u256: | ||
my_array: u256[3] = [10, 20, 30] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if we, say, use an array of size 20? 👿
Im wondering if we have stack problems during yul compilation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, I missed to check that...I'll check it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the tests, appears to be working fine 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good 👍
.contract_scope() | ||
.borrow() | ||
.function_def(func_name) | ||
}) = called_func |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, Ive ran into this too
f789cba
to
cab4436
Compare
cab4436
to
4fb26e6
Compare
What was wrong?
We do not yet support list expressions e.g.
[1, 2, 3]
How was it fixed?
into_node()
andinto_boxed_node()
methods to make working without significant spans less annoying