vyper
vyper copied to clipboard
feat: use kwargs for struct instantiation
What I did
closes: https://github.com/vyperlang/vyper/issues/2381
Change the instantiation of a struct from a dictionary to a callable with kwargs.
How I did it
Update the analysis and codegen, as well as grammar.
How to verify it
See tests
Commit message
feat: use kwargs for struct instantiation
This PR changes the instantiation of a struct from a dictionary to a
callable with kwargs.
Description for the changelog
Use kwargs for struct instantiation.
Cute Animal Picture

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
1fc819c) 84.99% compared to head (a777514) 85.00%. Report is 2 commits behind head on master.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #3777 +/- ##
=======================================
Coverage 84.99% 85.00%
=======================================
Files 92 92
Lines 13717 13731 +14
Branches 3079 3079
=======================================
+ Hits 11659 11672 +13
- Misses 1570 1572 +2
+ Partials 488 487 -1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
btw isn't this tracked in a VIP somewhere? please link
btw isn't this tracked in a VIP somewhere? please link
Yes, I have updated the top-level post with the link to the VIP.
btw isn't this tracked in a VIP somewhere? please link
Yes, I have updated the top-level post with the link to the VIP.
if you use closes: #X it will link and close the VIP as completed when this is merged
btw isn't this tracked in a VIP somewhere? please link
Yes, I have updated the top-level post with the link to the VIP.
if you use
closes: #Xit will link and close the VIP as completed when this is merged
I was under the impression that this PR partially addressed the issue, as there was a separate discussion about having default values, but then I noticed your last comment. Thanks anyway!
I'm not sure whether this is intended behaviour or simply a regression tbh but if I use now the same variable name for the kwarg, it will fail:
struct Batch:
target: address
allow_failure: bool
call_data: Bytes[max_value(uint16)]
struct Result:
success: bool
return_data: Bytes[max_value(uint8)]
@external
def multicall(data: DynArray[Batch, max_value(uint8)]) -> DynArray[Result, max_value(uint8)]:
results: DynArray[Result, max_value(uint8)] = []
return_data: Bytes[max_value(uint8)] = b""
success: bool = empty(bool)
for batch: Batch in data:
if (batch.allow_failure == False):
return_data = raw_call(batch.target, batch.call_data, max_outsize=255)
success = True
results.append(Result({success: success, return_data: return_data})) # This will pass
results.append(Result({success=success, return_data=return_data})) # This will raise
else:
success, return_data = \
raw_call(batch.target, batch.call_data, max_outsize=255, revert_on_failure=False)
results.append(Result({success: success, return_data: return_data})) # This will pass
results.append(Result({success=success, return_data=return_data})) # This will raise
return results
Since the intention was to make it backward-compatible, using e.g. success=success should also compile IMO.
results.append(Result({success=success, return_data=return_data})) # This will raise
The curly brackets are dropped in the new syntax, so this should compile:
results.append(Result(success=success, return_data=return_data))
results.append(Result({success=success, return_data=return_data})) # This will raiseThe curly brackets are dropped in the new syntax, so this should compile:
results.append(Result(success=success, return_data=return_data))
Ah interesting, that indeed compiles, thx! Added this new feature to snekmate modules branch: https://github.com/pcaversaccio/snekmate/pull/207/commits/0c3a20308ef46cfe51cef4912704faa608bea712. Vey nitpick: we should always follow PEP-8 where it states:
Don’t use spaces around the
=sign when used to indicate a keyword argument
The two following tests don't follow this:
- https://github.com/vyperlang/vyper/pull/3777/files#diff-cb5228f23a8a9317e7de9bc3780d62cdfd69bb2655261be60e55143a874615f4
- https://github.com/vyperlang/vyper/pull/3777/files#diff-51af4b3b600a938c59a20303d4a83f82f12746942d85c99bc7284bad5abfdc43
Also, I think it would make sense to update the docs here with the new syntax.
Also, I think it would make sense to update the docs here with the new syntax.
Good point, I will follow up on this and the tests. Thanks!