[llgo] llgoi: separate evaluation from printing

[axw]

Summary

Separate the evaluation of expressions from printing
of results. This is in preparation for splitting the
core of the interpreter out for use in alternative
interpreter frontends.

At the same time, the output is made less noisy in
response to comments on the golang-nuts announcement.
We would ideally print out values using Go syntax,
but this is impractical until we have libgo based on
Go 1.5. When that happens, fmt's %#v will handle
reflect.Value better, and so we can fix/filter type
names to remove automatically generated package names.

[axw]

Revert to using NamedGlobal/PointerToGlobal

Unnecessary change that would require a change to bindings.

[pcc]

cmd/llgoi/llgoi.go

54

I normally just hacked this in manually whenever I needed it. Would that not work for you?

Nit: can the variable be named like the flag name?

182

Wouldn't this return null if the source package isn't a interpretLine-generated package? How do things work in that case?

200

I don't much like this conspiracy between loadSourcePackage and interpretLine. Can we maybe have this function return getPackageSymbol instead of result and make interpretLine responsible for retrieving the results?

531

I was working on a pretty printing package for this, mainly because I wanted more whitespace than what %#v provides, but I suppose it could also be used to hide internal names. If that sounds useful at all I'll see if I can dig it up.

test/llgoi/maps.test

10

Here (and in a few other places) you are removing CHECK lines instead of replacing them with something equivalent. In this particular case the expression line isn't really testing much any more (not even the parser/type checker, as you aren't testing for absence of errors). I'd prefer to keep the checks unless you have a reason not to, or otherwise remove lines like this.

[axw]

Addressed review comments

• removed source printing flag
• moved __llgoiResults extraction to interpretLine
• replaced existing CHECKs in lit tests

[axw]

@pcc PTAL. Sorry for leaving this for so long.

cmd/llgoi/llgoi.go

54

I'll back this out for now.

182

Fixed to return nil if the symbol isn't found. Should never happen now, anyway, since the __llgoiResults call has been moved into interpretLine

531

If you still have this, please do send it along. If not, I'll do it later.

[pcc]

LGTM

cmd/llgoi/llgoi.go

531

I've uploaded the code I wrote to D19473. It may not be very useful though as I didn't get very far.

cmd/llgoi/llgoi.go

 //===- llgoi.go - llgo-based Go REPL --------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This is llgoi, a Go REPL based on llgo and the LLVM JIT.
 //
 //===----------------------------------------------------------------------===//
 
 package main
 
 import (
+  "bufio"
   "bytes"
   "errors"
+  "flag"
   "fmt"
   "go/ast"
   "go/build"
   "go/parser"
   "go/scanner"
   "go/token"
   "io"
   "os"
   "os/exec"
   "path/filepath"
   "runtime/debug"
   "strconv"
   "strings"
   "unsafe"
 
   "llvm.org/llgo/driver"
   "llvm.org/llgo/irgen"
   "llvm.org/llgo/third_party/gotools/go/types"
   "llvm.org/llgo/third_party/liner"
   "llvm.org/llvm/bindings/go/llvm"
 )
 
