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

fix: print declared type in output #1143

Merged
merged 5 commits into from
Sep 22, 2023
Merged

Conversation

piux2
Copy link
Contributor

@piux2 piux2 commented Sep 18, 2023

This the first PR required for this feature #1141

@piux2 piux2 requested review from jaekwon, moul and a team as code owners September 18, 2023 01:40
@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Sep 18, 2023
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (cfefb3b) 46.97% compared to head (e1aef0a) 46.98%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1143   +/-   ##
=======================================
  Coverage   46.97%   46.98%           
=======================================
  Files         365      365           
  Lines       61156    61159    +3     
=======================================
+ Hits        28729    28735    +6     
+ Misses      30066    30065    -1     
+ Partials     2361     2359    -2     
Flag Coverage Δ
gno.land-_test.gnokey 0.00% <ø> (ø)
gno.land-_test.gnoland 88.14% <ø> (ø)
gno.land-_test.pkgs 27.88% <ø> (ø)
gnovm-_test.cmd 45.89% <ø> (ø)
gnovm-_test.gnolang.native 63.09% <ø> (ø)
gnovm-_test.gnolang.other 16.63% <ø> (ø)
gnovm-_test.gnolang.pkg0 17.98% <ø> (ø)
gnovm-_test.gnolang.pkg1 8.21% <ø> (ø)
gnovm-_test.gnolang.pkg2 9.87% <ø> (ø)
gnovm-_test.gnolang.realm 41.68% <ø> (ø)
gnovm-_test.gnolang.stdlibs 53.53% <ø> (ø)
gnovm-_test.pkg 25.95% <0.00%> (-0.01%) ⬇️
tm2-_test.flappy ∅ <ø> (∅)
tm2-_test.pkg.amino 58.32% <ø> (ø)
tm2-_test.pkg.bft 63.46% <ø> (+0.02%) ⬆️
tm2-_test.pkg.others 59.22% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
gnovm/pkg/gnolang/values_string.go 10.62% <0.00%> (-0.13%) ⬇️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@piux2 piux2 changed the title print declared type in output fix: print declared type in output Sep 18, 2023
@moul moul added this to the 🚀 main.gno.land (required) milestone Sep 18, 2023
@moul moul merged commit e10c0c7 into gnolang:master Sep 22, 2023
179 checks passed
gfanton pushed a commit to gfanton/gno that referenced this pull request 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>
@piux2 piux2 mentioned this pull request Apr 15, 2024
thehowl added a commit that referenced this pull request 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 pull request 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 pull request 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>
piux2 added a commit that referenced this pull request 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 pull request 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
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants