Skip to content

feat(gnovm): Rewrite AST prior go type checking to avoid inter-realm type check errors. #4156

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 20, 2025
+48 −4
Merged

feat(gnovm): Rewrite AST prior go type checking to avoid inter-realm type check errors. #4156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b915914
Rewrite AST prior go type checking to avoid inter-realm statements.
mvertes Apr 15, 2025
fdcc3f9
s/switchrealm/crossing/, s/witchswitch/cross/
mvertes Apr 15, 2025
3b5ac5b
add syntax checking
mvertes Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions gnovm/pkg/gnolang/gotypecheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"bytes"
"errors"
"go/ast"
"go/format"
"go/parser"
Expand All @@ -13,6 +14,7 @@

"github.com/gnolang/gno/gnovm"
"go.uber.org/multierr"
"golang.org/x/tools/go/ast/astutil"
)

// type checking (using go/types)
Expand Down Expand Up @@ -140,6 +142,11 @@
deleteOldIdents(delFunc, f)
}

if err := filterCrossing(f); err != nil {
errs = multierr.Append(errs, err)
continue
}

// enforce formatting
if fmt {
var buf bytes.Buffer
Expand Down Expand Up @@ -179,3 +186,34 @@
}
}
}

func filterCrossing(f *ast.File) (err error) {
astutil.Apply(f, nil, func(c *astutil.Cursor) bool {
switch n := c.Node().(type) {
case *ast.ExprStmt:
if ce, ok := n.X.(*ast.CallExpr); ok {
if id, ok := ce.Fun.(*ast.Ident); ok && id.Name == "crossing" {

Check failure on line 195 in gnovm/pkg/gnolang/gotypecheck.go

View workflow job for this annotation

GitHub Actions / Run GnoVM suite / Go Lint / lint 

string `crossing` has 3 occurrences, make it a constant (goconst)

Check failure on line 195 in gnovm/pkg/gnolang/gotypecheck.go

View workflow job for this annotation

GitHub Actions / Run GnoVM suite / Go Lint / lint 

string `crossing` has 3 occurrences, make it a constant (goconst)
// Validate syntax.
if len(ce.Args) != 0 {
err = errors.New("crossing called with non empty parameters")
}
// Delete statement 'crossing()'.
c.Delete()
}
}
case *ast.CallExpr:
if id, ok := n.Fun.(*ast.Ident); ok && id.Name == "cross" {

Check failure on line 205 in gnovm/pkg/gnolang/gotypecheck.go

View workflow job for this annotation

GitHub Actions / Run GnoVM suite / Go Lint / lint 

string `cross` has 3 occurrences, make it a constant (goconst)

Check failure on line 205 in gnovm/pkg/gnolang/gotypecheck.go

View workflow job for this annotation

GitHub Actions / Run GnoVM suite / Go Lint / lint 

string `cross` has 3 occurrences, make it a constant (goconst)
// Replace expression 'cross(x)' by 'x'.
var a ast.Node
if len(n.Args) == 1 {
a = n.Args[0]
} else {
err = errors.New("cross called with invalid parameters")
}
c.Replace(a)
}
}
return true
})
return err
}
6 changes: 6 additions & 0 deletions gnovm/pkg/test/filetest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ func (opts *TestOptions) runTest(m *gno.Machine, pkgPath, filename string, conte
}
orig, tx := m.Store, m.Store.BeginTransaction(nil, nil, nil)
m.Store = tx

// Validate Gno syntax and type check.
if err := gno.TypeCheckMemPackageTest(memPkg, m.Store); err != nil {
return runResult{Error: err.Error()}
}

Comment on lines +265 to +270
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding, this is a signal that we’re no longer adding new features to the gno type checker, right?
Do we already have a clear clarification on this plan?

I think what we need to do is review the existing issues related to the gno type checker—either mark them appropriately or close them.
There are also some related PRs, such as #4033. Should we go ahead and merge them? @thehowl @mvertes

Copy link
Member

@thehowl thehowl Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

From what jae said, we should go through the PRs and issues relating to the type-checker and label them. I think we can close them for now, then recover them if we want to go ahead and improve our own preprocessor / type checker.

Longer term, I hope that we can migrate away from the GnoVM sooner than we consider removing our reliance on the go type checlker.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's going to take a long time to remove the go type checker. At least we should keep it around for fuzz testing against our own, with fuzz tools that can generate any valid code to run against both go and gno.

// Run decls and init functions.
m.RunMemPackage(memPkg, true)
// Clear store cache and reconstruct machine from committed info
Expand Down
8 changes: 4 additions & 4 deletions gnovm/tests/files/extern/timtadh/data_structures/types/string.gno
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ func (self ByteSlice) Hash() int {

func hash(s []byte) int {
res := sha256.Sum256(s)
return int(s[0]) |
int(s[1]<<8) |
int(s[2]<<16) |
int(s[3]<<24)
return int(res[0]) |
int(res[1]<<8) |
int(res[2]<<16) |
int(res[3]<<24)
}
Loading