Skip to content

Commit

Permalink
interp: expose fset to fix CompileAST issue
Browse files Browse the repository at this point in the history
The interpreter has its own internal fileset and expects all code to have been parsed using that fileset. If a user creates a fileset, calls `go/parser.Parse*`, then passes the result to `interp.CompileAST`, strange things can happen.

The solutions I can see are:

1. Expose the fileset so the user can use it when parsing source.
2. Add the fileset as an option (input to New) so that the user can tell the interpreter to use a specific fileset.
3. Pass the fileset into `CompileAST`

There are two ways to implement option 3. One is to add a field to nodes and update every reference to `interp.fset` to use `node.fset`. The other is to add a parameter to every function that references `interp.fset` or calls a function that does. Both of these are significant changes and involve an extra pointer for every node or most function calls.

Options 1 and 2 are easy. Option 2 involves adding an option so I went with option 1. I can imagine situations where option 2 could be necessary, but I can open another issue/PR if and when I need that.

Fixes #1383
  • Loading branch information
firelizzard18 committed Apr 22, 2022
1 parent 5665c9a commit 7be17d3
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
4 changes: 2 additions & 2 deletions interp/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import (
)

func TestCompileAST(t *testing.T) {
file, err := parser.ParseFile(token.NewFileSet(), "_.go", `
i := New(Options{})
file, err := parser.ParseFile(i.FileSet(), "_.go", `
package main
import "fmt"
Expand Down Expand Up @@ -61,7 +62,6 @@ func TestCompileAST(t *testing.T) {
{desc: "expr", node: dFunc.Body.List[0]},
}

i := New(Options{})
_ = i.Use(stdlib.Symbols)

for _, c := range cases {
Expand Down
34 changes: 34 additions & 0 deletions interp/interp_eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"context"
"fmt"
"go/parser"
"io"
"log"
"net/http"
Expand Down Expand Up @@ -1749,3 +1750,36 @@ func TestRestrictedEnv(t *testing.T) {
t.Fatal("expected \"\", got " + s)
}
}

func TestIssue1383(t *testing.T) {
const src = `
package main
func main() {
fmt.Println("Hello")
}
`

interp := interp.New(interp.Options{})
err := interp.Use(stdlib.Symbols)
if err != nil {
t.Fatal(err)
}
_, err = interp.Eval(`import "fmt"`)
if err != nil {
t.Fatal(err)
}

ast, err := parser.ParseFile(interp.FileSet(), "_.go", src, parser.DeclarationErrors)
if err != nil {
t.Fatal(err)
}
prog, err := interp.CompileAST(ast)
if err != nil {
t.Fatal(err)
}
_, err = interp.Execute(prog)
if err != nil {
t.Fatal(err)
}
}
10 changes: 10 additions & 0 deletions interp/program.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package interp
import (
"context"
"go/ast"
"go/token"
"os"
"reflect"
"runtime"
Expand All @@ -16,6 +17,12 @@ type Program struct {
init []*node
}

// FileSet is the fileset that must be used for parsing Go that will be passed
// to interp.CompileAST().
func (interp *Interpreter) FileSet() *token.FileSet {
return interp.fset
}

// Compile parses and compiles a Go code represented as a string.
func (interp *Interpreter) Compile(src string) (*Program, error) {
return interp.compileSrc(src, "", true)
Expand Down Expand Up @@ -55,6 +62,9 @@ func (interp *Interpreter) compileSrc(src, name string, inc bool) (*Program, err
// CompileAST builds a Program for the given Go code AST. Files and block
// statements can be compiled, as can most expressions. Var declaration nodes
// cannot be compiled.
//
// WARNING: The node must have been parsed using interp.FileSet(). Results are
// unpredictable otherwise.
func (interp *Interpreter) CompileAST(n ast.Node) (*Program, error) {
// Convert AST.
pkgName, root, err := interp.ast(n)
Expand Down

0 comments on commit 7be17d3

Please sign in to comment.