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

feat(precompile): use //line directive in generated go files #1688

Merged

Conversation

tbruyelle
Copy link
Contributor

PR #1670 updated the gno precompile output by translating errors reported by generated go files into their corresponding gno files. This was done by shifting the line number, since generated go files have an extra header of 4 lines.

@thehowl found a better way to do this by using the //line directive. By adding this directive at the end of this extra header we have in generated go files, the go compiler will skip it and output a file position that exactly matches the gno file position. This eliminates the need for the line shifting mentioned above.

See https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives

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, if any. More info here.

PR gnolang#1670 updated the `gno precompile` output by translating errors reported
by generated go files into their corresponding gno files. This was done
by shifting the line number, since generated go files have an extra header
of 4 lines.

@thehowl found a better way to do this by using the //line directive. By
adding this directive at the end of this extra header we have in generated
go files, the go compiler will skip it and output a file position that
exactly matches the gno file position. This eliminates the need for the
line shifting mentioned above.

See https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives
@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Feb 23, 2024
Copy link

codecov bot commented Feb 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.48%. Comparing base (6f4efc6) to head (a7b84e0).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1688   +/-   ##
=======================================
  Coverage   47.48%   47.48%           
=======================================
  Files         385      385           
  Lines       61408    61406    -2     
=======================================
  Hits        29159    29159           
+ Misses      29812    29810    -2     
  Partials     2437     2437           

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

gnovm/pkg/gnolang/precompile.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/precompile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/testdata/gno_precompile/06_build_flag.txtar Outdated Show resolved Hide resolved
So we don't even have to remove the .gen.go extension when we parse the
go build errors.
@thehowl thehowl changed the title refactor: use //line directive in generated go files feat(precompile): use //line directive in generated go files Feb 28, 2024
@thehowl thehowl merged commit f8e67c1 into gnolang:master Feb 28, 2024
189 of 190 checks passed
leohhhn pushed a commit to leohhhn/gno that referenced this pull request Feb 29, 2024
…ng#1688)

PR gnolang#1670 updated the `gno precompile` output by translating errors
reported by generated go files into their corresponding gno files. This
was done by shifting the line number, since generated go files have an
extra header of 4 lines.

@thehowl found a better way to do this by using the //line directive. By
adding this directive at the end of this extra header we have in
generated go files, the go compiler will skip it and output a file
position that exactly matches the gno file position. This eliminates the
need for the line shifting mentioned above.

See https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives

<!-- 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
- [x] 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>
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
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants