go.tools/go/types: separate package descriptor from package object

Includes changes by adonovan to make oracle work again
(former CL 13395050).

R=adonovan
CC=golang-dev
https://golang.org/cl/13672044

go/types/api.go

@@ -54,11 +54,17 @@ type Config struct {
 	// If FakeImportC is set, `import "C"` (for packages requiring Cgo)
 	// declares an empty "C" package and errors are omitted for qualified
 	// identifiers referring to package C (which won't find an object).
-	// Caution: Effects may be unpredictable due to unpredictable follow-
+	// This feature is intended for the standard library cmd/api tool.
-	// up errors - do not use casually! This feature is mainly intended
+	//
-	// for the standard library cmd/api tool.
+	// Caution: Effects may be unpredictable due to follow-up errors.
+	//          Do not use casually!
 	FakeImportC bool
 
+	// Packages is used to look up (and thus canonicalize) packages by
+	// package path. If Packages is nil, it is set to a new empty map.
+	// During type-checking, imported packages are added to the map.
+	Packages map[string]*Package
+
 	// If Error != nil, it is called with each error found
 	// during type checking. The error strings of errors with
 	// detailed position information are formatted as follows:
@@ -67,15 +73,16 @@ type Config struct {
 
 	// If Import != nil, it is called for each imported package.
 	// Otherwise, GcImporter is called.
-	// An importer resolves import paths to Package objects.
+	// An importer resolves import paths to Packages.
-	// The imports map records the packages already imported,
+	// The imports map records packages already known,
-	// indexed by package id (canonical import path).
+	// indexed by canonical package path. The type-checker will
-	// An importer must determine the canonical import path and
+	// invoke Import with Config.Packages.
-	// check the map to see if it is already present in the imports map.
+	// An importer must determine the canonical package path and
-	// If so, the Importer can return the map entry.  Otherwise, the
+	// check imports to see if it is already present in the map.
-	// importer should load the package data for the given path into
+	// If so, the Importer can return the map entry.  Otherwise,
-	// a new *Package, record pkg in the imports map, and then
+	// the importer must load the package data for the given path
-	// return pkg.
+	// into a new *Package, record it in imports map, and return
+	// the package.
 	Import func(imports map[string]*Package, path string) (pkg *Package, err error)
 
 	// If Alignof != nil, it is called to determine the alignment
@@ -110,9 +117,10 @@ type Info struct {
 
 	// Objects maps identifiers to their corresponding objects (including
 	// package names, dots "." of dot-imports, and blank "_" identifiers).
-	// For identifiers that do not denote objects (e.g., blank identifiers
+	// For identifiers that do not denote objects (e.g., the package name
-	// on the lhs of assignments, or symbolic variables t in t := x.(type)
+	// in package clauses, blank identifiers on the lhs of assignments, or
-	// of type switch headers), the corresponding objects are nil.
+	// symbolic variables t in t := x.(type) of type switch headers), the
+	// corresponding objects are nil.
 	// BUG(gri) Label identifiers in break, continue, or goto statements
 	// are not yet mapped.
 	Objects map[*ast.Ident]Object
@@ -122,7 +130,7 @@ type Info struct {
 	//
 	//	node               declared object
 	//
-	//	*ast.ImportSpec    *Package (imports w/o renames), or imported objects (dot-imports)
+	//	*ast.ImportSpec    *PkgName (imports w/o renames), or imported objects (dot-imports)
 	//	*ast.CaseClause    type-specific *Var for each type switch case clause (incl. default)
 	//      *ast.Field         anonymous struct field or parameter *Var
 	//
@@ -157,9 +165,9 @@ type Info struct {
 // The package is marked as complete if no errors occurred, otherwise it is
 // incomplete.
 //
-// The package is specified by a list of *ast.Files and corresponding file
+// The package is specified by a list of *ast.Files and corresponding
-// set, and the import path the package is identified with. The clean path
+// file set, and the package path the package is identified with.
-// must not be empty or dot (".").
+// The clean path must not be empty or dot (".").
 func (conf *Config) Check(path string, fset *token.FileSet, files []*ast.File, info *Info) (*Package, error) {
 	pkg, err := conf.check(path, fset, files, info)
 	if err == nil {

go/types/call.go

@@ -185,11 +185,11 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) {
 	// can only appear in qualified identifiers which are mapped to
 	// selector expressions.
 	if ident, ok := e.X.(*ast.Ident); ok {
-		if pkg, _ := check.topScope.LookupParent(ident.Name).(*Package); pkg != nil {
+		if pkg, _ := check.topScope.LookupParent(ident.Name).(*PkgName); pkg != nil {
 			check.recordObject(ident, pkg)
-			exp := pkg.scope.Lookup(sel)
+			exp := pkg.pkg.scope.Lookup(sel)
 			if exp == nil {
-				if !pkg.fake {
+				if !pkg.pkg.fake {
 					check.errorf(e.Pos(), "%s not declared by package %s", sel, ident)
 				}
 				goto Error

go/types/check.go

@@ -138,16 +138,16 @@ func (check *checker) handleBailout(err *error) {
 }
 
 func (conf *Config) check(pkgPath string, fset *token.FileSet, files []*ast.File, info *Info) (pkg *Package, err error) {
-	pkg = &Package{
+	// make sure we have a package canonicalization map
-		path:    pkgPath,
+	if conf.Packages == nil {
-		scope:   NewScope(Universe),
+		conf.Packages = make(map[string]*Package)
-		imports: make(map[string]*Package),
 	}
 
+	pkg = NewPackage(pkgPath, "", NewScope(Universe)) // package name is set below
 	check := newChecker(conf, fset, pkg)
 	defer check.handleBailout(&err)
 
-	// we need a reasonable path to continue
+	// we need a reasonable package path to continue
 	if path.Clean(pkgPath) == "." {
 		check.errorf(token.NoPos, "invalid package path provided: %q", pkgPath)
 		return

go/types/gcimporter.go

@@ -370,7 +370,7 @@ func (p *gcParser) getPkg(id, name string) *Package {
 	}
 	pkg := p.imports[id]
 	if pkg == nil && name != "" {
-		pkg = NewPackage(token.NoPos, id, name, NewScope(nil), nil)
+		pkg = NewPackage(id, name, NewScope(nil))
 		p.imports[id] = pkg
 	}
 	return pkg
@@ -985,7 +985,7 @@ func (p *gcParser) parseExport() *Package {
 	}
 
 	// package was imported completely and without errors
-	pkg.MarkComplete()
+	pkg.complete = true
 
 	return pkg
 }

go/types/lookup.go

@@ -73,7 +73,7 @@ func LookupFieldOrMethod(T Type, pkg *Package, name string) (obj Object, index [
 
 func lookupFieldOrMethod(T Type, pkg *Package, name string) (obj Object, index []int, indirect bool) {
 	// WARNING: The code in this function is extremely subtle - do not modify casually!
-	//          This function and NewMethodSet should kept in sync.
+	//          This function and NewMethodSet should be kept in sync.
 
 	if name == "_" {
 		return // blank fields/methods are never found

go/types/objects.go

@@ -121,58 +121,16 @@ func (obj *object) sameId(pkg *Package, name string) bool {
 	return pkg.path == obj.pkg.path
 }
 
-// A Package represents the contents (objects) of a Go package.
+// A PkgName represents an imported Go package.
-//
+type PkgName struct {
-// A package is complete if all its package scope objects are present.
-// Incomplete packages may arise via imports where the exported data
-// contains only partial information about transitively imported and
-// re-exported packages; or as a result of type-checking a package
-// that contains errors.
-//
-// There are two kinds of Package objects, primary and secondary.
-// A primary Package has no declaring identifier, and is referenced by
-// each ast.File.Name within that package.
-// A secondary Package object is created for each ast.ImportSpec that
-// imports it; its declaring identifier is the ImportSpec.Name (if
-// any), and each qualified reference (e.g. fmt.Println) is a
-// reference to a secondary Package.
-// The Primary() method of a secondary package returns its primary
-// package; called on a primary package, it returns nil.
-// The Path(), Name() and Scope() attributes of primary and secondary
-// Packages are equal.
-// TODO(gri): consider whether this distinction carries its weight;
-// adonovan thinks not.
-//
-type Package struct {
 	object
-	path     string              // import path, "" for current (non-imported) package
-	scope    *Scope              // imported objects
-	imports  map[string]*Package // map of import paths to imported packages
-	complete bool                // if set, this package is complete
-	fake     bool                // if set, this package is fake (internal use only)
-	primary  *Package            // associated primary package (nil => self)
 }
 
-func NewPackage(pos token.Pos, path, name string, scope *Scope, imports map[string]*Package) *Package {
+func NewPkgName(pos token.Pos, pkg *Package, name string) *PkgName {
-	obj := &Package{object{nil, pos, nil, name, Typ[Invalid]}, path, scope, imports, false, false, nil}
+	return &PkgName{object{nil, pos, pkg, name, Typ[Invalid]}}
-	obj.pkg = obj
-	return obj
 }
 
-func (obj *Package) String() string               { return fmt.Sprintf("package %s", obj.Path()) }
+func (obj *PkgName) String() string { return obj.toString("package", nil) }
-func (obj *Package) Path() string                 { return obj.path }
-func (obj *Package) Scope() *Scope                { return obj.scope }
-func (obj *Package) Imports() map[string]*Package { return obj.imports }
-func (obj *Package) Complete() bool               { return obj.complete }
-func (obj *Package) Primary() *Package {
-	if obj.primary != nil {
-		return obj.primary
-	}
-	return obj
-}
-
-// MarkComplete marks a package as complete.
-func (obj *Package) MarkComplete() { obj.complete = true }
 
 // A Const represents a declared constant.
 type Const struct {

go/types/package.go

@@ -0,0 +1,41 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package types
+
+// A Package describes a Go package.
+type Package struct {
+	path     string
+	name     string
+	scope    *Scope
+	complete bool
+	imports  []*Package
+	fake     bool // scope lookup errors are silently dropped if package is fake (internal use only)
+}
+
+// NewPackage returns a new Package for the given package path,
+// name, and scope. The package is not complete and contains no
+// explicit imports.
+func NewPackage(path, name string, scope *Scope) *Package {
+	return &Package{path: path, name: name, scope: scope}
+}
+
+// Path returns the package path.
+func (pkg *Package) Path() string { return pkg.path }
+
+// Name returns the package name.
+func (pkg *Package) Name() string { return pkg.name }
+
+// Scope returns the (complete or incomplete) package scope
+// holding the objects declared at package level (TypeNames,
+// Consts, Vars, and Funcs).
+func (pkg *Package) Scope() *Scope { return pkg.scope }
+
+// A package is complete if its scope contains (at least) all
+// exported objects; otherwise it is incomplete.
+func (pkg *Package) Complete() bool { return pkg.complete }
+
+// Imports returns the list of packages explicitly imported by
+// pkg; the list is in source order. Package unsafe is excluded.
+func (pkg *Package) Imports() []*Package { return pkg.imports }

go/types/resolver.go

@@ -145,9 +145,13 @@ func (check *checker) resolveFiles(files []*ast.File) {
 		importer = GcImport
 	}
 
+	// only add packages to pkg.imports that have not been seen already
+	seen := make(map[*Package]bool)
+
 	for _, file := range files {
-		// the package identifier denotes the current package, but it is in no scope
+		// The package identifier denotes the current package,
-		check.recordObject(file.Name, pkg)
+		// but there is no corresponding package object.
+		check.recordObject(file.Name, nil)
 
 		scope = NewScope(pkg.scope)
 		check.recordScope(file, scope)
@@ -163,15 +167,16 @@ func (check *checker) resolveFiles(files []*ast.File) {
 				for iota, spec := range d.Specs {
 					switch s := spec.(type) {
 					case *ast.ImportSpec:
+						// import package
 						var imp *Package
 						path, _ := strconv.Unquote(s.Path.Value)
 						if path == "C" && check.conf.FakeImportC {
 							// TODO(gri) shouldn't create a new one each time
-							imp = NewPackage(token.NoPos, "C", "C", NewScope(nil), nil)
+							imp = NewPackage("C", "C", NewScope(nil))
 							imp.fake = true
 						} else {
 							var err error
-							imp, err = importer(pkg.imports, path)
+							imp, err = importer(check.conf.Packages, path)
 							if imp == nil && err == nil {
 								err = errors.New("Config.Import returned nil but no error")
 							}
@@ -181,6 +186,16 @@ func (check *checker) resolveFiles(files []*ast.File) {
 							}
 						}
 
+						// add package to list of explicit imports
+						// (this functionality is provided as a convenience
+						// for clients; it is not needed for type-checking)
+						if !seen[imp] {
+							seen[imp] = true
+							if imp != Unsafe {
+								pkg.imports = append(pkg.imports, imp)
+							}
+						}
+
 						// local name overrides imported package name
 						name := imp.name
 						if s.Name != nil {
@@ -191,33 +206,30 @@ func (check *checker) resolveFiles(files []*ast.File) {
 							}
 						}
 
-						imp2 := NewPackage(s.Pos(), path, name, imp.scope, nil)
+						obj := NewPkgName(s.Pos(), imp, name)
-						imp2.primary = imp
-						imp2.complete = imp.complete
-						imp2.fake = imp.fake
 						if s.Name != nil {
-							check.recordObject(s.Name, imp2)
+							// in a dot-import, the dot represents the package
+							check.recordObject(s.Name, obj)
 						} else {
-							check.recordImplicit(s, imp2)
+							check.recordImplicit(s, obj)
 						}
 
 						// add import to file scope
 						if name == "." {
 							// merge imported scope with file scope
 							for _, obj := range imp.scope.elems {
-								// gcimported package scopes contain non-exported
+								// A package scope may contain non-exported objects,
-								// objects such as types used in partially exported
+								// do not import them!
-								// objects - do not accept them
 								if obj.IsExported() {
 									// Note: This will change each imported object's scope!
-									//       May be an issue for types aliases.
+									//       May be an issue for type aliases.
 									check.declareObj(scope, nil, obj)
 									check.recordImplicit(s, obj)
 								}
 							}
 						} else {
 							// declare imported package object in file scope
-							check.declareObj(scope, nil, imp2)
+							check.declareObj(scope, nil, obj)
 						}
 
 					case *ast.ValueSpec:

go/types/resolver_test.go

@@ -75,14 +75,14 @@ func TestResolveIdents(t *testing.T) {
 	// resolve and type-check package AST
 	var conf Config
 	idents := make(map[*ast.Ident]Object)
-	pkg, err := conf.Check("testResolveIdents", fset, files, &Info{Objects: idents})
+	_, err := conf.Check("testResolveIdents", fset, files, &Info{Objects: idents})
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// check that all packages were imported
 	for _, name := range pkgnames {
-		if pkg.imports[name] == nil {
+		if conf.Packages[name] == nil {
 			t.Errorf("package %s not imported", name)
 		}
 	}
@@ -97,7 +97,7 @@ func TestResolveIdents(t *testing.T) {
 						t.Errorf("%s: unresolved qualified identifier %s", fset.Position(x.Pos()), x.Name)
 						return false
 					}
-					if _, ok := obj.(*Package); ok && idents[s.Sel] == nil {
+					if _, ok := obj.(*PkgName); ok && idents[s.Sel] == nil {
 						t.Errorf("%s: unresolved selector %s", fset.Position(s.Sel.Pos()), s.Sel.Name)
 						return false
 					}

go/types/typexpr.go

@@ -46,7 +46,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool)
 	assert(typ != nil)
 
 	switch obj := obj.(type) {
-	case *Package:
+	case *PkgName:
 		check.errorf(e.Pos(), "use of package %s not in selector", obj.name)
 		return
 

go/types/universe.go

@@ -87,7 +87,7 @@ var predeclaredFunctions = [...]*Builtin{
 
 func init() {
 	Universe = NewScope(nil)
-	Unsafe = NewPackage(token.NoPos, "unsafe", "unsafe", NewScope(Universe), nil)
+	Unsafe = NewPackage("unsafe", "unsafe", NewScope(Universe))
 	Unsafe.complete = true
 
 	// predeclared types

importer/pkginfo.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"strconv"
 
 	"code.google.com/p/go.tools/go/exact"
 	"code.google.com/p/go.tools/go/types"
@@ -33,33 +32,6 @@ func (info *PackageInfo) String() string {
 	return fmt.Sprintf("PackageInfo(%s)", info.Pkg.Path())
 }
 
-// Imports returns the set of packages imported by this one, in source
-// order.  Callers should not mutate the result.
-//
-func (info *PackageInfo) Imports() []*types.Package {
-	var imports []*types.Package
-
-	// We iterate over the syntax (info.Files) not the types
-	// (info.Pkg.Imports()) because the latter may contain the
-	// transitive closure of dependencies, e.g. when using GcImporter.
-	seen := make(map[*types.Package]bool)
-	for _, file := range info.Files {
-		for _, imp := range file.Imports {
-			path, _ := strconv.Unquote(imp.Path.Value)
-			if path == "unsafe" {
-				continue // not a true package
-			}
-			typkg := info.Pkg.Imports()[path]
-			if seen[typkg] {
-				continue // already seen
-			}
-			seen[typkg] = true
-			imports = append(imports, typkg)
-		}
-	}
-	return imports
-}
-
 // TypeOf returns the type of expression e.
 // Precondition: e belongs to the package's ASTs.
 //

oracle/describe.go

@@ -196,7 +196,7 @@ func findInterestingNode(pkginfo *importer.PackageInfo, path []ast.Node) ([]ast.
 
 		case *ast.Ident:
 			switch obj := pkginfo.ObjectOf(n).(type) {
-			case *types.Package:
+			case *types.PkgName:
 				return path, actionPackage
 
 			case *types.Const:
@@ -264,6 +264,10 @@ func findInterestingNode(pkginfo *importer.PackageInfo, path []ast.Node) ([]ast.
 				// FuncType.{Params.Results} -- actionExpr
 				// FuncDecl.Recv             -- actionExpr
 
+			case *ast.File:
+				// 'package foo'
+				return path, actionPackage
+
 			case *ast.ImportSpec:
 				// TODO(adonovan): fix: why no package object? go/types bug?
 				return path[1:], actionPackage
@@ -715,21 +719,14 @@ func describePackage(o *oracle, path []ast.Node) (*describePackageResult, error)
 		pkg = o.prog.ImportedPackage(importPath).Object
 
 	case *ast.Ident:
-		pkg = o.queryPkgInfo.ObjectOf(n).(*types.Package)
-
 		if _, isDef := path[1].(*ast.File); isDef {
 			// e.g. package id
+			pkg = o.queryPkgInfo.Pkg
 			description = fmt.Sprintf("definition of package %q", pkg.Path())
 		} else {
 			// e.g. import id
 			//  or  id.F()
-
+			pkg = o.queryPkgInfo.ObjectOf(n).Pkg()
-			// go/types internally creates a new Package
-			// object for each import, so the packages for
-			// 'package x' and 'import "x"' differ.
-			// Call Primary() to get the real thing.
-			pkg = pkg.Primary()
-
 			description = fmt.Sprintf("reference to package %q", pkg.Path())
 		}
 
@@ -870,7 +867,7 @@ func tokenOf(o types.Object) string {
 		return "type"
 	case *types.Const:
 		return "const"
-	case *types.Package:
+	case *types.PkgName:
 		return "package"
 	}
 	panic(o)

oracle/freevars.go

@@ -54,7 +54,7 @@ func freevars(o *oracle) (queryResult, error) {
 			return nil // TODO(adonovan): fix: this fails for *types.Label.
 			panic(o.errorf(n, "no types.Object for ast.Ident"))
 		}
-		if _, ok := obj.(*types.Package); ok {
+		if _, ok := obj.(*types.PkgName); ok {
 			return nil // imported package
 		}
 		if n.Pos() == obj.Pos() {

oracle/referrers.go

@@ -28,14 +28,11 @@ func referrers(o *oracle) (queryResult, error) {
 		return nil, o.errorf(false, "no object for identifier")
 	}
 
-	obj = primaryPkg(obj)
-
 	// Iterate over all go/types' resolver facts for the entire program.
 	var refs []token.Pos
 	for _, info := range o.typeInfo {
 		for id2, obj2 := range info.Objects {
-			obj2 = primaryPkg(obj2)
+			if sameObj(obj, obj2) {
-			if obj2 == obj {
 				if id2.NamePos == obj.Pos() {
 					continue // skip defining ident
 				}
@@ -52,20 +49,19 @@ func referrers(o *oracle) (queryResult, error) {
 	}, nil
 }
 
-// primaryPkg returns obj unchanged unless it is a (secondary) package
+// same reports whether x and y are identical, or both are PkgNames
-// object created by an ImportSpec, in which case the canonical
+// referring to the same Package.
-// (primary) object is returned.
 //
-// TODO(adonovan): The need for this function argues against the
+func sameObj(x, y types.Object) bool {
-// wisdom of the primary/secondary distinction.  Discuss with gri.
+	if x == y {
-//
+		return true
-func primaryPkg(obj types.Object) types.Object {
+	}
-	if pkg, ok := obj.(*types.Package); ok {
+	if _, ok := x.(*types.PkgName); ok {
-		if prim := pkg.Primary(); prim != nil {
+		if _, ok := y.(*types.PkgName); ok {
-			return prim
+			return x.Pkg() == y.Pkg()
 		}
 	}
-	return obj
+	return false
 }
 
 type referrersResult struct {
@@ -93,7 +89,7 @@ func (r *referrersResult) toJSON(res *json.Result, fset *token.FileSet) {
 		Pos:  fset.Position(r.query).String(),
 		Desc: r.obj.String(),
 	}
-	if pos := r.obj.Pos(); pos != token.NoPos { // primary package objects have no Pos()
+	if pos := r.obj.Pos(); pos != token.NoPos { // Package objects have no Pos()
 		referrers.ObjPos = fset.Position(pos).String()
 	}
 	for _, ref := range r.refs {

oracle/testdata/src/main/referrers-json.golden

@@ -3,11 +3,11 @@
 	"mode": "referrers",
 	"referrers": {
 		"pos": "testdata/src/main/referrers-json.go:14:8",
+		"objpos": "testdata/src/main/referrers-json.go:7:8",
 		"desc": "package lib",
 		"refs": [
 			"testdata/src/main/referrers-json.go:14:8",
-			"testdata/src/main/referrers-json.go:14:19",
+			"testdata/src/main/referrers-json.go:14:19"
-			"testdata/src/lib/lib.go:1:9"
 		]
 	}
 }-------- @referrers ref-method --------

ssa/builder.go

@@ -2350,10 +2350,10 @@ func (p *Package) Build() {
 	emitStore(init, initguard, vTrue)
 
 	// Call the init() function of each package we import.
-	for _, obj := range p.info.Imports() {
+	for _, pkg := range p.info.Pkg.Imports() {
-		prereq := p.Prog.packages[obj]
+		prereq := p.Prog.packages[pkg]
 		if prereq == nil {
-			panic(fmt.Sprintf("Package(%q).Build(): unsatisified import: Program.CreatePackage(%q) was not called", p.Object.Path(), obj.Path()))
+			panic(fmt.Sprintf("Package(%q).Build(): unsatisfied import: Program.CreatePackage(%q) was not called", p.Object.Path(), pkg.Path()))
 		}
 		var v Call
 		v.Call.Value = prereq.init

ssa/interp/reflect.go

@@ -28,7 +28,7 @@ type opaqueType struct {
 func (t *opaqueType) String() string { return t.name }
 
 // A bogus "reflect" type-checker package.  Shared across interpreters.
-var reflectTypesPackage = types.NewPackage(token.NoPos, "reflect", "reflect", nil, nil)
+var reflectTypesPackage = types.NewPackage("reflect", "reflect", nil)
 
 // rtype is the concrete type the interpreter uses to implement the
 // reflect.Type interface.  Since its type is opaque to the target