-
Notifications
You must be signed in to change notification settings - Fork 374
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
Feature: support named and unnamed type assignment #1141
Labels
🐞 bug
Something isn't working
Milestone
Comments
moul
added a commit
that referenced
this issue
Sep 22, 2023
This the first PR required for this feature #1141 --------- Co-authored-by: piux2 <> Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
7 tasks
gfanton
pushed a commit
to gfanton/gno
that referenced
this issue
Nov 9, 2023
This the first PR required for this feature gnolang#1141 --------- Co-authored-by: piux2 <> Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
Merged
thehowl
added a commit
that referenced
this issue
May 31, 2024
This is part 2 of 3 of the solution for issue #1141. The part 1 of 3 of the solution can be found in issue #1143. In this part of the solution, we have made several improvements: - Support both named and unnamed type assignments in assignment statements and function return values. - Resolved the issue related to incorrect method selectors that is caused by mixing named and unnamed assignments. - Added 62 file tests to ensure the correctness of the code. - Included 2 realm tests to further validate the cross realm assignment and method selector. - Enhanced the support for GNO file tests in nested directories. This allows us to organize tests in intuitively named folders. To achieve the above improvements in the preprocessing phase, we made the following changes: - Introduced an isNamed() function on the Type Interface and marked named types with isNamed() returning true. This helps distinguish between named and unnamed types. - Followed the specifications to convert the right-hand side type into a constant function type. - As for determining the package associated with a test file, we've maintained the original convention. We keeps relying on the comment directive "//PKGPATH: gno.land/r/xyz" in the test file itself to identify the package it belongs to. We do not using the local folder structure to derive the package for file tests. Therefore the tests in tests/files folder can be stored in any intuitively named sub directories. **NOTE:** The named and unnamed type conversions that involve the decomposition of function calls returning multiple values in the preprocess have not yet been included in this pull request. This functionality will be addressed in part 3 of 3 of the entire solution. <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: deelawn <dboltz03@gmail.com> Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
DIGIX666
pushed a commit
to DIGIX666/gno
that referenced
this issue
Jun 2, 2024
This is part 2 of 3 of the solution for issue gnolang#1141. The part 1 of 3 of the solution can be found in issue gnolang#1143. In this part of the solution, we have made several improvements: - Support both named and unnamed type assignments in assignment statements and function return values. - Resolved the issue related to incorrect method selectors that is caused by mixing named and unnamed assignments. - Added 62 file tests to ensure the correctness of the code. - Included 2 realm tests to further validate the cross realm assignment and method selector. - Enhanced the support for GNO file tests in nested directories. This allows us to organize tests in intuitively named folders. To achieve the above improvements in the preprocessing phase, we made the following changes: - Introduced an isNamed() function on the Type Interface and marked named types with isNamed() returning true. This helps distinguish between named and unnamed types. - Followed the specifications to convert the right-hand side type into a constant function type. - As for determining the package associated with a test file, we've maintained the original convention. We keeps relying on the comment directive "//PKGPATH: gno.land/r/xyz" in the test file itself to identify the package it belongs to. We do not using the local folder structure to derive the package for file tests. Therefore the tests in tests/files folder can be stored in any intuitively named sub directories. **NOTE:** The named and unnamed type conversions that involve the decomposition of function calls returning multiple values in the preprocess have not yet been included in this pull request. This functionality will be addressed in part 3 of 3 of the entire solution. <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: deelawn <dboltz03@gmail.com> Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
omarsy
pushed a commit
to TERITORI/gno
that referenced
this issue
Jun 3, 2024
This is part 2 of 3 of the solution for issue gnolang#1141. The part 1 of 3 of the solution can be found in issue gnolang#1143. In this part of the solution, we have made several improvements: - Support both named and unnamed type assignments in assignment statements and function return values. - Resolved the issue related to incorrect method selectors that is caused by mixing named and unnamed assignments. - Added 62 file tests to ensure the correctness of the code. - Included 2 realm tests to further validate the cross realm assignment and method selector. - Enhanced the support for GNO file tests in nested directories. This allows us to organize tests in intuitively named folders. To achieve the above improvements in the preprocessing phase, we made the following changes: - Introduced an isNamed() function on the Type Interface and marked named types with isNamed() returning true. This helps distinguish between named and unnamed types. - Followed the specifications to convert the right-hand side type into a constant function type. - As for determining the package associated with a test file, we've maintained the original convention. We keeps relying on the comment directive "//PKGPATH: gno.land/r/xyz" in the test file itself to identify the package it belongs to. We do not using the local folder structure to derive the package for file tests. Therefore the tests in tests/files folder can be stored in any intuitively named sub directories. **NOTE:** The named and unnamed type conversions that involve the decomposition of function calls returning multiple values in the preprocess have not yet been included in this pull request. This functionality will be addressed in part 3 of 3 of the entire solution. <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: deelawn <dboltz03@gmail.com> Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
7 tasks
piux2
added a commit
that referenced
this issue
Jul 19, 2024
This is the last part of the solution for issue #1141. The 1 of 3 of the solution can be found in PR #1143. The 2 of 3 of the solution can be found in PR #1246 It decomposes function calls that return multiple values in the preprocess. ### Here is the problem to solve: ` u1, n2 = x() ` How do we ensure that the returned multiple values from a function call adhere to named and unnamed type assignment specifications? Additionally, we want to solve this problem during preprocessing instead of at runtime to minimize the impact on runtime performance. ### The main ideas: u1, n2 = x() << decompose the statement to the following two lines // .tmp_1, .tmp_2 := x() // u1, n2 = .tmp_1, .tmp_2 then we can apply name and unname type conversion specs to the second line. u1, n2 = _tmp_1, _tmp_2 ### Here are the example code and the explanation ``` // decompose_filetest.gno package main type nat []int func x() (nat, []int) { a := nat{1} b := []int{2} return a, b } func main() { var u1 []int var n2 nat u1, n2 = x() // .tmp_1, .tmp_2 := x() // u1, n2 = .tmp_1, .tmp_2 println(u1) println(n2) } // Output: // slice[(1 int)] // (slice[(2 int)] main.nat) ``` ### Here is the simplified recursive tree of the transformation in the preprocess <img width="1336" alt="image" src="https://github.com/gnolang/gno/assets/90544084/306a4770-457d-4131-a82a-2de5c6b0dadf"> ### Here are the major steps involved in this decomposition during preprocessing: - Create hidden temporary name expressions .tmp1, .tmp2. In Go, a leading dot is not valid in variable names, ensuring that users cannot create names that clash with these hidden variables. - Create two statements in the block: one for defining and one for assigning. ``` .tmp1, .tmp2 := x() u1, n2 = .tmp_1, .tmp_2 ``` - Preprocess each newly created statements - Replace the original statement with the two newly created statements. ### Here are two additional changes to facilitate above. - Update the FuncValue's body in `updates := pn.PrepareNewValues(pv) `since its source Body has been changed during preprocessing. - Replace all ` for index := range Body` with `for i:=0; i < len(Body); i++` in transcribe.go since the body length might change due to the decomposition. <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [x] No breaking changes were made - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com> Co-authored-by: Morgan <git@howl.moe>
gfanton
pushed a commit
to gfanton/gno
that referenced
this issue
Jul 23, 2024
This is the last part of the solution for issue gnolang#1141. The 1 of 3 of the solution can be found in PR gnolang#1143. The 2 of 3 of the solution can be found in PR gnolang#1246 It decomposes function calls that return multiple values in the preprocess. ### Here is the problem to solve: ` u1, n2 = x() ` How do we ensure that the returned multiple values from a function call adhere to named and unnamed type assignment specifications? Additionally, we want to solve this problem during preprocessing instead of at runtime to minimize the impact on runtime performance. ### The main ideas: u1, n2 = x() << decompose the statement to the following two lines // .tmp_1, .tmp_2 := x() // u1, n2 = .tmp_1, .tmp_2 then we can apply name and unname type conversion specs to the second line. u1, n2 = _tmp_1, _tmp_2 ### Here are the example code and the explanation ``` // decompose_filetest.gno package main type nat []int func x() (nat, []int) { a := nat{1} b := []int{2} return a, b } func main() { var u1 []int var n2 nat u1, n2 = x() // .tmp_1, .tmp_2 := x() // u1, n2 = .tmp_1, .tmp_2 println(u1) println(n2) } // Output: // slice[(1 int)] // (slice[(2 int)] main.nat) ``` ### Here is the simplified recursive tree of the transformation in the preprocess <img width="1336" alt="image" src="https://github.com/gnolang/gno/assets/90544084/306a4770-457d-4131-a82a-2de5c6b0dadf"> ### Here are the major steps involved in this decomposition during preprocessing: - Create hidden temporary name expressions .tmp1, .tmp2. In Go, a leading dot is not valid in variable names, ensuring that users cannot create names that clash with these hidden variables. - Create two statements in the block: one for defining and one for assigning. ``` .tmp1, .tmp2 := x() u1, n2 = .tmp_1, .tmp_2 ``` - Preprocess each newly created statements - Replace the original statement with the two newly created statements. ### Here are two additional changes to facilitate above. - Update the FuncValue's body in `updates := pn.PrepareNewValues(pv) `since its source Body has been changed during preprocessing. - Replace all ` for index := range Body` with `for i:=0; i < len(Body); i++` in transcribe.go since the body length might change due to the decomposition. <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [x] No breaking changes were made - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com> Co-authored-by: Morgan <git@howl.moe>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context:
Thank you @r3v4s for identifying the issue.
#651
Thank you @ltzmaxwell for providing a solution.
#674
Root cause is the incomplete support of the spec of named type, which is previously noted in the preprocess.go
Here is the example and go specification
https://go.dev/play/p/uWkp8HXZkAo
https://go.dev/ref/spec#Assignability
Assignability
https://go.dev/ref/spec#Assignment_statements
Assignment statements
An assignment replaces the current value stored in a variable with a new value specified by an expression.
In assignments, each value must be assignable to the type of the operand to which it is assigned, with the following special cases:
Solution
To complete the named and un-name type assignment specs, within existing preprocessing flow while keeping future expanding capability, there are many things need to be considered
Here is the basic idea:
Even though this value assignment happens at runtime, we want to handle it during preprocessing by converting unnamed types to named type, and vice versa, when the assignment happens. This will save logic execution steps when a MsgCall transaction is executed during runtime.
identify all named and un-named types of gno type and tag them with IsNamed() method as a type property.
Identify when name and unnamed assignment happens
there is a special case of assigning returned results from a func call to more than one variables
This occurs at runtime, but we want to handle the conversion in the preprocessing stage by decomposing it into one 'define' statement, which does not involve conversion, and one assignment statement, which involves conversion as in a normal assignment, like 'a,b = c,d'. This is quite complex, and many more details need to be considered. We will explain the implementation later.
corner cases to verify
convert right hand sides to Const Type Call Expr according to the left hand side type.
add print named type in prinln() so that we can verify all the unnamed and named type in file tests.
Implementation:
The full implementation of entire code changes are located here https://github.com/piux2/gno/pull/2/files for review.
file changes:
gnovm/pkg/gnolang
contracts:
this directory is newly created for easy review and regression test of this feature, we will find proper place to merge tests to main branch
newly added 80 testing cases for this feature.
tests/files:
println() added missing named type to the variables to the output of the file tests.
To make it easier for us to review and and reduce the risk of introducing side effect and hard to find bugs, we break the entire code changes to a few PRs. First is to add named type in println() results. This need to be merged first, otherwise we will not be able to merge rest of code with proper testing cases integrated in CI. #1143
The text was updated successfully, but these errors were encountered: