search

gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + Gno.land: a blockchain for timeless code and fair open-source
https://gno.land/
Other
839 stars 342 forks source link

fix(gnovm): source location always precise with line and column #2362

Open mvertes opened 1 week ago

mvertes commented 1 week ago

The column number was not stored in the GnoVM location. A Nonce field was used to distinguished 2 block nodes on the same line. This was applied only to block nodes, not expressions or statements.

As a result, the node location was the same for all expressions and statements located at the same source line. This change ensures the uniqueness of location for all nodes, in addition to providing an accurate position in source code.

We replace Nonce by Column, computed by the Go tokenizer (the same as line numbers). Column is always included in all AST nodes, and the location string has a uniform format.

With this change it is now possible for the debugger to step in infinite loops on one line like:

for { i++; println(i) }

This change is also necessary for a correct accounting for code coverage, and also may be gas accounting.

Note: as Nonce is renamed to Column in protocol definitions (type and index unchanged), it may qualify as a BREAKING CHANGE, but I'm not completely sure of that.

Contributors' checklist... - [*] 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 - [ ] 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).
codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.

Project coverage is 54.61%. Comparing base (6ec4bb8) to head (83b7751). Report is 2 commits behind head on master.

Files Patch % Lines
gnovm/pkg/gnolang/machine.go 0.00% 4 Missing :warning:
gnovm/pkg/gnolang/preprocess.go 66.66% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2362 +/- ## ========================================== - Coverage 54.61% 54.61% -0.01% ========================================== Files 581 581 Lines 77963 78018 +55 ========================================== + Hits 42578 42608 +30 - Misses 32209 32230 +21 - Partials 3176 3180 +4 ``` | [Flag](https://app.codecov.io/gh/gnolang/gno/pull/2362/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gnolang) | Coverage Δ | | |---|---|---| | [gnovm](https://app.codecov.io/gh/gnolang/gno/pull/2362/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gnolang) | `60.05% <83.78%> (+0.05%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gnolang#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

thehowl commented 6 days ago

please merge and fix conflicts :pray:

cc @deelawn for a second review