go/loader: rename SourceImports flag to ImportFromBinary and invert sense

...since the zero value is more useful by far.

This is a breaking API change, obviously.  (One or two tests in this
CL have intentional been left using the zero value, i.e., they now
load source.)

Change-Id: I42287bfcdb1afef8ee84e5eac12534dd0a1fd5d2
Reviewed-on: https://go-review.googlesource.com/5653
Reviewed-by: Robert Griesemer <gri@golang.org>

cmd/callgraph/main.go

@@ -153,10 +153,7 @@ func main() {
 var stdout io.Writer = os.Stdout
 
 func doCallgraph(ctxt *build.Context, algo, format string, tests bool, args []string) error {
-	conf := loader.Config{
+	conf := loader.Config{Build: ctxt}
-		Build:         ctxt,
-		SourceImports: true,
-	}
 
 	if len(args) == 0 {
 		fmt.Fprintln(os.Stderr, Usage)

cmd/eg/eg.go

@@ -55,9 +55,8 @@ func doMain() error {
 	}
 
 	conf := loader.Config{
-		Fset:          token.NewFileSet(),
+		Fset:       token.NewFileSet(),
-		ParserMode:    parser.ParseComments,
+		ParserMode: parser.ParseComments,
-		SourceImports: true,
 	}
 
 	// The first Created package is the template.

cmd/ssadump/main.go

@@ -79,8 +79,8 @@ func doMain() error {
 	args := flag.Args()
 
 	conf := loader.Config{
-		Build:         &build.Default,
+		Build:            &build.Default,
-		SourceImports: !*importbinFlag,
+		ImportFromBinary: *importbinFlag,
 	}
 	// TODO(adonovan): make go/types choose its default Sizes from
 	// build.Default or a specified *build.Context.

go/callgraph/cha/cha_test.go

@@ -51,8 +51,7 @@ func TestCHA(t *testing.T) {
 		}
 
 		conf := loader.Config{
-			SourceImports: true,
+			ParserMode: parser.ParseComments,
-			ParserMode:    parser.ParseComments,
 		}
 		f, err := conf.ParseFile(filename, content)
 		if err != nil {

go/callgraph/rta/rta_test.go

@@ -55,8 +55,7 @@ func TestRTA(t *testing.T) {
 		}
 
 		conf := loader.Config{
-			SourceImports: true,
+			ParserMode: parser.ParseComments,
-			ParserMode:    parser.ParseComments,
 		}
 		f, err := conf.ParseFile(filename, content)
 		if err != nil {

go/loader/loader.go

@@ -234,24 +234,30 @@ type Config struct {
 	// checked.
 	TypeCheckFuncBodies func(string) bool
 
-	// SourceImports determines whether to satisfy dependencies by
+	// ImportFromBinary determines whether to satisfy dependencies by
-	// loading Go source code.
+	// loading gc export data instead of Go source code.
 	//
-	// If true, the entire program---the initial packages and
+	// If false, the entire program---the initial packages and their
-	// their transitive closure of dependencies---will be loaded,
+	// transitive closure of dependencies---will be loaded from
-	// parsed and type-checked.  This is required for
+	// source, parsed, and type-checked.  This is required for
 	// whole-program analyses such as pointer analysis.
 	//
-	// If false, the TypeChecker.Import mechanism will be used
+	// If true, the go/gcimporter mechanism is used instead to read
-	// instead.  Since that typically supplies only the types of
+	// the binary export-data files written by the gc toolchain.
-	// package-level declarations and values of constants, but no
+	// They supply only the types of package-level declarations and
-	// code, it will not yield a whole program.  It is intended
+	// values of constants, but no code, this option will not yield
-	// for analyses that perform modular analysis of a
+	// a whole program.  It is intended for analyses that perform
-	// single package, e.g. traditional compilation.
+	// modular analysis of a single package, e.g. traditional
+	// compilation.
+	//
+	// No check is made that the export data files are up-to-date.
 	//
 	// The initial packages (CreatePkgs and ImportPkgs) are always
 	// loaded from Go source, regardless of this flag's setting.
-	SourceImports bool
+	//
+	// NB: there is a bug when loading multiple initial packages with
+	// this flag enabled: https://github.com/golang/go/issues/9955.
+	ImportFromBinary bool
 
 	// If Build is non-nil, it is used to locate source packages.
 	// Otherwise &build.Default is used.
@@ -1031,7 +1037,7 @@ func (imp *importer) load(path string, ii *importInfo) {
 	var info *PackageInfo
 	var err error
 	// Find and create the actual package.
-	if _, ok := imp.conf.ImportPkgs[path]; ok || imp.conf.SourceImports {
+	if _, ok := imp.conf.ImportPkgs[path]; ok || !imp.conf.ImportFromBinary {
 		info, err = imp.loadFromSource(path)
 	} else {
 		info, err = imp.importFromBinary(path)

go/loader/loader_test.go

@@ -238,10 +238,7 @@ func TestLoad_MissingIndirectImport(t *testing.T) {
 		"a": `package a; import _ "b"`,
 		"b": `package b; import _ "c"`,
 	}
-	conf := loader.Config{
+	conf := loader.Config{Build: fakeContext(pkgs)}
-		SourceImports: true,
-		Build:         fakeContext(pkgs),
-	}
 	conf.Import("a")
 
 	const wantErr = "couldn't load packages due to errors: b"
@@ -292,9 +289,8 @@ func TestLoad_BadDependency_AllowErrors(t *testing.T) {
 		},
 	} {
 		conf := loader.Config{
-			AllowErrors:   true,
+			AllowErrors: true,
-			SourceImports: true,
+			Build:       fakeContext(test.pkgs),
-			Build:         fakeContext(test.pkgs),
 		}
 		conf.Import("a")
 
@@ -347,9 +343,8 @@ func TestTransitivelyErrorFreeFlag(t *testing.T) {
 		"e": `package e; import _ "d"`,
 	}
 	conf := loader.Config{
-		AllowErrors:   true,
+		AllowErrors: true,
-		SourceImports: true,
+		Build:       fakeContext(pkgs),
-		Build:         fakeContext(pkgs),
 	}
 	conf.Import("a")
 
@@ -398,9 +393,8 @@ func TestErrorReporting(t *testing.T) {
 		"b": `package b; 'syntax error!`,
 	}
 	conf := loader.Config{
-		AllowErrors:   true,
+		AllowErrors: true,
-		SourceImports: true,
+		Build:       fakeContext(pkgs),
-		Build:         fakeContext(pkgs),
 	}
 	var mu sync.Mutex
 	var allErrors []error
@@ -509,9 +503,8 @@ func TestCycles(t *testing.T) {
 		// },
 	} {
 		conf := loader.Config{
-			AllowErrors:   true,
+			AllowErrors: true,
-			SourceImports: true,
+			Build:       test.ctxt,
-			Build:         test.ctxt,
 		}
 		var mu sync.Mutex
 		var allErrors []error

go/pointer/analysis.go

@@ -255,7 +255,7 @@ func Analyze(config *Config) (result *Result, err error) {
 		// (This only checks that the package scope is complete,
 		// not that func bodies exist, but it's a good signal.)
 		if !pkg.Object.Complete() {
-			return nil, fmt.Errorf(`pointer analysis requires a complete program yet package %q was incomplete (set loader.Config.SourceImports during loading)`, pkg.Object.Path())
+			return nil, fmt.Errorf(`pointer analysis requires a complete program yet package %q was incomplete (don't set loader.Config.ImportFromBinary during loading)`, pkg.Object.Path())
 		}
 	}
 

go/pointer/example_test.go

@@ -41,8 +41,7 @@ func main() {
 	i.f(x) // dynamic method call
 }
 `
-	// Construct a loader.
+	var conf loader.Config
-	conf := loader.Config{SourceImports: true}
 
 	// Parse the input file, a string.
 	// (Command-line tools should use conf.FromArgs.)

go/pointer/pointer_test.go

@@ -153,7 +153,7 @@ func findProbe(prog *ssa.Program, probes map[*ssa.CallCommon]bool, queries map[s
 }
 
 func doOneInput(input, filename string) bool {
-	conf := loader.Config{SourceImports: true}
+	var conf loader.Config
 
 	// Parsing.
 	f, err := conf.ParseFile(filename, input)

go/pointer/stdlib_test.go

@@ -35,10 +35,7 @@ func TestStdlib(t *testing.T) {
 	// Load, parse and type-check the program.
 	ctxt := build.Default // copy
 	ctxt.GOPATH = ""      // disable GOPATH
-	conf := loader.Config{
+	conf := loader.Config{Build: &ctxt}
-		SourceImports: true,
-		Build:         &ctxt,
-	}
 	if _, err := conf.FromArgs(buildutil.AllPackages(conf.Build), true); err != nil {
 		t.Errorf("FromArgs failed: %v", err)
 		return

go/ssa/builder_test.go

@@ -21,7 +21,7 @@ func isEmpty(f *ssa.Function) bool { return f.Blocks == nil }
 
 // Tests that programs partially loaded from gc object files contain
 // functions with no code for the external portions, but are otherwise ok.
-func TestExternalPackages(t *testing.T) {
+func TestImportFromBinary(t *testing.T) {
 	test := `
 package main
 
@@ -43,7 +43,7 @@ func main() {
 `
 
 	// Create a single-file main package.
-	var conf loader.Config
+	conf := loader.Config{ImportFromBinary: true}
 	f, err := conf.ParseFile("<input>", test)
 	if err != nil {
 		t.Error(err)
@@ -212,7 +212,7 @@ func TestRuntimeTypes(t *testing.T) {
 	}
 	for _, test := range tests {
 		// Create a single-file main package.
-		var conf loader.Config
+		conf := loader.Config{ImportFromBinary: true}
 		f, err := conf.ParseFile("<input>", test.input)
 		if err != nil {
 			t.Errorf("test %q: %s", test.input[:15], err)

go/ssa/interp/interp_test.go

@@ -188,7 +188,7 @@ func run(t *testing.T, dir, input string, success successPredicate) bool {
 		inputs = append(inputs, i)
 	}
 
-	conf := loader.Config{SourceImports: true}
+	var conf loader.Config
 	if _, err := conf.FromArgs(inputs, true); err != nil {
 		t.Errorf("FromArgs(%s) failed: %s", inputs, err)
 		return false

go/ssa/stdlib_test.go

@@ -38,10 +38,7 @@ func TestStdlib(t *testing.T) {
 	// Load, parse and type-check the program.
 	ctxt := build.Default // copy
 	ctxt.GOPATH = ""      // disable GOPATH
-	conf := loader.Config{
+	conf := loader.Config{Build: &ctxt}
-		SourceImports: true,
-		Build:         &ctxt,
-	}
 	if _, err := conf.FromArgs(buildutil.AllPackages(conf.Build), true); err != nil {
 		t.Errorf("FromArgs failed: %v", err)
 		return

godoc/analysis/analysis.go

@@ -338,8 +338,7 @@ func (a *analysis) posURL(pos token.Pos, len int) string {
 //
 func Run(pta bool, result *Result) {
 	conf := loader.Config{
-		SourceImports: true,
+		AllowErrors: true,
-		AllowErrors:   true,
 	}
 
 	// Silence the default error handler.

oracle/oracle.go

@@ -193,7 +193,7 @@ func (res *Result) Serial() *serial.Result {
 // Clients that intend to perform multiple queries against the same
 // analysis scope should use this pattern instead:
 //
-//	conf := loader.Config{Build: buildContext, SourceImports: true}
+//	conf := loader.Config{Build: buildContext}
 //	... populate config, e.g. conf.FromArgs(args) ...
 //	iprog, err := conf.Load()
 //	if err != nil { ... }
@@ -223,7 +223,7 @@ func Query(args []string, mode, pos string, ptalog io.Writer, buildContext *buil
 		return nil, fmt.Errorf("invalid mode type: %q", mode)
 	}
 
-	conf := loader.Config{Build: buildContext, SourceImports: true}
+	conf := loader.Config{Build: buildContext}
 
 	// Determine initial packages.
 	args, err := conf.FromArgs(args, true)

oracle/oracle_test.go

@@ -272,7 +272,7 @@ func TestMultipleQueries(t *testing.T) {
 	// Loader
 	var buildContext = build.Default
 	buildContext.GOPATH = "testdata"
-	conf := loader.Config{Build: &buildContext, SourceImports: true}
+	conf := loader.Config{Build: &buildContext}
 	filename := "testdata/src/main/multi.go"
 	conf.CreateFromFilenames("", filename)
 	iprog, err := conf.Load()

refactor/eg/eg_test.go

@@ -36,9 +36,8 @@ func Test(t *testing.T) {
 	}
 
 	conf := loader.Config{
-		Fset:          token.NewFileSet(),
+		Fset:       token.NewFileSet(),
-		ParserMode:    parser.ParseComments,
+		ParserMode: parser.ParseComments,
-		SourceImports: true,
 	}
 
 	// Each entry is a single-file package.

refactor/lexical/lexical_test.go

@@ -32,10 +32,7 @@ func TestStdlib(t *testing.T) {
 		"golang.org/x/tools/refactor/lexical")
 
 	// Load, parse and type-check the program.
-	conf := loader.Config{
+	conf := loader.Config{Build: &ctxt}
-		Build:         &ctxt,
-		SourceImports: true,
-	}
 	for _, path := range pkgs {
 		conf.ImportWithTests(path)
 	}

refactor/rename/rename.go

@@ -339,9 +339,8 @@ func Main(ctxt *build.Context, offsetFlag, fromFlag, to string) error {
 // context.  Only packages in pkgs will have their functions bodies typechecked.
 func loadProgram(ctxt *build.Context, pkgs map[string]bool) (*loader.Program, error) {
 	conf := loader.Config{
-		Build:         ctxt,
+		Build:      ctxt,
-		SourceImports: true,
+		ParserMode: parser.ParseComments,
-		ParserMode:    parser.ParseComments,
 
 		// TODO(adonovan): enable this.  Requires making a lot of code more robust!
 		AllowErrors: false,