From 3b954861ad1c15a268304b6179e6e56bc74dae90 Mon Sep 17 00:00:00 2001 From: Petar Dambovaliev Date: Tue, 9 Apr 2024 13:56:28 +0200 Subject: [PATCH] fix: top sort var/const globals (#1854) This PR fixes [this](https://github.com/gnolang/gno/issues/1849) and [this](https://github.com/gnolang/gno/issues/1463). Before preprocessing, a dependency graph is created and edges between the nodes which represent the relationship between global `var` and `const` declarations. Then, a new slice of declarations is created that is topologically sorted. This enables the rest of the preprocessing code to work the way it is now. Small scale refactoring is included by removing unnecessary else statements in `PredefineFileSet`. --- gnovm/pkg/gnolang/nodes.go | 19 +- gnovm/pkg/gnolang/preprocess.go | 35 ++-- gnovm/pkg/gnolang/value_decl_dep_graph.go | 245 ++++++++++++++++++++++ gnovm/tests/files/var18.gno | 14 ++ gnovm/tests/files/var19.gno | 14 ++ gnovm/tests/files/var20.gno | 17 ++ gnovm/tests/files/var21.gno | 17 ++ 7 files changed, 341 insertions(+), 20 deletions(-) create mode 100644 gnovm/pkg/gnolang/value_decl_dep_graph.go create mode 100644 gnovm/tests/files/var18.gno create mode 100644 gnovm/tests/files/var19.gno create mode 100644 gnovm/tests/files/var20.gno create mode 100644 gnovm/tests/files/var21.gno diff --git a/gnovm/pkg/gnolang/nodes.go b/gnovm/pkg/gnolang/nodes.go index 8f2c5054a8a..5d430a8bcde 100644 --- a/gnovm/pkg/gnolang/nodes.go +++ b/gnovm/pkg/gnolang/nodes.go @@ -158,6 +158,23 @@ type Attributes struct { data map[interface{}]interface{} // not persisted } +func (attr *Attributes) Copy() Attributes { + if attr == nil { + return Attributes{} + } + + data := make(map[interface{}]interface{}) + for k, v := range attr.data { + data[k] = v + } + + return Attributes{ + Line: attr.Line, + Label: attr.Label, + data: data, + } +} + func (attr *Attributes) GetLine() int { return attr.Line } @@ -1614,7 +1631,7 @@ func (sb *StaticBlock) GetPathForName(store Store, n Name) ValuePath { bp = bp.GetParentNode(store) gen++ if 0xff < gen { - panic("value path depth overflow") + panic("GetPathForName: value path depth overflow") } } } diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go index f98ed899cfc..fba01750a20 100644 --- a/gnovm/pkg/gnolang/preprocess.go +++ b/gnovm/pkg/gnolang/preprocess.go @@ -13,11 +13,21 @@ import ( // Anything predefined or preprocessed here get skipped during the Preprocess // phase. func PredefineFileSet(store Store, pn *PackageNode, fset *FileSet) { + for _, fn := range fset.Files { + decls, err := sortValueDeps(fn.Decls) + if err != nil { + panic(err) + } + + fn.Decls = decls + } + // First, initialize all file nodes and connect to package node. for _, fn := range fset.Files { SetNodeLocations(pn.PkgPath, string(fn.Name), fn) fn.InitStaticBlock(fn, pn) } + // NOTE: much of what follows is duplicated for a single *FileNode // in the main Preprocess translation function. Keep synced. @@ -29,11 +39,7 @@ func PredefineFileSet(store Store, pn *PackageNode, fset *FileSet) { d := fn.Decls[i] switch d.(type) { case *ImportDecl: - if d.GetAttribute(ATTR_PREDEFINED) == true { - // skip declarations already predefined - // (e.g. through recursion for a - // dependent) - } else { + if d.GetAttribute(ATTR_PREDEFINED) != true { // recursively predefine dependencies. d2, _ := predefineNow(store, fn, d) fn.Decls[i] = d2 @@ -41,17 +47,14 @@ func PredefineFileSet(store Store, pn *PackageNode, fset *FileSet) { } } } + // Predefine all type decls decls. for _, fn := range fset.Files { for i := 0; i < len(fn.Decls); i++ { d := fn.Decls[i] switch d.(type) { case *TypeDecl: - if d.GetAttribute(ATTR_PREDEFINED) == true { - // skip declarations already predefined - // (e.g. through recursion for a - // dependent) - } else { + if d.GetAttribute(ATTR_PREDEFINED) != true { // recursively predefine dependencies. d2, _ := predefineNow(store, fn, d) fn.Decls[i] = d2 @@ -65,11 +68,7 @@ func PredefineFileSet(store Store, pn *PackageNode, fset *FileSet) { d := fn.Decls[i] switch d.(type) { case *FuncDecl: - if d.GetAttribute(ATTR_PREDEFINED) == true { - // skip declarations already predefined - // (e.g. through recursion for a - // dependent) - } else { + if d.GetAttribute(ATTR_PREDEFINED) != true { // recursively predefine dependencies. d2, _ := predefineNow(store, fn, d) fn.Decls[i] = d2 @@ -77,15 +76,13 @@ func PredefineFileSet(store Store, pn *PackageNode, fset *FileSet) { } } } + // Finally, predefine other decls and // preprocess ValueDecls.. for _, fn := range fset.Files { for i := 0; i < len(fn.Decls); i++ { d := fn.Decls[i] - if d.GetAttribute(ATTR_PREDEFINED) == true { - // skip declarations already predefined (e.g. - // through recursion for a dependent) - } else { + if d.GetAttribute(ATTR_PREDEFINED) != true { // recursively predefine dependencies. d2, _ := predefineNow(store, fn, d) fn.Decls[i] = d2 diff --git a/gnovm/pkg/gnolang/value_decl_dep_graph.go b/gnovm/pkg/gnolang/value_decl_dep_graph.go new file mode 100644 index 00000000000..fe130dbf43a --- /dev/null +++ b/gnovm/pkg/gnolang/value_decl_dep_graph.go @@ -0,0 +1,245 @@ +package gnolang + +import ( + "fmt" + "slices" + "strings" +) + +// sortValueDeps creates a new topologically sorted +// decl slice ready for processing in order +func sortValueDeps(decls Decls) (Decls, error) { + graph := &graph{ + edges: make(map[string][]string), + vertices: make([]string, 0), + } + + otherDecls := make(Decls, 0) + + for i := 0; i < len(decls); i++ { + d := decls[i] + vd, ok := d.(*ValueDecl) + + if !ok { + otherDecls = append(otherDecls, d) + continue + } + + if isTuple(vd) { + _, ok := vd.Values[0].(*CallExpr) + if ok { + graph.addVertex(vd.NameExprs.String()) + continue + } + } + + for j := 0; j < len(vd.NameExprs); j++ { + graph.addVertex(string(vd.NameExprs[j].Name)) + } + } + + for i := 0; i < len(decls); i++ { + d := decls[i] + vd, ok := d.(*ValueDecl) + + if !ok { + continue + } + + if isTuple(vd) { + ce, ok := vd.Values[0].(*CallExpr) + if ok { + addDepFromExpr(graph, vd.NameExprs.String(), ce) + continue + } + } + + for j := 0; j < len(vd.NameExprs); j++ { + if len(vd.Values) > j { + addDepFromExpr(graph, string(vd.NameExprs[j].Name), vd.Values[j]) + } + } + } + + sorted := make(Decls, 0) + + for _, node := range graph.topologicalSort() { + var dd Decl + + for _, decl := range decls { + vd, ok := decl.(*ValueDecl) + + if !ok { + continue + } + + if isCompoundNode(node) { + dd = processCompound(node, vd, decl) + break + } + + for i, nameExpr := range vd.NameExprs { + if string(nameExpr.Name) == node { + if len(vd.Values) > i { + dd = &ValueDecl{ + Attributes: vd.Attributes.Copy(), + NameExprs: []NameExpr{nameExpr}, + Type: vd.Type, + Values: []Expr{vd.Values[i]}, + Const: vd.Const, + } + break + } else { + dd = vd + break + } + } + } + } + + if dd == nil { + continue + } + + sorted = append(sorted, dd) + } + + slices.Reverse(sorted) + + otherDecls = append(otherDecls, sorted...) + + return otherDecls, nil +} + +func addDepFromExpr(dg *graph, fromNode string, expr Expr) { + switch e := expr.(type) { + case *FuncLitExpr: + for _, stmt := range e.Body { + addDepFromExprStmt(dg, fromNode, stmt) + } + case *CallExpr: + addDepFromExpr(dg, fromNode, e.Func) + + for _, arg := range e.Args { + addDepFromExpr(dg, fromNode, arg) + } + case *NameExpr: + if isUverseName(e.Name) { + break + } + + toNode := string(e.Name) + dg.addEdge(fromNode, toNode) + } +} + +func addDepFromExprStmt(dg *graph, fromNode string, stmt Stmt) { + switch e := stmt.(type) { + case *ExprStmt: + addDepFromExpr(dg, fromNode, e.X) + case *IfStmt: + addDepFromExprStmt(dg, fromNode, e.Init) + addDepFromExpr(dg, fromNode, e.Cond) + + for _, stm := range e.Then.Body { + addDepFromExprStmt(dg, fromNode, stm) + } + for _, stm := range e.Else.Body { + addDepFromExprStmt(dg, fromNode, stm) + } + case *ReturnStmt: + for _, stm := range e.Results { + addDepFromExpr(dg, fromNode, stm) + } + case *AssignStmt: + for _, stm := range e.Rhs { + addDepFromExpr(dg, fromNode, stm) + } + case *SwitchStmt: + addDepFromExpr(dg, fromNode, e.X) + for _, clause := range e.Clauses { + addDepFromExpr(dg, fromNode, clause.bodyStmt.Cond) + for _, s := range clause.bodyStmt.Body { + addDepFromExprStmt(dg, fromNode, s) + } + } + case *ForStmt: + addDepFromExpr(dg, fromNode, e.Cond) + for _, s := range e.bodyStmt.Body { + addDepFromExprStmt(dg, fromNode, s) + } + case *BlockStmt: + for _, s := range e.Block.bodyStmt.Body { + addDepFromExprStmt(dg, fromNode, s) + } + } +} + +type graph struct { + edges map[string][]string + vertices []string +} + +func (g *graph) addEdge(u, v string) { + g.edges[u] = append(g.edges[u], v) +} + +func (g *graph) addVertex(v string) { + g.vertices = append(g.vertices, v) +} + +func (g *graph) topologicalSortUtil(v string, visited map[string]bool, stack *[]string) { + visited[v] = true + + for _, u := range g.edges[v] { + if !visited[u] { + g.topologicalSortUtil(u, visited, stack) + } + } + + *stack = append([]string{v}, *stack...) +} + +func (g *graph) topologicalSort() []string { + stack := make([]string, 0) + visited := make(map[string]bool) + + for _, v := range g.vertices { + if !visited[v] { + g.topologicalSortUtil(v, visited, &stack) + } + } + + return stack +} + +func isTuple(vd *ValueDecl) bool { + return len(vd.NameExprs) > len(vd.Values) && len(vd.Values) > 0 +} + +func isCompoundNode(node string) bool { + return strings.Contains(node, ", ") +} + +func processCompound(node string, vd *ValueDecl, decl Decl) Decl { + names := strings.Split(node, ", ") + + if len(names) != len(vd.NameExprs) { + panic("should not happen") + } + + equal := true + + for i, name := range names { + if vd.NameExprs[i].String() != name { + equal = false + break + } + } + + if !equal { + panic(fmt.Sprintf("names: %+v != nameExprs: %+v\n", names, vd.NameExprs)) + } + + return decl +} diff --git a/gnovm/tests/files/var18.gno b/gnovm/tests/files/var18.gno new file mode 100644 index 00000000000..619aedef779 --- /dev/null +++ b/gnovm/tests/files/var18.gno @@ -0,0 +1,14 @@ +package main + +func main() { + println(a) + println(b) +} + +func r2() (int, int) { return 1, 2 } + +var a, b int = r2() + +// Output: +// 1 +// 2 diff --git a/gnovm/tests/files/var19.gno b/gnovm/tests/files/var19.gno new file mode 100644 index 00000000000..f26c230de0d --- /dev/null +++ b/gnovm/tests/files/var19.gno @@ -0,0 +1,14 @@ +package main + +func main() { + println(a) + println(b) + println(c) +} + +var a, b, c = 1, a + 1, b + 1 + +// Output: +// 1 +// 2 +// 3 diff --git a/gnovm/tests/files/var20.gno b/gnovm/tests/files/var20.gno new file mode 100644 index 00000000000..1aec4792fcb --- /dev/null +++ b/gnovm/tests/files/var20.gno @@ -0,0 +1,17 @@ +package main + +func main() { + println(a) + println(b) + println(c) + println(d) +} + +var a, b, c = 1, a + d, 3 +var d = a + +// Output: +// 1 +// 2 +// 3 +// 1 diff --git a/gnovm/tests/files/var21.gno b/gnovm/tests/files/var21.gno new file mode 100644 index 00000000000..7ec32e1ebfa --- /dev/null +++ b/gnovm/tests/files/var21.gno @@ -0,0 +1,17 @@ +package main + +func main() { + myDep = "123" + myVar() +} + +func hello(s string) { + println(s) +} + +var myVar = func() { hello(myDep) } + +var myDep string + +// Output: +// 123