Skip to content
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

VIP: Convert Struct construction to use kwargs #2381

Closed
Tracked by #2375
fubuloubu opened this issue Jul 8, 2021 · 8 comments · Fixed by #3777
Closed
Tracked by #2375

VIP: Convert Struct construction to use kwargs #2381

fubuloubu opened this issue Jul 8, 2021 · 8 comments · Fixed by #3777
Labels
VIP: Approved VIP Approved
Milestone

Comments

@fubuloubu
Copy link
Member

Simple Summary

Add less overhead and improve type safety of creating a struct

Motivation

Currently, to create a struct it takes a lot of overhead because of the use of modified dict syntax:

struct MyStruct:
    a: address
    b: bytes32
    c: bool

...

s: MyStruct = MyStruct({a: a, b: b, c: True})

This might be a little confusing to newcomers because of how we expected to write strings for dict args (most of the time), plus you don't get the benefit of additional features like struct member name matching or default values for struct members.

Specification

This proposal would modify the structure syntax to the following:

s: MyStruct = MyStruct(a, b, c=True)

The trick is to interpret this as using them like keyword args, where all struct members have a default value (e.g. whatever their "empty" value would be if the struct member didn't appear). We would gain additional type safety here via the member_name=value syntax, which can be bypassed if and only if the name of the variable matches the name of the struct member (e.g. a and b here). This would be typical for input arg names to functions, making it very easy to create a new struct from call args.

Backwards Compatibility

This would be a backwards-incompatible change to the struct creation syntax

Dependencies

No dependnecies

References

No references

Copyright

Copyright and related rights waived via CC0

@fubuloubu fubuloubu added the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Jul 8, 2021
@fubuloubu
Copy link
Member Author

Note: we should do the same with Event logs

@charles-cooper
Copy link
Member

I like the idea of using the kwargs syntax, not so excited about default values and positional args (both increase the risk of misleading code IMO).

@fubuloubu
Copy link
Member Author

fubuloubu commented Jul 8, 2021

I like the idea of using the kwargs syntax, not so excited about default values and positional args (both increase the risk of misleading code IMO).

To be clear, this proposal would not have positional args in the traditional sense, all args would be kwargs (meaning they can be in any position and still work as kwargs because we will use the actual name to determine positioning and disallow out-of-order positioning). This is more akin to how Rust's syntax for structs are, or more like how foo(**kwargs) applies names without using the positioning

Default values are nice for code deduplication. It's a little more opaque, but still acceptable because any good reviewer should know what the "default" value should be


Edit: let's talk about this more, your concern is valid, but the implementation of this would be slightly different than how Python works w/ kwargs

@charles-cooper
Copy link
Member

Yeah let's talk about it more. My thought right now is I think we should split out the default kwargs for its own VIP. I feel like that's a feature that's easy to add and hard to take out.

Also, defaults for kwargs is a semantic change. If we only discuss the syntax here then we can be sure the VIP deals strictly with a syntactic change to the language, that makes it simpler to reason about, any proposed semantic changes can be dealt with separately.

@fubuloubu fubuloubu mentioned this issue Jul 9, 2021
3 tasks
@charles-cooper
Copy link
Member

charles-cooper commented Jul 9, 2021

EDIT: removed my comment because I proposed something @fubuloubu proposed to begin with (because I am a derp)

@fubuloubu
Copy link
Member Author

Here's an idea though, what if we allow "positional" args but only if the variable name is exactly the same as the parameter name.

Yes! This is exactly what I was trying to say above, very much like how Rust handles structs.

@charles-cooper
Copy link
Member

charles-cooper commented Jul 9, 2021

Yes! This is exactly what I was trying to say above, very much like how Rust handles structs.

Yes I see that, lol! My bad

@fubuloubu
Copy link
Member Author

Meeting: only do kwargs

@fubuloubu fubuloubu added VIP: Approved VIP Approved and removed VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting labels Jul 19, 2021
@charles-cooper charles-cooper added this to the v0.4.0 milestone Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Approved VIP Approved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants