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(transpiler): transpile gno standard libraries #1695

Merged
merged 105 commits into from
Jun 19, 2024

Conversation

thehowl
Copy link
Member

@thehowl thehowl commented Feb 26, 2024

Merge order:

  1. feat(stdlibs): remove support for linkedType in native bindings #1700
  2. feat(gno.land): add go type checking to keeper + tx simulation in gnokey #1702
  3. feat(transpiler): transpile gno standard libraries #1695 (this one!) -- review earlier ones first, if they're still open!

This PR modifies the Gno transpiler (fka precompiler) to use Gno's standard libraries rather than Go's when performing transpilation. This creates the necessity to transpile Gno standard libraries, and as such support their native bindings. And it removes the necessity for a package like stdshim, and a mechanism like stdlibWhitelist.

cc/ @leohhhn @tbruyelle, as this relates to your work

Why?

  • This PR enables us to perform Go type-checking across the board, and not use Go's standard libraries in transpiled code. This enables us to properly support our own standard libraries, such as std but any others we might want or need.
  • It also paves the way further to go full circle, and have Gno code be transpiled to Go, and then have "compilable" gno code

Summary of changes

  • The transpiler has been thoroughly refactored.
    • The biggest change is described above: instead of maintaing the import paths like "strconv" and "math" the same (so using Gno's stdlibs in Gno, and Go's in Go), the import paths for standard libraries is now also updated to point to the Gno standard libraries.
    • Native functions are handled by removing their definitions when transpiling, and changing their call expressions where appropriate. This links the transpiled code directly to their native counterparts.
    • This removes the necessity for stdlibWhitelist.
    • As a consequence, stdshim is no longer needed and has been removed.
    • Test files are still not "strictly checked": they may reference stdlibs with no matching source, and will not be tested when running with --gobuild. This is because packages like fmt have no representation in Gno code; they only exist as injections in tests/imports.go. I'll fix this eventually :)
  • The CLI (gno transpile) has been changed to reflect the above changes.
    • Flag --skip-fmt has been removed (the result of transpile is always formatted, anyway), and --gofmt-binary too, obviously. gno transpile does not perform validation, but will gladly provide helpful validation with the --gobuild flag.
    • There is another PR that adds type checking in gno lint, without needing to run through the transpilation step first: feat(cmd/gno): perform type checking when calling linter #1730
    • It now works by default by looking at "packages" rather than individual files. This is necessary so that when performing transpile on the examples directory, we can skip those where the gno.mod marks the module as draft. These modules make use of packages like "fmt", which because they don't have an underlying gno/go source, cannot be transpiled.
    • Running with -gobuild now handles more errors correctly; ie., all errors not previously captured by the errorRe which only matches those pertaining to a specific file/line.
    • gnoFilesFromArgs was unused and as such deleted
  • gnomod's behaviour was slightly changed.
    • I am of the opinion that gno mod download should not precompile what it downloads; especially to gather the dependencies it has. I've changed it so that it does a OnlyImports parse of the file it downloads to fetch additional dependencies

Misc:

  • Makefile now contains a recipe to calculate the coverage for gnovm/cmd/gno, and also view it via the HTML interface. This is needed as it has a few extra steps (which @gfanton already previously added in the CI).
  • Realms r/demo/art/gnoface and r/x/manfred_outfmt have been marked as draft, as they depend on packages which are not actually present in the Gno standard libraries.
    • The transpiler now ignores draft packages by default.
  • ReadMemPackage now also considers Go files. This is meant to have on-chain the code for standard libraries like std which have native bindings. We still exclude Go code if it's not in a standard library.
  • //go:build constraints have been removed from standard libraries, as go files can only have one and we already add our own when transpiling

Further improvements

after this PR

  • Scope understanding in transpiler (so call expressions are not incorrectly rewritten)
  • Correctly transpile gno.mod

@thehowl thehowl self-assigned this Feb 26, 2024
@github-actions github-actions bot added 📦 🤖 gnovm Issues or PRs gnovm related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Feb 26, 2024
@thehowl thehowl mentioned this pull request Feb 27, 2024
8 tasks
@thehowl thehowl changed the title feat(transpiler): transpile gno standard libraries feat(transpiler): transpile gno standard libraries, add go type checking Feb 27, 2024
@thehowl thehowl changed the title feat(transpiler): transpile gno standard libraries, add go type checking feat: transpile gno standard libraries, add go type checking to keeper Feb 27, 2024
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Feb 27, 2024
@thehowl thehowl force-pushed the dev/morgan/precompile-refactor-2 branch from f6cdc11 to d778788 Compare February 27, 2024 21:29
@zivkovicmilos
Copy link
Member

@thehowl can you merge in master?

@thehowl
Copy link
Member Author

thehowl commented May 29, 2024

@zivkovicmilos done

Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some nits, apart from them, LGTM.

examples/gno.land/r/demo/art/gnoface/gno.mod Show resolved Hide resolved
examples/gno.land/r/x/manfred_outfmt/gno.mod Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/stdlibs/stdlibs.go Outdated Show resolved Hide resolved
gnovm/stdlibs/stdlibs.go Outdated Show resolved Hide resolved
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good 💯

I've left some comments and nits. We're good to merge after resolving 🚀

gnovm/cmd/gno/transpile.go Show resolved Hide resolved
gnovm/stdlibs/stdlibs.go Outdated Show resolved Hide resolved
gnovm/stdlibs/stdlibs.go Show resolved Hide resolved
gnovm/tests/stdlibs/native.go Show resolved Hide resolved
gnovm/cmd/gno/util.go Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/transpile.go Show resolved Hide resolved
@thehowl thehowl merged commit c4664ed into master Jun 19, 2024
98 checks passed
@thehowl thehowl deleted the dev/morgan/precompile-refactor-2 branch June 19, 2024 15:38
@thehowl thehowl mentioned this pull request Jun 20, 2024
7 tasks
leohhhn pushed a commit that referenced this pull request Jun 20, 2024
Somehow it slipped #1695

