[llgo] llgoi: Fix type identity for imported binary packages.

[pcc]

Summary

go/loader creates a fresh package map for each source package it imports. In
llgoi this caused binary imported packages to be imported anew for every
input line, resulting in spurious type errors and panics in go/ssa when
encountering previously imported types. Fix this by ignoring the provided
package map completely in the importer wrapper and using our own.

[axw]

cmd/llgoi/llgoi.go

101 ↗

(On Diff #22162)

I'd prefer if we stuck to the contract of go/types.Importer (as before), and instead just set go/types.Config.Packages to the same map each time. I understand that the package map is only read/updated by the importer currently, but I'm not sure if that's guaranteed to be the case forever. Perhaps I'm being paranoid?

[pcc]

• Set types.Config.Packages instead of overriding the package map

cmd/llgoi/llgoi.go

101 ↗

(On Diff #22162)

Seems reasonable, done.

[axw]

LGTM, thanks.

llgo/trunk/cmd/llgoi/llgoi.go

type interp struct {
 
   pendingLine line
 
   copts irgen.CompilerOptions
 
   imports []*types.Package
   scope   map[string]types.Object
 
-  pkgmap, inputPkgmap map[string]*types.Package
-  pkgnum              int
+  pkgmap map[string]*types.Package
+  pkgnum int
 }
 
 func (in *interp) makeCompilerOptions() error {
   prefix, err := getInstPrefix()
   if err != nil {
     return err
   }
 
   importPaths := []string{filepath.Join(prefix, "lib", "go", "llgo-"+llvmVersion())}
   in.copts = irgen.CompilerOptions{
     TargetTriple:  llvm.DefaultTargetTriple(),
     ImportPaths:   importPaths,
     GenerateDebug: true,
+    Packages:      in.pkgmap,
   }
   err = in.copts.MakeImporter()
   if err != nil {
     return err
   }
 
   origImporter := in.copts.Importer
   in.copts.Importer = func(pkgmap map[string]*types.Package, pkgpath string) (*types.Package, error) {
-    if pkg, ok := in.inputPkgmap[pkgpath]; ok {
-      return pkg, nil
-    }
     if pkg, ok := pkgmap[pkgpath]; ok && pkg.Complete() {
       return pkg, nil
     }
     return origImporter(pkgmap, pkgpath)
   }
   return nil
 }
 
 func (in *interp) init() error {
   in.scope = make(map[string]types.Object)
   in.pkgmap = make(map[string]*types.Package)
-  in.inputPkgmap = make(map[string]*types.Package)
 
   err := in.makeCompilerOptions()
   if err != nil {
     return err
   }
 
   return nil
 }
func (in *interp) loadSourcePackage(fset *token.FileSet, files []*ast.File, pkgpath string, copts irgen.CompilerOptions) (pkg *types.Package, err error) {
 
   defer func() {
     p := recover()
     if p != nil {
       err = fmt.Errorf("panic: %v\n%v", p, string(debug.Stack()))
     }
   }()
   importfunc()
-  in.inputPkgmap[pkgpath] = pkg
+  in.pkgmap[pkgpath] = pkg
   return
 }
 
 func (in *interp) augmentPackageScope(pkg *types.Package) {
   for _, obj := range in.scope {
     pkg.Scope().Insert(obj)
   }
 }

llgo/trunk/irgen/compiler.go

type CompilerOptions struct {
   // PackageCreated is a hook passed to the go/loader package via
   // loader.Config, see the documentation for that package for more
   // information.
   PackageCreated func(*types.Package)
 
   // DisableUnusedImportCheck disables the unused import check performed
   // by go/types if set to true.
   DisableUnusedImportCheck bool
+
+  // Packages is used by go/types as the imported package map if non-nil.
+  Packages map[string]*types.Package
 }
 
 type Compiler struct {
   opts       CompilerOptions
   dataLayout string
   pnacl      bool
 }
 
if compiler.Importer == nil {
     if err != nil {
       return nil, err
     }
   }
 
   impcfg := &loader.Config{
     Fset: fset,
     TypeChecker: types.Config{
-      Import: compiler.Importer,
-      Sizes:  compiler.llvmtypes,
+      Packages: compiler.Packages,
+      Import:   compiler.Importer,
+      Sizes:    compiler.llvmtypes,
       DisableUnusedImportCheck: compiler.DisableUnusedImportCheck,
     },
     Build:          &buildctx.Context,
     PackageCreated: compiler.PackageCreated,
   }
   // If no import path is specified, then set the import
   // path to be the same as the package's name.
   if importpath == "" {

llgo/trunk/test/llgoi/import-source.test

-// RUN: env GOPATH=%S/Inputs llgoi < %s | FileCheck %s
+// 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
 
 strconv.FormatBool(true)
 // CHECK: #0 string = 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