feat: struct destructuring #2243
Conversation
…elLabs/sway into mattauer/struct_destructuring
…elLabs/sway into mattauer/struct_destructuring
There was a problem hiding this comment.
Love the changes -- clean, in line with @emilyaherbert's awesome tuple destructuring impl, and well commented.
Two things --
- Could you document this in the
docs/folder as a part of this PR? - Does this support nested destructuring? If so, we should test it. If not, we should document that limitation for both structs and tuples.
| let tuple_tys_opt = match ty_opt { | ||
| Some(Ty::Tuple(tys)) => Some(tys.into_inner().to_tys()), | ||
| _ => None, | ||
| }; | ||
|
|
There was a problem hiding this comment.
thanks for these comments, great addition
| @@ -0,0 +1,3 @@ | |||
| category = "run" | |||
| expected_result = { action = "return", value = 42 } | |||
| validate_abi = true | |||
There was a problem hiding this comment.
as this isn't a contract, do we need to validate the abi?
There was a problem hiding this comment.
Good catch! I'm not sure, there are other test cases in the same folder such as tuple_access that are scripts and validate the abi.
There was a problem hiding this comment.
Hm, I don't think it is necessary. Cc @otrho for consensus on that.
There was a problem hiding this comment.
We've been validating the ABI for all passing tests including scripts because scripts emit a JSON ABI now. Not sure how important that is though.
There was a problem hiding this comment.
BTW, all I did was refactor the test harness. This doesn't make me the authority on how or what to test. 😉
| let line = Line { p1: point1, p2: point2 }; | ||
| let Line { p1: Point { x: x0, y: y0 }, p2: Point { x: x1, y: y1} } = line; |
There was a problem hiding this comment.
alright, don't hate me, but what about a nested tuple in a struct and vice versa?
There was a problem hiding this comment.
Yeah I can add a test for that. Should I also include an example for the docs?
There was a problem hiding this comment.
Docs example could be cool, to show people just how deep destructuring can go!
| @@ -0,0 +1,3 @@ | |||
| category = "run" | |||
| expected_result = { action = "return", value = 42 } | |||
| validate_abi = true | |||
There was a problem hiding this comment.
BTW, all I did was refactor the test harness. This doesn't make me the authority on how or what to test. 😉
closes issue #1167
There is definitely some repeated code between the Struct and Tuple Pattern branches in the statement_let_to_ast_nodes function. Let me know if I should add some helper functions.