<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
- [x] 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
- [x] Provided any useful hints for running manual tests
- [x] 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>
@thehowl thehowl mentioned this pull request Jun 24, 2024
7 tasks
gfanton pushed a commit to gfanton/gno that referenced this pull request Jul 23, 2024
Merge order:

1. gnolang#1700 
2. gnolang#1702
3. gnolang#1695 (this one!) -- review earlier ones first, if they're still
open!

This PR modifies the Gno transpiler (fka precompiler) to use Gno's
standard libraries rather than Go's when performing transpilation. This
creates the necessity to transpile Gno standard libraries, and as such
support their native bindings. And it removes the necessity for a
package like `stdshim`, and a mechanism like `stdlibWhitelist`.

- Fixes gnolang#668. Fixes gnolang#1865.
- Resolves gnolang#892.
- Part of gnolang#814. 
- Makes gnolang#1475 / gnolang#1576 possible without using hacks like `stdshim`.

cc/ @leohhhn @tbruyelle, as this relates to your work

## Why?

- This PR enables us to perform Go type-checking across the board, and
not use Go's standard libraries in transpiled code. This enables us to
_properly support our own standard libraries_, such as `std` but any
others we might want or need.
- It also paves the way further to go full circle, and have Gno code be
transpiled to Go, and then have "compilable" gno code

## Summary of changes

- The transpiler has been thoroughly refactored.
- The biggest change is described above: instead of maintaing the import
paths like `"strconv"` and `"math"` the same (so using Gno's stdlibs in
Gno, and Go's in Go), the import paths for standard libraries is now
also updated to point to the Gno standard libraries.
- Native functions are handled by removing their definitions when
transpiling, and changing their call expressions where appropriate. This
links the transpiled code directly to their native counterparts.
  - This removes the necessity for `stdlibWhitelist`. 
- As a consequence, `stdshim` is no longer needed and has been removed.
- Test files are still not "strictly checked": they may reference
stdlibs with no matching source, and will not be tested when running
with `--gobuild`. This is because packages like `fmt` have no
representation in Gno code; they only exist as injections in
`tests/imports.go`. I'll fix this eventually :)
- The CLI (`gno transpile`) has been changed to reflect the above
changes.
- Flag `--skip-fmt` has been removed (the result of transpile is always
formatted, anyway), and `--gofmt-binary` too, obviously. `gno transpile`
does not perform validation, but will gladly provide helpful validation
with the `--gobuild` flag.
- There is another PR that adds type checking in `gno lint`, without
needing to run through the transpilation step first:
gnolang#1730
- It now works by default by looking at "packages" rather than
individual files. This is necessary so that when performing `transpile`
on the `examples` directory, we can skip those where the gno.mod marks
the module as draft. These modules make use of packages like "fmt",
which because they don't have an underlying gno/go source, cannot be
transpiled.
- Running with `-gobuild` now handles more errors correctly; ie., all
errors not previously captured by the `errorRe` which only matches those
pertaining to a specific file/line.
  - `gnoFilesFromArgs` was unused and as such deleted
- `gnomod`'s behaviour was slightly changed.
- I am of the opinion that `gno mod download` should not precompile what
it downloads; _especially_ to gather the dependencies it has. I've
changed it so that it does a `OnlyImports` parse of the file it
downloads to fetch additional dependencies

Misc:

- `Makefile` now contains a recipe to calculate the coverage for
`gnovm/cmd/gno`, and also view it via the HTML interface. This is needed
as it has a few extra steps (which @gfanton already previously added in
the CI).
- Realms r/demo/art/gnoface and r/x/manfred_outfmt have been marked as
draft, as they depend on packages which are not actually present in the
Gno standard libraries.
  - The transpiler now ignores draft packages by default.
- `ReadMemPackage` now also considers Go files. This is meant to have
on-chain the code for standard libraries like `std` which have native
bindings. We still exclude Go code if it's not in a standard library.
- `//go:build` constraints have been removed from standard libraries, as
go files can only have one and we already add our own when transpiling

## Further improvements

after this PR

- Scope understanding in `transpiler` (so call expressions are not
incorrectly rewritten)
- Correctly transpile gno.mod

---------

Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com>
gfanton pushed a commit to gfanton/gno that referenced this pull request Jul 23, 2024
Somehow it slipped gnolang#1695

<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
- [x] 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
- [x] Provided any useful hints for running manual tests
- [x] 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>
@@ -68,6 +68,8 @@ func SetupGno(p *testscript.Params, buildDir string) error {
return fmt.Errorf("unable to create temporary home directory: %w", err)
}
env.Setenv("HOME", home)
// Avoids go command printing errors relating to lack of go.mod.
env.Setenv("GO111MODULE", "off")
Copy link
Contributor

@tbruyelle tbruyelle Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thehowl Sorry for coming after the merge, but do you consider setting this env var directly when go transpile -gobuild is invoked ?
Because else the error relating to lack of go.mod will pop up each time this command is invoked with a directory argument (which is a quite common situation when using gnols).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an idea. I guess more in general the thing is that we shouldn't use go build; because even if GO111MODULE=off, then it will try to use go's old depedency system.

This could eventually be fixed by #1730, which makes it so we don't need go build to get typecheck errors. I haven't managed to keep up with that one much, dealing with other priorities, alas. But I want to try to maybe wrap it up this week, before I go on holiday.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it would be even better without using go build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: Done
Status: Done
4 participants