From 08fadac0710af0af980b537e051f474d23056a55 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 11 Feb 2014 16:52:16 -0500 Subject: [PATCH] go.tools/go/loader: simplify command-line syntax. Previously, each word could be a package import path or a comma-separated list of *.go file names. Now, if the first word ends with ".go", all words are assumed to be Go source files. This makes it impossible to specify two ad-hoc packages from source files, but no-one needs that. FromArgs also takes a boolean indicating whether tests are wanted or not. Also: ssadump: add -test flag to set that boolean. For the oracle it's always true. LGTM=gri R=gri CC=golang-codereviews https://golang.org/cl/61470047 --- cmd/ssadump/main.go | 78 ++++++++++++++++++++++++-------------- go/loader/importer_test.go | 31 ++++++++++++--- go/loader/loader.go | 64 +++++++++++++++++-------------- go/ssa/stdlib_test.go | 2 +- oracle/oracle.go | 2 +- 5 files changed, 112 insertions(+), 65 deletions(-) diff --git a/cmd/ssadump/main.go b/cmd/ssadump/main.go index 3009bb7a05..caa794cb45 100644 --- a/cmd/ssadump/main.go +++ b/cmd/ssadump/main.go @@ -9,7 +9,6 @@ import ( "flag" "fmt" "go/build" - "log" "os" "runtime" "runtime/pprof" @@ -32,6 +31,8 @@ L build distinct packages seria[L]ly instead of in parallel. N build [N]aive SSA form: don't replace local loads/stores with registers. `) +var testFlag = flag.Bool("test", false, "Loads test code (*_test.go) for imported packages.") + var runFlag = flag.Bool("run", false, "Invokes the SSA interpreter on the program.") var interpFlag = flag.String("interp", "", `Options controlling the SSA test interpreter. @@ -45,14 +46,15 @@ Usage: ssadump [ ...] ... Use -help flag to display options. Examples: -% ssadump -build=FPG hello.go # quickly dump SSA form of a single package -% ssadump -run -interp=T hello.go # interpret a program, with tracing -% ssadump -run unicode -- -test.v # interpret the unicode package's tests, verbosely +% ssadump -build=FPG hello.go # quickly dump SSA form of a single package +% ssadump -run -interp=T hello.go # interpret a program, with tracing +% ssadump -run -test unicode -- -test.v # interpret the unicode package's tests, verbosely ` + loader.FromArgsUsage + ` -When -run is specified, ssadump will find the first package that -defines a main function and run it in the interpreter. -If none is found, the tests of each package will be run instead. +When -run is specified, ssadump will run the program. +The entry point depends on the -test flag: +if clear, it runs the first package named main. +if set, it runs the tests of each package. ` var cpuprofile = flag.String("cpuprofile", "", "write cpu profile to file") @@ -70,6 +72,13 @@ func init() { } func main() { + if err := doMain(); err != nil { + fmt.Fprintf(os.Stderr, "ssadump: %s.\n", err) + os.Exit(1) + } +} + +func doMain() error { flag.Parse() args := flag.Args() @@ -109,7 +118,7 @@ func main() { case 'L': mode |= ssa.BuildSerially default: - log.Fatalf("Unknown -build option: '%c'.", c) + return fmt.Errorf("unknown -build option: '%c'", c) } } @@ -121,7 +130,8 @@ func main() { case 'R': interpMode |= interp.DisableRecover default: - log.Fatalf("Unknown -interp option: '%c'.", c) + fmt.Fprintf(os.Stderr, "ssadump: unknown -interp option: '%c'.", c) + os.Exit(1) } } @@ -134,16 +144,17 @@ func main() { if *cpuprofile != "" { f, err := os.Create(*cpuprofile) if err != nil { - log.Fatal(err) + fmt.Fprintln(os.Stderr, err) + os.Exit(1) } pprof.StartCPUProfile(f) defer pprof.StopCPUProfile() } // Use the initial packages from the command line. - args, err := conf.FromArgs(args) + args, err := conf.FromArgs(args, *testFlag) if err != nil { - log.Fatal(err) + return err } // The interpreter needs the runtime package. @@ -154,7 +165,7 @@ func main() { // Load, parse and type-check the whole program. iprog, err := conf.Load() if err != nil { - log.Fatal(err) + return err } // Create and build SSA-form program representation. @@ -163,29 +174,38 @@ func main() { // Run the interpreter. if *runFlag { - // If a package named "main" defines func main, run that. - // Otherwise run all packages' tests. var main *ssa.Package pkgs := prog.AllPackages() - for _, pkg := range pkgs { - if pkg.Object.Name() == "main" && pkg.Func("main") != nil { - main = pkg - break + if *testFlag { + // If -test, run all packages' tests. + if len(pkgs) > 0 { + main = prog.CreateTestMainPackage(pkgs...) + } + if main == nil { + return fmt.Errorf("no tests") + } + } else { + // Otherwise, run main.main. + for _, pkg := range pkgs { + if pkg.Object.Name() == "main" { + main = pkg + if main.Func("main") == nil { + return fmt.Errorf("no func main() in main package") + } + break + } + } + if main == nil { + return fmt.Errorf("no main package") } } - if main == nil && len(pkgs) > 0 { - // TODO(adonovan): only run tests if -test flag specified. - main = prog.CreateTestMainPackage(pkgs...) - } - if main == nil { - log.Fatal("No main package and no tests") - } - if runtime.GOARCH != conf.Build.GOARCH { - log.Fatalf("Cross-interpretation is not yet supported (target has GOARCH %s, interpreter has %s).", - conf.Build.GOARCH, runtime.GOARCH) + if runtime.GOARCH != build.Default.GOARCH { + return fmt.Errorf("cross-interpretation is not yet supported (target has GOARCH %s, interpreter has %s)", + build.Default.GOARCH, runtime.GOARCH) } interp.Interpret(main, interpMode, conf.TypeChecker.Sizes, main.Object.Path(), args) } + return nil } diff --git a/go/loader/importer_test.go b/go/loader/importer_test.go index be6a4704b5..54aec5b0f5 100644 --- a/go/loader/importer_test.go +++ b/go/loader/importer_test.go @@ -14,7 +14,7 @@ import ( func loadFromArgs(args []string) (prog *loader.Program, rest []string, err error) { conf := &loader.Config{} - rest, err = conf.FromArgs(args) + rest, err = conf.FromArgs(args, true) if err == nil { prog, err = conf.Load() } @@ -39,11 +39,10 @@ func TestLoadFromArgs(t *testing.T) { } // Successful load. - args = []string{"fmt", "errors", "testdata/a.go,testdata/b.go", "--", "surplus"} + args = []string{"fmt", "errors", "--", "surplus"} prog, rest, err := loadFromArgs(args) if err != nil { - t.Errorf("loadFromArgs(%q) failed: %s", args, err) - return + t.Fatalf("loadFromArgs(%q) failed: %s", args, err) } if got, want := fmt.Sprint(rest), "[surplus]"; got != want { t.Errorf("loadFromArgs(%q) rest: got %s, want %s", args, got, want) @@ -54,7 +53,7 @@ func TestLoadFromArgs(t *testing.T) { pkgnames = append(pkgnames, info.Pkg.Path()) } // Only the first import path (currently) contributes tests. - if got, want := fmt.Sprint(pkgnames), "[fmt_test P]"; got != want { + if got, want := fmt.Sprint(pkgnames), "[fmt_test]"; got != want { t.Errorf("Created: got %s, want %s", got, want) } @@ -82,3 +81,25 @@ func TestLoadFromArgs(t *testing.T) { } } } + +func TestLoadFromArgsSource(t *testing.T) { + // mixture of *.go/non-go. + args := []string{"testdata/a.go", "fmt"} + prog, _, err := loadFromArgs(args) + if err == nil { + t.Errorf("loadFromArgs(%q) succeeded, want failure", args) + } else { + // "named files must be .go files: fmt": ok + } + + // successful load + args = []string{"testdata/a.go", "testdata/b.go"} + prog, _, err = loadFromArgs(args) + if err != nil { + t.Errorf("loadFromArgs(%q) failed: %s", args, err) + return + } + if len(prog.Created) != 1 || prog.Created[0].Pkg.Path() != "P" { + t.Errorf("loadFromArgs(%q): got %v, want [P]", prog.Created) + } +} diff --git a/go/loader/loader.go b/go/loader/loader.go index 06c260315b..3fcd7f8764 100644 --- a/go/loader/loader.go +++ b/go/loader/loader.go @@ -23,7 +23,7 @@ // // Use the command-line arguments to specify // // a set of initial packages to load from source. // // See FromArgsUsage for help. -// rest, err := conf.FromArgs(os.Args[1:]) +// rest, err := conf.FromArgs(os.Args[1:], wantTests) // // // Parse the specified files and create an ad-hoc package with path "foo". // // All files must have the same 'package' declaration. @@ -228,15 +228,14 @@ func (conf *Config) ParseFile(filename string, src interface{}, mode parser.Mode // FromArgs may wish to include in their -help output. const FromArgsUsage = ` is a list of arguments denoting a set of initial packages. -Each argument may take one of two forms: +It may take one of two forms: -1. A comma-separated list of *.go source files. +1. A list of *.go source files. All of the specified files are loaded, parsed and type-checked - as a single package. - All the files must belong to the same directory. + as a single package. All the files must belong to the same directory. -2. An import path. +2. A list of import paths, each denoting a package. The package's directory is found relative to the $GOROOT and $GOPATH using similar logic to 'go build', and the *.go files in @@ -248,9 +247,8 @@ Each argument may take one of two forms: the non-*_test.go files are included in the primary package. Test files whose package declaration ends with "_test" are type-checked as another package, the 'external' test package, so that a single - import path may denote two packages. This behaviour may be - disabled by prefixing the import path with "notest:", - e.g. "notest:fmt". + import path may denote two packages. (Whether this behaviour is + enabled is tool-specific, and may depend on additional flags.) Due to current limitations in the type-checker, only the first import path of the command line will contribute any tests. @@ -266,33 +264,40 @@ A '--' argument terminates the list of packages. // set of initial packages to be specified; see FromArgsUsage message // for details. // -func (conf *Config) FromArgs(args []string) (rest []string, err error) { - for len(args) > 0 { - arg := args[0] - args = args[1:] +func (conf *Config) FromArgs(args []string, xtest bool) (rest []string, err error) { + for i, arg := range args { if arg == "--" { + rest = args[i+1:] + args = args[:i] break // consume "--" and return the remaining args } + } - if strings.HasSuffix(arg, ".go") { - // Assume arg is a comma-separated list of *.go files - // denoting a single ad-hoc package. - err = conf.CreateFromFilenames("", strings.Split(arg, ",")...) - } else { - // Assume arg is a directory name denoting a - // package, perhaps plus an external test - // package unless prefixed by "notest:". - if path := strings.TrimPrefix(arg, "notest:"); path != arg { - conf.Import(path) - } else { - err = conf.ImportWithTests(path) + if len(args) > 0 && strings.HasSuffix(args[0], ".go") { + // Assume args is a list of a *.go files + // denoting a single ad-hoc package. + for _, arg := range args { + if !strings.HasSuffix(arg, ".go") { + return nil, fmt.Errorf("named files must be .go files: %s", arg) } } - if err != nil { - return nil, err + err = conf.CreateFromFilenames("", args...) + } else { + // Assume args are directories each denoting a + // package and (perhaps) an external test, iff xtest. + for _, arg := range args { + if xtest { + err = conf.ImportWithTests(arg) + if err != nil { + break + } + } else { + conf.Import(arg) + } } } - return args, nil + + return } // CreateFromFilenames is a convenience function that parses the @@ -371,7 +376,8 @@ func (conf *Config) Import(path string) { if conf.ImportPkgs == nil { conf.ImportPkgs = make(map[string]bool) } - conf.ImportPkgs[path] = false // unaugmented source package + // Subtle: adds value 'false' unless value is already true. + conf.ImportPkgs[path] = conf.ImportPkgs[path] // unaugmented source package } // PathEnclosingInterval returns the PackageInfo and ast.Node that diff --git a/go/ssa/stdlib_test.go b/go/ssa/stdlib_test.go index e3ae81cef2..b09be7e989 100644 --- a/go/ssa/stdlib_test.go +++ b/go/ssa/stdlib_test.go @@ -53,7 +53,7 @@ func TestStdlib(t *testing.T) { t0 := time.Now() var conf loader.Config - if _, err := conf.FromArgs(allPackages()); err != nil { + if _, err := conf.FromArgs(allPackages(), true); err != nil { t.Errorf("FromArgs failed: %v", err) return } diff --git a/oracle/oracle.go b/oracle/oracle.go index 9b25ae9064..9625344529 100644 --- a/oracle/oracle.go +++ b/oracle/oracle.go @@ -225,7 +225,7 @@ func Query(args []string, mode, pos string, ptalog io.Writer, buildContext *buil conf := loader.Config{Build: buildContext, SourceImports: true} // Determine initial packages. - args, err := conf.FromArgs(args) + args, err := conf.FromArgs(args, true) if err != nil { return nil, err }