+// /* Force exporting __morestack if it's available, so that it is
+//    available to the engine when linking with libLLVM.so. */
+//
+// void *__morestack __attribute__((weak));
+import "C"
+
+var (
+  dumpSource bool
+)
+
+func init() {
+  flag.BoolVar(&dumpSource, "fdump-trace", false, "enable dumping source to stderr before execution")
+}
+
 func getInstPrefix() (string, error) {
   path, err := exec.LookPath(os.Args[0])
   if err != nil {
     return "", err
   }
 
   path, err = filepath.EvalSymlinks(path)
   if err != nil {
func (in *interp) init() error {
   return nil
 }
 
 func (in *interp) dispose() {
   in.liner.Close()
   in.engine.Dispose()
 }
 
-func (in *interp) loadSourcePackageFromCode(pkgcode, pkgpath string, copts irgen.CompilerOptions) (*types.Package, error) {
+func (in *interp) loadSourcePackageFromCode(pkgcode, pkgpath string, copts irgen.CompilerOptions) (*types.Package, []interface{}, error) {
   fset := token.NewFileSet()
   file, err := parser.ParseFile(fset, "<input>", pkgcode, parser.DeclarationErrors|parser.ParseComments)
   if err != nil {
-    return nil, err
+    return nil, nil, err
   }
-
   files := []*ast.File{file}
-
   return in.loadSourcePackage(fset, files, pkgpath, copts)
 }
 
-func (in *interp) loadSourcePackage(fset *token.FileSet, files []*ast.File, pkgpath string, copts irgen.CompilerOptions) (pkg *types.Package, err error) {
+func (in *interp) loadSourcePackage(fset *token.FileSet, files []*ast.File, pkgpath string, copts irgen.CompilerOptions) (
+  _ *types.Package, _ []interface{}, resultErr error,
+) {
   compiler, err := irgen.NewCompiler(copts)
   if err != nil {
-    return
+    return nil, nil, err
   }
 
   module, err := compiler.Compile(fset, files, pkgpath)
   if err != nil {
-    return
+    return nil, nil, err
   }
-  pkg = module.Package
+  pkg := module.Package
 
   if in.engine.C != nil {
     in.engine.AddModule(module.Module)
   } else {
     options := llvm.NewMCJITCompilerOptions()
     in.engine, err = llvm.NewMCJITCompiler(module.Module, options)
     if err != nil {
-      return
+      return nil, nil, err
     }
   }
 
-  importname := irgen.ManglePackagePath(pkgpath) + "..import$descriptor"
-  importglobal := module.Module.NamedGlobal(importname)
+  getPackageSymbol := func(name string) unsafe.Pointer {
+    symbolName := irgen.ManglePackagePath(pkgpath) + "." + name
+    global := module.Module.NamedGlobal(symbolName)
+    return in.engine.PointerToGlobal(global)
+  }
 
-  var importfunc func()
-  *(*unsafe.Pointer)(unsafe.Pointer(&importfunc)) = in.engine.PointerToGlobal(importglobal)
+  var importFunc func()
+  importAddress := getPackageSymbol(".import$descriptor")
+  *(*unsafe.Pointer)(unsafe.Pointer(&importFunc)) = importAddress
 
   defer func() {
     p := recover()
     if p != nil {
-      err = fmt.Errorf("panic: %v\n%v", p, string(debug.Stack()))
+      resultErr = fmt.Errorf("panic: %v\n%v", p, string(debug.Stack()))
     }
   }()
-  importfunc()
+  importFunc()
   in.pkgmap[pkgpath] = pkg
-  return
+
+  var results []interface{}
+  llgoiResultsAddress := getPackageSymbol("__llgoiResults$descriptor")
+  if llgoiResultsAddress != nil {
+    var resultsFunc func() []interface{}
+    *(*unsafe.Pointer)(unsafe.Pointer(&resultsFunc)) = llgoiResultsAddress
+    results = resultsFunc()
+  }
+
+  return pkg, results, nil
 }
 
 func (in *interp) augmentPackageScope(pkg *types.Package) {
   for _, obj := range in.scope {
     pkg.Scope().Insert(obj)
   }
 }
 
func (l *line) append(str string, assigns []string) {
   }
   l.line += str
 }
 
 func (l *line) ready() bool {
   return l.parens <= 0 && l.bracks <= 0 && l.braces <= 0
 }
 
-func (in *interp) readExprLine(str string, assigns []string) error {
+func (in *interp) readExprLine(str string, assigns []string) ([]interface{}, error) {
   in.pendingLine.append(str, assigns)
-
-  if in.pendingLine.ready() {
-    err := in.interpretLine(in.pendingLine)
-    in.pendingLine = line{}
-    return err
-  } else {
-    return nil
+  if !in.pendingLine.ready() {
+    return nil, nil
   }
+  results, err := in.interpretLine(in.pendingLine)
+  in.pendingLine = line{}
+  return results, err
 }
 
-func (in *interp) interpretLine(l line) error {
+func (in *interp) interpretLine(l line) ([]interface{}, error) {
   pkgname := fmt.Sprintf("input%05d", in.pkgnum)
   in.pkgnum++
 
   pkg := types.NewPackage(pkgname, pkgname)
   scope := pkg.Scope()
 
   for _, imppkg := range in.imports {
     obj := types.NewPkgName(token.NoPos, pkg, imppkg.Name(), imppkg)
     scope.Insert(obj)
   }
 
   in.augmentPackageScope(pkg)
 
   var tv types.TypeAndValue
   if l.declName == "" && !l.isStmt {
     var err error
     tv, err = types.Eval(l.line, pkg, scope)
     if err != nil {
-      return err
+      return nil, err
     }
   }
 
   var code bytes.Buffer
-  fmt.Fprintf(&code, "package %s", pkgname)
-  code.WriteString("\n\nimport __fmt__ \"fmt\"\n")
-  code.WriteString("import __os__ \"os\"\n")
+  fmt.Fprintf(&code, "package %s\n", pkgname)
 
   for _, pkg := range in.imports {
     fmt.Fprintf(&code, "import %q\n", pkg.Path())
   }
 
   if l.declName != "" {
     code.WriteString(l.line)
   } else if !l.isStmt && tv.IsValue() {
     var typs []types.Type
     if tuple, ok := tv.Type.(*types.Tuple); ok {
       typs = make([]types.Type, tuple.Len())
       for i := range typs {
         typs[i] = tuple.At(i).Type()
       }
     } else {
       typs = []types.Type{tv.Type}
     }
     if len(l.assigns) == 2 && tv.HasOk() {
       typs = append(typs, types.Typ[types.Bool])
     }
     if len(l.assigns) != 0 && len(l.assigns) != len(typs) {
-      return errors.New("return value mismatch")
+      return nil, errors.New("return value mismatch")
     }
 
     code.WriteString("var ")
     for i := range typs {
       if i != 0 {
         code.WriteString(", ")
       }
       if len(l.assigns) != 0 && l.assigns[i] != "" {
         if _, ok := in.scope[l.assigns[i]]; ok {
           fmt.Fprintf(&code, "__llgoiV%d", i)
         } else {
           code.WriteString(l.assigns[i])
         }
       } else {
         fmt.Fprintf(&code, "__llgoiV%d", i)
       }
     }
-    fmt.Fprintf(&code, " = %s\n\n", l.line)
+    fmt.Fprintf(&code, " = %s\n", l.line)
 
-    code.WriteString("func init() {\n\t")
-    for i, t := range typs {
-      var varname, prefix string
+    code.WriteString("func init() {\n")
+    varnames := make([]string, len(typs))
+    for i := range typs {
+      var varname string
       if len(l.assigns) != 0 && l.assigns[i] != "" {
         if _, ok := in.scope[l.assigns[i]]; ok {
           fmt.Fprintf(&code, "\t%s = __llgoiV%d\n", l.assigns[i], i)
         }
         varname = l.assigns[i]
-        prefix = l.assigns[i]
       } else {
         varname = fmt.Sprintf("__llgoiV%d", i)
-        prefix = fmt.Sprintf("#%d", i)
       }
-      if _, ok := t.Underlying().(*types.Interface); ok {
-        fmt.Fprintf(&code, "\t__fmt__.Printf(\"%s %s (%%T) = %%+v\\n\", %s, %s)\n", prefix, t.String(), varname, varname)
-      } else {
-        fmt.Fprintf(&code, "\t__fmt__.Printf(\"%s %s = %%+v\\n\", %s)\n", prefix, t.String(), varname)
-      }
+      varnames[i] = varname
+    code.WriteString("}\n\n")
+
+    code.WriteString("func __llgoiResults() []interface{} {\n")
+    code.WriteString("\treturn []interface{}{\n")
+    for _, varname := range varnames {
+      fmt.Fprintf(&code, "\t\t%s,\n", varname)
     }
-    code.WriteString("}")
+    code.WriteString("\t}\n")
+    code.WriteString("}\n")
   } else {
     if len(l.assigns) != 0 {
-      return errors.New("return value mismatch")
+      return nil, errors.New("return value mismatch")
     }
 
     fmt.Fprintf(&code, "func init() {\n\t%s}", l.line)
   }
 
+  source := code.String()
+  if dumpSource {
+    scanner := bufio.NewScanner(&code)
+    for line := 1; scanner.Scan(); line++ {
+      fmt.Fprintf(os.Stderr, "//%04d: %s\n", line, scanner.Text())
+    }
+  }
+
   copts := in.copts
   copts.PackageCreated = in.augmentPackageScope
   copts.DisableUnusedImportCheck = true
-  pkg, err := in.loadSourcePackageFromCode(code.String(), pkgname, copts)
+  pkg, results, err := in.loadSourcePackageFromCode(source, pkgname, copts)
   if err != nil {
-    return err
+    return nil, err
   }
 
   in.imports = append(in.imports, pkg)
 
   for _, assign := range l.assigns {
     if assign != "" {
       if _, ok := in.scope[assign]; !ok {
         in.scope[assign] = pkg.Scope().Lookup(assign)
       }
     }
   }
 
   if l.declName != "" {
     in.scope[l.declName] = pkg.Scope().Lookup(l.declName)
   }
 
-  return nil
+  return results, nil
 }
 
 func (in *interp) maybeReadAssignment(line string, s *scanner.Scanner, initial string, base int) (bool, error) {
   if initial == "_" {
     initial = ""
   }
   assigns := []string{initial}
 
for tok == token.COMMA {
 
     pos, tok, lit = s.Scan()
   }
 
   if tok != token.DEFINE {
     return false, nil
   }
 
-  return true, in.readExprLine(line[int(pos)-base+2:], assigns)
+  // It's an assignment statement, there are no results.
+  _, err := in.readExprLine(line[int(pos)-base+2:], assigns)
+  return true, err
 }
 
 func (in *interp) loadPackage(pkgpath string) (*types.Package, error) {
   pkg, err := in.copts.Importer(in.pkgmap, pkgpath)
   if err == nil {
     return pkg, nil
   }
 
   buildpkg, err := build.Import(pkgpath, ".", 0)
   if err != nil {
     return nil, err
   }
   if len(buildpkg.CgoFiles) != 0 {
     return nil, fmt.Errorf("%s: cannot load cgo package", pkgpath)
   }
 
   for _, imp := range buildpkg.Imports {
     _, err := in.loadPackage(imp)
     if err != nil {
       return nil, err
     }
   }
 
-  fmt.Printf("# %s\n", pkgpath)
-
   inputs := make([]string, len(buildpkg.GoFiles))
   for i, file := range buildpkg.GoFiles {
     inputs[i] = filepath.Join(buildpkg.Dir, file)
   }
 
   fset := token.NewFileSet()
   files, err := driver.ParseFiles(fset, inputs)
   if err != nil {
     return nil, err
   }
 
-  return in.loadSourcePackage(fset, files, pkgpath, in.copts)
+  pkg, _, err = in.loadSourcePackage(fset, files, pkgpath, in.copts)
+  return pkg, err
 }
 
 // readLine accumulates lines of input, including trailing newlines,
 // executing statements as they are completed.
-func (in *interp) readLine(line string) error {
+func (in *interp) readLine(line string) ([]interface{}, error) {
   if !in.pendingLine.ready() {
     return in.readExprLine(line, nil)
   }
 
   var s scanner.Scanner
   fset := token.NewFileSet()
   file := fset.AddFile("", fset.Base(), len(line))
   s.Init(file, []byte(line), nil, 0)
 
   _, tok, lit := s.Scan()
   switch tok {
   case token.EOF:
-    return nil
+    return nil, nil
 
   case token.IMPORT:
     _, tok, lit = s.Scan()
     if tok != token.STRING {
-      return errors.New("expected string literal")
+      return nil, errors.New("expected string literal")
     }
     pkgpath, err := strconv.Unquote(lit)
     if err != nil {
-      return err
+      return nil, err
     }
     pkg, err := in.loadPackage(pkgpath)
     if err != nil {
-      return err
+      return nil, err
     }
     in.imports = append(in.imports, pkg)
-    return nil
+    return nil, nil
 
   case token.IDENT:
     ok, err := in.maybeReadAssignment(line, &s, lit, file.Base())
     if err != nil {
-      return err
+      return nil, err
     }
     if ok {
-      return nil
+      return nil, nil
     }
-
     fallthrough
 
   default:
     return in.readExprLine(line, nil)
   }
 }
 
+// printResult prints a value that was the result of an expression evaluated
+// by the interpreter.
+func printResult(w io.Writer, v interface{}) {
+  // TODO the result should be formatted in Go syntax, without
+  // package qualifiers for types defined within the interpreter.
+  fmt.Fprintf(w, "%+v", v)
+}
+
 // formatHistory reformats the provided Go source by collapsing all lines
 // and adding semicolons where required, suitable for adding to line history.
 func formatHistory(input []byte) string {
   var buf bytes.Buffer
   var s scanner.Scanner
   fset := token.NewFileSet()
   file := fset.AddFile("", fset.Base(), len(input))
   s.Init(file, input, nil, 0)
if semicolon {
         buf.WriteRune(';')
       }
     }
   }
   return buf.String()
 }
 
 func main() {
+  flag.Parse()
   llvm.LinkInMCJIT()
   llvm.InitializeNativeTarget()
   llvm.InitializeNativeAsmPrinter()
 
   var in interp
   err := in.init()
   if err != nil {
     panic(err)
if err == io.EOF {
       break
     } else if err != nil {
       panic(err)
     }
     if line == "" {
       continue
     }
     buf.WriteString(line + "\n")
-    err = in.readLine(line + "\n")
+    results, err := in.readLine(line + "\n")
     if err != nil {
       fmt.Println(err)
     }
+    for _, result := range results {
+      printResult(os.Stdout, result)
+      fmt.Println()
+    }
   }
 
   if liner.TerminalSupported() {
     fmt.Println()
   }
 }

test/llgoi/arith.test

 // RUN: llgoi < %s | FileCheck %s
 
 1 + 1
-// CHECK: #0 untyped int = 2
+// CHECK: 2

test/llgoi/import-source.test

 // RUN: env GOPATH=%S/Inputs llgoi < %s 2>&1 | FileCheck %s
 
 // make sure user symbols do not conflict with imported source package
 Answer := 1
 
 import "foo"
-// CHECK: # bar
-// CHECK: # foo
 
 // Test that importing binary after source works.
 import "strconv"
 
 foo.Answer()
-// CHECK: #0 int = 42
+// CHECK: 42
 
 strconv.FormatBool(true)
-// CHECK: #0 string = true
+// CHECK: true
 
 var v1 strconv.NumError
 var v2 strconv.NumError
 
 // v1 and v2 should have the same type identity.
 // CHECK-NOT: cannot assign
 v1 = v2
 
 // Method lookup relies on v1 having a consistent type.
 v1.Error
 
 import "foo_cgo"
 // CHECK: foo_cgo: cannot load cgo package

test/llgoi/import-source2.test

 // RUN: env GOPATH=%S/Inputs llgoi < %s | FileCheck %s
 
 // Test that importing binary before source works.
 import "strconv"
 
 import "foo"
-// CHECK: # bar
-// CHECK: # foo
 
 foo.Answer()
-// CHECK: #0 int = 42
+// CHECK: 42
 
 strconv.FormatBool(true)
-// CHECK: #0 string = true
+// CHECK: true

test/llgoi/interfaces.test

 // RUN: llgoi < %s | FileCheck %s
 
 import "errors"
 err := errors.New("foo")
-// CHECK: err error {{.*}} = foo
 
 err.(interface{Foo()})
 // CHECK: panic: interface conversion
 
-_, _ := err.(interface{Foo()})
-// CHECK: #0 interface{Foo()} (<nil>) = <nil>
-// CHECK: #1 bool = false
+_, ok := err.(interface{Foo()})
+ok
+// CHECK: false
 
 err.(interface{Error() string})
-// CHECK: #0 interface{Error() string} {{.*}} = foo
+// CHECK: foo
 
 _, _ := err.(interface{Error() string})
-// CHECK: #0 interface{Error() string} {{.*}} = foo
-// CHECK: #1 bool = true

test/llgoi/maps.test

 // RUN: llgoi < %s | FileCheck %s
 
 m := make(map[int]int)
-// CHECK: m map[int]int = map[]
+m
+// CHECK: map[]
 
 m[0]
-// CHECK: #0 int = 0
+// CHECK: 0
 
 _, _ := m[0]
-// CHECK: #0 int = 0
-// CHECK: #1 bool = false
 
 func() {
   m[0] = 1
 }()
 
 m[0]
-// CHECK: #0 int = 1
+// CHECK: 1
 
-_, _ := m[0]
-// CHECK: #0 int = 1
-// CHECK: #1 bool = true
+m0, ok := m[0]
+m0
+// CHECK: 1
+ok
+// CHECK: true

test/llgoi/vars.test

 // RUN: llgoi < %s 2>&1 | FileCheck %s
 
 x := 3
-// CHECK: x untyped int = 3
+x
+// CHECK: 3
 
 x + x
-// CHECK: #0 int = 6
+// CHECK: 6
 
 x * x
-// CHECK: #0 int = 9
+// CHECK: 9
 
 x = 4
 x + x
-// CHECK: #0 int = 8
+// CHECK: 8
 
 x := true
 // CHECK: cannot assign {{.*}} to x (variable of type int)
 
 x, y := func() (int, int) {
   return 1, 2
 }()
-// CHECK: x int = 1
-// CHECK: y int = 2
+// CHECK: 1
+// CHECK: 2
 
 x, _ = func() (int, int) {
   return 3, 4
 }()
 x
-// CHECK: #0 int = 3
+// CHECK: 3