Index: llgo/trunk/irgen/compiler.go =================================================================== --- llgo/trunk/irgen/compiler.go +++ llgo/trunk/irgen/compiler.go @@ -216,8 +216,9 @@ Sizes: compiler.llvmtypes, DisableUnusedImportCheck: compiler.DisableUnusedImportCheck, }, - Build: &buildctx.Context, - PackageCreated: compiler.PackageCreated, + ImportFromBinary: true, + 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. Index: llgo/trunk/third_party/gofrontend/config.sub =================================================================== --- llgo/trunk/third_party/gofrontend/config.sub +++ llgo/trunk/third_party/gofrontend/config.sub @@ -1354,7 +1354,7 @@ | -os2* | -vos* | -palmos* | -uclinux* | -nucleus* \ | -morphos* | -superux* | -rtmk* | -rtmk-nova* | -windiss* \ | -powermax* | -dnix* | -nx6 | -nx7 | -sei* | -dragonfly* \ - | -skyos* | -haiku* | -rdos* | -toppers* | -drops* | -es*) + | -skyos* | -haiku* | -rdos* | -toppers* | -drops* | -es* | -bitrig*) # Remember, each alternative MUST END IN *, to match a version number. ;; -qnx*) Index: llgo/trunk/third_party/gotools/go/ast/astutil/imports.go =================================================================== --- llgo/trunk/third_party/gotools/go/ast/astutil/imports.go +++ llgo/trunk/third_party/gotools/go/ast/astutil/imports.go @@ -308,13 +308,15 @@ return false } -// matchLen returns the length of the longest prefix shared by x and y. +// matchLen returns the length of the longest path segment prefix shared by x and y. func matchLen(x, y string) int { - i := 0 - for i < len(x) && i < len(y) && x[i] == y[i] { - i++ + n := 0 + for i := 0; i < len(x) && i < len(y) && x[i] == y[i]; i++ { + if x[i] == '/' { + n++ + } } - return i + return n } // isTopName returns true if n is a top-level unresolved identifier with the given name. Index: llgo/trunk/third_party/gotools/go/ast/astutil/imports_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/ast/astutil/imports_test.go +++ llgo/trunk/third_party/gotools/go/ast/astutil/imports_test.go @@ -315,6 +315,31 @@ type T time.Time `, }, + + // Issue 9961: Match prefixes using path segments rather than bytes + { + name: "issue 9961", + pkg: "regexp", + in: `package main + +import ( + "flag" + "testing" + + "rsc.io/p" +) +`, + out: `package main + +import ( + "flag" + "regexp" + "testing" + + "rsc.io/p" +) +`, + }, } func TestAddImport(t *testing.T) { Index: llgo/trunk/third_party/gotools/go/buildutil/allpackages.go =================================================================== --- llgo/trunk/third_party/gotools/go/buildutil/allpackages.go +++ llgo/trunk/third_party/gotools/go/buildutil/allpackages.go @@ -30,11 +30,8 @@ // func AllPackages(ctxt *build.Context) []string { var list []string - var mu sync.Mutex ForEachPackage(ctxt, func(pkg string, _ error) { - mu.Lock() list = append(list, pkg) - mu.Unlock() }) sort.Strings(list) return list @@ -47,27 +44,42 @@ // If the package directory exists but could not be read, the second // argument to the found function provides the error. // -// The found function and the build.Context file system interface -// accessors must be concurrency safe. +// All I/O is done via the build.Context file system interface, +// which must be concurrency-safe. // func ForEachPackage(ctxt *build.Context, found func(importPath string, err error)) { // We use a counting semaphore to limit // the number of parallel calls to ReadDir. sema := make(chan bool, 20) + ch := make(chan item) + var wg sync.WaitGroup for _, root := range ctxt.SrcDirs() { root := root wg.Add(1) go func() { - allPackages(ctxt, sema, root, found) + allPackages(ctxt, sema, root, ch) wg.Done() }() } - wg.Wait() + go func() { + wg.Wait() + close(ch) + }() + + // All calls to found occur in the caller's goroutine. + for i := range ch { + found(i.importPath, i.err) + } +} + +type item struct { + importPath string + err error // (optional) } -func allPackages(ctxt *build.Context, sema chan bool, root string, found func(string, error)) { +func allPackages(ctxt *build.Context, sema chan bool, root string, ch chan<- item) { root = filepath.Clean(root) + string(os.PathSeparator) var wg sync.WaitGroup @@ -92,7 +104,7 @@ files, err := ReadDir(ctxt, dir) <-sema if pkg != "" || err != nil { - found(pkg, err) + ch <- item{pkg, err} } for _, fi := range files { fi := fi Index: llgo/trunk/third_party/gotools/go/buildutil/fakecontext.go =================================================================== --- llgo/trunk/third_party/gotools/go/buildutil/fakecontext.go +++ llgo/trunk/third_party/gotools/go/buildutil/fakecontext.go @@ -0,0 +1,108 @@ +package buildutil + +import ( + "fmt" + "go/build" + "io" + "io/ioutil" + "os" + "path" + "path/filepath" + "sort" + "strings" + "time" +) + +// FakeContext returns a build.Context for the fake file tree specified +// by pkgs, which maps package import paths to a mapping from file base +// names to contents. +// +// The fake Context has a GOROOT of "/go" and no GOPATH, and overrides +// the necessary file access methods to read from memory instead of the +// real file system. +// +// Unlike a real file tree, the fake one has only two levels---packages +// and files---so ReadDir("/go/src/") returns all packages under +// /go/src/ including, for instance, "math" and "math/big". +// ReadDir("/go/src/math/big") would return all the files in the +// "math/big" package. +// +func FakeContext(pkgs map[string]map[string]string) *build.Context { + clean := func(filename string) string { + f := path.Clean(filepath.ToSlash(filename)) + // Removing "/go/src" while respecting segment + // boundaries has this unfortunate corner case: + if f == "/go/src" { + return "" + } + return strings.TrimPrefix(f, "/go/src/") + } + + ctxt := build.Default // copy + ctxt.GOROOT = "/go" + ctxt.GOPATH = "" + ctxt.IsDir = func(dir string) bool { + dir = clean(dir) + if dir == "" { + return true // needed by (*build.Context).SrcDirs + } + return pkgs[dir] != nil + } + ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { + dir = clean(dir) + var fis []os.FileInfo + if dir == "" { + // enumerate packages + for importPath := range pkgs { + fis = append(fis, fakeDirInfo(importPath)) + } + } else { + // enumerate files of package + for basename := range pkgs[dir] { + fis = append(fis, fakeFileInfo(basename)) + } + } + sort.Sort(byName(fis)) + return fis, nil + } + ctxt.OpenFile = func(filename string) (io.ReadCloser, error) { + filename = clean(filename) + dir, base := path.Split(filename) + content, ok := pkgs[path.Clean(dir)][base] + if !ok { + return nil, fmt.Errorf("file not found: %s", filename) + } + return ioutil.NopCloser(strings.NewReader(content)), nil + } + ctxt.IsAbsPath = func(path string) bool { + path = filepath.ToSlash(path) + // Don't rely on the default (filepath.Path) since on + // Windows, it reports virtual paths as non-absolute. + return strings.HasPrefix(path, "/") + } + return &ctxt +} + +type byName []os.FileInfo + +func (s byName) Len() int { return len(s) } +func (s byName) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s byName) Less(i, j int) bool { return s[i].Name() < s[j].Name() } + +type fakeFileInfo string + +func (fi fakeFileInfo) Name() string { return string(fi) } +func (fakeFileInfo) Sys() interface{} { return nil } +func (fakeFileInfo) ModTime() time.Time { return time.Time{} } +func (fakeFileInfo) IsDir() bool { return false } +func (fakeFileInfo) Size() int64 { return 0 } +func (fakeFileInfo) Mode() os.FileMode { return 0644 } + +type fakeDirInfo string + +func (fd fakeDirInfo) Name() string { return string(fd) } +func (fakeDirInfo) Sys() interface{} { return nil } +func (fakeDirInfo) ModTime() time.Time { return time.Time{} } +func (fakeDirInfo) IsDir() bool { return true } +func (fakeDirInfo) Size() int64 { return 0 } +func (fakeDirInfo) Mode() os.FileMode { return 0755 } Index: llgo/trunk/third_party/gotools/go/callgraph/cha/cha_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/callgraph/cha/cha_test.go +++ llgo/trunk/third_party/gotools/go/callgraph/cha/cha_test.go @@ -51,8 +51,7 @@ } conf := loader.Config{ - SourceImports: true, - ParserMode: parser.ParseComments, + ParserMode: parser.ParseComments, } f, err := conf.ParseFile(filename, content) if err != nil { Index: llgo/trunk/third_party/gotools/go/callgraph/rta/rta_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/callgraph/rta/rta_test.go +++ llgo/trunk/third_party/gotools/go/callgraph/rta/rta_test.go @@ -55,8 +55,7 @@ } conf := loader.Config{ - SourceImports: true, - ParserMode: parser.ParseComments, + ParserMode: parser.ParseComments, } f, err := conf.ParseFile(filename, content) if err != nil { Index: llgo/trunk/third_party/gotools/go/exact/go13.go =================================================================== --- llgo/trunk/third_party/gotools/go/exact/go13.go +++ llgo/trunk/third_party/gotools/go/exact/go13.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// +build !go1.4 package exact Index: llgo/trunk/third_party/gotools/go/exact/go14.go =================================================================== --- llgo/trunk/third_party/gotools/go/exact/go14.go +++ llgo/trunk/third_party/gotools/go/exact/go14.go @@ -0,0 +1,13 @@ +// Copyright 2014 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. + +// +build go1.4 + +package exact + +import "math/big" + +func ratToFloat32(x *big.Rat) (float32, bool) { + return x.Float32() +} Index: llgo/trunk/third_party/gotools/go/gccgoimporter/importer_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/gccgoimporter/importer_test.go +++ llgo/trunk/third_party/gotools/go/gccgoimporter/importer_test.go @@ -95,7 +95,8 @@ {pkgpath: "complexnums", name: "NP", want: "const NP untyped complex", wantval: "(-1/1 + 1/1i)"}, {pkgpath: "complexnums", name: "PN", want: "const PN untyped complex", wantval: "(1/1 + -1/1i)"}, {pkgpath: "complexnums", name: "PP", want: "const PP untyped complex", wantval: "(1/1 + 1/1i)"}, - {pkgpath: "imports", wantinits: []string{"imports..import", "fmt..import", "math..import"}}, + // TODO: enable this entry once bug has been tracked down + //{pkgpath: "imports", wantinits: []string{"imports..import", "fmt..import", "math..import"}}, } func TestGoxImporter(t *testing.T) { Index: llgo/trunk/third_party/gotools/go/loader/loader.go =================================================================== --- llgo/trunk/third_party/gotools/go/loader/loader.go +++ llgo/trunk/third_party/gotools/go/loader/loader.go @@ -27,19 +27,19 @@ // // // Parse the specified files and create an ad-hoc package with path "foo". // // All files must have the same 'package' declaration. -// err := conf.CreateFromFilenames("foo", "foo.go", "bar.go") +// conf.CreateFromFilenames("foo", "foo.go", "bar.go") // // // Create an ad-hoc package with path "foo" from // // the specified already-parsed files. // // All ASTs must have the same 'package' declaration. -// err := conf.CreateFromFiles("foo", parsedFiles) +// conf.CreateFromFiles("foo", parsedFiles) // // // Add "runtime" to the set of packages to be loaded. // conf.Import("runtime") // // // Adds "fmt" and "fmt_test" to the set of packages // // to be loaded. "fmt" will include *_test.go files. -// err := conf.ImportWithTests("fmt") +// conf.ImportWithTests("fmt") // // // Finally, load all the packages specified by the configuration. // prog, err := conf.Load() @@ -73,7 +73,7 @@ // DEPENDENCY is a package loaded to satisfy an import in an initial // package or another dependency. // -package loader // import "llvm.org/llgo/third_party/gotools/go/loader" +package loader // 'go test', in-package test files, and import cycles // --------------------------------------------------- @@ -112,19 +112,79 @@ // compress/bzip2/bzip2_test.go (package bzip2) imports "io/ioutil" // io/ioutil/tempfile_test.go (package ioutil) imports "regexp" // regexp/exec_test.go (package regexp) imports "compress/bzip2" +// +// +// Concurrency +// ----------- +// +// Let us define the import dependency graph as follows. Each node is a +// list of files passed to (Checker).Files at once. Many of these lists +// are the production code of an importable Go package, so those nodes +// are labelled by the package's import path. The remaining nodes are +// ad-hoc packages and lists of in-package *_test.go files that augment +// an importable package; those nodes have no label. +// +// The edges of the graph represent import statements appearing within a +// file. An edge connects a node (a list of files) to the node it +// imports, which is importable and thus always labelled. +// +// Loading is controlled by this dependency graph. +// +// To reduce I/O latency, we start loading a package's dependencies +// asynchronously as soon as we've parsed its files and enumerated its +// imports (scanImports). This performs a preorder traversal of the +// import dependency graph. +// +// To exploit hardware parallelism, we type-check unrelated packages in +// parallel, where "unrelated" means not ordered by the partial order of +// the import dependency graph. +// +// We use a concurrency-safe blocking cache (importer.imported) to +// record the results of type-checking, whether success or failure. An +// entry is created in this cache by startLoad the first time the +// package is imported. The first goroutine to request an entry becomes +// responsible for completing the task and broadcasting completion to +// subsequent requestors, which block until then. +// +// Type checking occurs in (parallel) postorder: we cannot type-check a +// set of files until we have loaded and type-checked all of their +// immediate dependencies (and thus all of their transitive +// dependencies). If the input were guaranteed free of import cycles, +// this would be trivial: we could simply wait for completion of the +// dependencies and then invoke the typechecker. +// +// But as we saw in the 'go test' section above, some cycles in the +// import graph over packages are actually legal, so long as the +// cycle-forming edge originates in the in-package test files that +// augment the package. This explains why the nodes of the import +// dependency graph are not packages, but lists of files: the unlabelled +// nodes avoid the cycles. Consider packages A and B where B imports A +// and A's in-package tests AT import B. The naively constructed import +// graph over packages would contain a cycle (A+AT) --> B --> (A+AT) but +// the graph over lists of files is AT --> B --> A, where AT is an +// unlabelled node. +// +// Awaiting completion of the dependencies in a cyclic graph would +// deadlock, so we must materialize the import dependency graph (as +// importer.graph) and check whether each import edge forms a cycle. If +// x imports y, and the graph already contains a path from y to x, then +// there is an import cycle, in which case the processing of x must not +// wait for the completion of processing of y. +// +// When the type-checker makes a callback (doImport) to the loader for a +// given import edge, there are two possible cases. In the normal case, +// the dependency has already been completely type-checked; doImport +// does a cache lookup and returns it. In the cyclic case, the entry in +// the cache is still necessarily incomplete, indicating a cycle. We +// perform the cycle check again to obtain the error message, and return +// the error. +// +// The result of using concurrency is about a 2.5x speedup for stdlib_test. // TODO(adonovan): -// - (*Config).ParseFile is very handy, but feels like feature creep. -// (*Config).CreateFromFiles has a nasty precondition. -// - s/path/importPath/g to avoid ambiguity with other meanings of -// "path": a file name, a colon-separated directory list. // - cache the calls to build.Import so we don't do it three times per // test package. // - Thorough overhaul of package documentation. -// - Certain errors (e.g. parse error in x_test.go files, or failure to -// import an initial package) still cause Load() to fail hard. -// Fix that. (It's tricky because of the way x_test files are parsed -// eagerly.) import ( "errors" @@ -134,18 +194,23 @@ "go/parser" "go/token" "os" + "sort" "strings" + "sync" + "time" "llvm.org/llgo/third_party/gotools/go/ast/astutil" "llvm.org/llgo/third_party/gotools/go/gcimporter" "llvm.org/llgo/third_party/gotools/go/types" ) +const trace = false // show timing info for type-checking + // Config specifies the configuration for a program to load. // The zero value for Config is a ready-to-use default configuration. type Config struct { // Fset is the file set for the parser to use when loading the - // program. If nil, it will be lazily initialized by any + // program. If nil, it may be lazily initialized by any // method of Config. Fset *token.FileSet @@ -169,24 +234,30 @@ // checked. TypeCheckFuncBodies func(string) bool - // SourceImports determines whether to satisfy dependencies by - // loading Go source code. + // ImportFromBinary determines whether to satisfy dependencies by + // loading gc export data instead of Go source code. // - // If true, the entire program---the initial packages and - // their transitive closure of dependencies---will be loaded, - // parsed and type-checked. This is required for + // If false, the entire program---the initial packages and their + // transitive closure of dependencies---will be loaded from + // 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 - // instead. Since that typically supplies only the types of - // package-level declarations and values of constants, but no - // code, it will not yield a whole program. It is intended - // for analyses that perform modular analysis of a - // single package, e.g. traditional compilation. + // If true, the go/gcimporter mechanism is used instead to read + // the binary export-data files written by the gc toolchain. + // They supply only the types of package-level declarations and + // values of constants, but no code, this option will not yield + // a whole program. It is intended for analyses that perform + // 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. @@ -197,6 +268,11 @@ // to startup, or by setting Build.CgoEnabled=false. Build *build.Context + // The current directory, used for resolving relative package + // references such as "./go/loader". If empty, os.Getwd will be + // used instead. + Cwd string + // If DisplayPath is non-nil, it is used to transform each // file name obtained from Build.Import(). This can be used // to prevent a virtualized build.Config's file names from @@ -210,23 +286,30 @@ AllowErrors bool // CreatePkgs specifies a list of non-importable initial - // packages to create. Each element specifies a list of - // parsed files to be type-checked into a new package, and a - // path for that package. If the path is "", the package's - // name will be used instead. The path needn't be globally - // unique. - // - // The resulting packages will appear in the corresponding - // elements of the Program.Created slice. - CreatePkgs []CreatePkg + // packages to create. The resulting packages will appear in + // the corresponding elements of the Program.Created slice. + CreatePkgs []PkgSpec // ImportPkgs specifies a set of initial packages to load from // source. The map keys are package import paths, used to - // locate the package relative to $GOROOT. The corresponding - // values indicate whether to augment the package by *_test.go - // files in a second pass. + // locate the package relative to $GOROOT. + // + // The map value indicates whether to load tests. If true, Load + // will add and type-check two lists of files to the package: + // non-test files followed by in-package *_test.go files. In + // addition, it will append the external test package (if any) + // to Program.Created. ImportPkgs map[string]bool + // FindPackage is called during Load to create the build.Package + // for a given import path. If nil, a default implementation + // based on ctxt.Import is used. A client may use this hook to + // adapt to a proprietary build system that does not follow the + // "go build" layout conventions, for example. + // + // It must be safe to call concurrently from multiple goroutines. + FindPackage func(ctxt *build.Context, importPath string) (*build.Package, error) + // PackageCreated is a hook called when a types.Package // is created but before it has been populated. // @@ -241,9 +324,14 @@ PackageCreated func(*types.Package) } -type CreatePkg struct { - Path string - Files []*ast.File +// A PkgSpec specifies a non-importable package to be created by Load. +// Files are processed first, but typically only one of Files and +// Filenames is provided. The path needn't be globally unique. +// +type PkgSpec struct { + Path string // import path ("" => use package declaration) + Files []*ast.File // ASTs of already-parsed files + Filenames []string // names of files to be parsed } // A Program is a Go program loaded from source or binary @@ -251,8 +339,10 @@ type Program struct { Fset *token.FileSet // the file set for this program - // Created[i] contains the initial package whose ASTs were - // supplied by Config.CreatePkgs[i]. + // Created[i] contains the initial package whose ASTs or + // filenames were supplied by Config.CreatePkgs[i], followed by + // the external test package, if any, of each package in + // Config.ImportPkgs ordered by ImportPath. Created []*PackageInfo // Imported contains the initially imported packages, @@ -306,8 +396,12 @@ return conf.Fset } -// ParseFile is a convenience function that invokes the parser using -// the Config's FileSet, which is initialized if nil. +// ParseFile is a convenience function (intended for testing) that invokes +// the parser using the Config's FileSet, which is initialized if nil. +// +// src specifies the parser input as a string, []byte, or io.Reader, and +// filename is its apparent name. If src is nil, the contents of +// filename are read from the file system. // func (conf *Config) ParseFile(filename string, src interface{}) (*ast.File, error) { // TODO(adonovan): use conf.build() etc like parseFiles does. @@ -340,9 +434,6 @@ 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. - A '--' argument terminates the list of packages. ` @@ -354,7 +445,11 @@ // set of initial packages to be specified; see FromArgsUsage message // for details. // -func (conf *Config) FromArgs(args []string, xtest bool) (rest []string, err error) { +// Only superficial errors are reported at this stage; errors dependent +// on I/O are detected during Load. +// +func (conf *Config) FromArgs(args []string, xtest bool) ([]string, error) { + var rest []string for i, arg := range args { if arg == "--" { rest = args[i+1:] @@ -371,51 +466,35 @@ return nil, fmt.Errorf("named files must be .go files: %s", arg) } } - err = conf.CreateFromFilenames("", args...) + 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 - } + conf.ImportWithTests(arg) } else { conf.Import(arg) } } } - return + return rest, nil } -// CreateFromFilenames is a convenience function that parses the -// specified *.go files and adds a package entry for them to -// conf.CreatePkgs. +// CreateFromFilenames is a convenience function that adds +// a conf.CreatePkgs entry to create a package of the specified *.go +// files. // -// It fails if any file could not be loaded or parsed. -// -func (conf *Config) CreateFromFilenames(path string, filenames ...string) error { - files, errs := parseFiles(conf.fset(), conf.build(), nil, ".", filenames, conf.ParserMode) - if len(errs) > 0 { - return errs[0] - } - conf.CreateFromFiles(path, files...) - return nil +func (conf *Config) CreateFromFilenames(path string, filenames ...string) { + conf.CreatePkgs = append(conf.CreatePkgs, PkgSpec{Path: path, Filenames: filenames}) } -// CreateFromFiles is a convenience function that adds a CreatePkgs +// CreateFromFiles is a convenience function that adds a conf.CreatePkgs // entry to create package of the specified path and parsed files. // -// Precondition: conf.Fset is non-nil and was the fileset used to parse -// the files. (e.g. the files came from conf.ParseFile().) -// func (conf *Config) CreateFromFiles(path string, files ...*ast.File) { - if conf.Fset == nil { - panic("nil Fset") - } - conf.CreatePkgs = append(conf.CreatePkgs, CreatePkg{path, files}) + conf.CreatePkgs = append(conf.CreatePkgs, PkgSpec{Path: path, Files: files}) } // ImportWithTests is a convenience function that adds path to @@ -428,45 +507,21 @@ // declaration, an additional package comprising just those files will // be added to CreatePkgs. // -func (conf *Config) ImportWithTests(path string) error { - if path == "unsafe" { - return nil // ignore; not a real package - } - conf.Import(path) - - // Load the external test package. - bp, err := conf.findSourcePackage(path) - if err != nil { - return err // package not found - } - xtestFiles, errs := conf.parsePackageFiles(bp, 'x') - if len(errs) > 0 { - // TODO(adonovan): fix: parse errors in x_test.go files - // cause FromArgs() to fail completely. - return errs[0] // I/O or parse error - } - if len(xtestFiles) > 0 { - conf.CreateFromFiles(path+"_test", xtestFiles...) - } - - // Mark the non-xtest package for augmentation with - // in-package *_test.go files when we import it below. - conf.ImportPkgs[path] = true - return nil -} +func (conf *Config) ImportWithTests(path string) { conf.addImport(path, true) } // Import is a convenience function that adds path to ImportPkgs, the // set of initial packages that will be imported from source. // -func (conf *Config) Import(path string) { +func (conf *Config) Import(path string) { conf.addImport(path, false) } + +func (conf *Config) addImport(path string, tests bool) { if path == "unsafe" { return // ignore; not a real package } if conf.ImportPkgs == nil { conf.ImportPkgs = make(map[string]bool) } - // Subtle: adds value 'false' unless value is already true. - conf.ImportPkgs[path] = conf.ImportPkgs[path] // unaugmented source package + conf.ImportPkgs[path] = conf.ImportPkgs[path] || tests } // PathEnclosingInterval returns the PackageInfo and ast.Node that @@ -506,15 +561,63 @@ // importer holds the working state of the algorithm. type importer struct { - conf *Config // the client configuration - prog *Program // resulting program - imported map[string]*importInfo // all imported packages (incl. failures) by import path + conf *Config // the client configuration + prog *Program // resulting program + start time.Time // for logging + + // This mutex serializes access to prog.ImportMap (aka + // TypeChecker.Packages); we also use it for AllPackages. + // + // The TypeChecker.Packages map is not really used by this + // package, but may be used by the client's Import function, + // and by clients of the returned Program. + typecheckerMu sync.Mutex + + importedMu sync.Mutex + imported map[string]*importInfo // all imported packages (incl. failures) by import path + + // import dependency graph: graph[x][y] => x imports y + // + // Since non-importable packages cannot be cyclic, we ignore + // their imports, thus we only need the subgraph over importable + // packages. Nodes are identified by their import paths. + graphMu sync.Mutex + graph map[string]map[string]bool } // importInfo tracks the success or failure of a single import. +// +// Upon completion, exactly one of info and err is non-nil: +// info on successful creation of a package, err otherwise. +// A successful package may still contain type errors. +// type importInfo struct { - info *PackageInfo // results of typechecking (including errors) - err error // reason for failure to make a package + path string // import path + mu sync.Mutex // guards the following fields prior to completion + info *PackageInfo // results of typechecking (including errors) + err error // reason for failure to create a package + complete sync.Cond // complete condition is that one of info, err is non-nil. +} + +// awaitCompletion blocks until ii is complete, +// i.e. the info and err fields are safe to inspect without a lock. +// It is concurrency-safe and idempotent. +func (ii *importInfo) awaitCompletion() { + ii.mu.Lock() + for ii.info == nil && ii.err == nil { + ii.complete.Wait() + } + ii.mu.Unlock() +} + +// Complete marks ii as complete. +// Its info and err fields will not be subsequently updated. +func (ii *importInfo) Complete(info *PackageInfo, err error) { + ii.mu.Lock() + ii.info = info + ii.err = err + ii.complete.Broadcast() + ii.mu.Unlock() } // Load creates the initial packages specified by conf.{Create,Import}Pkgs, @@ -542,6 +645,26 @@ conf.TypeChecker.Error = func(e error) { fmt.Fprintln(os.Stderr, e) } } + // Set default working directory for relative package references. + if conf.Cwd == "" { + var err error + conf.Cwd, err = os.Getwd() + if err != nil { + return nil, err + } + } + + // Install default FindPackage hook using go/build logic. + if conf.FindPackage == nil { + conf.FindPackage = func(ctxt *build.Context, path string) (*build.Package, error) { + bp, err := ctxt.Import(path, conf.Cwd, 0) + if _, ok := err.(*build.NoGoError); ok { + return bp, nil // empty directory is not an error + } + return bp, err + } + } + prog := &Program{ Fset: conf.fset(), Imported: make(map[string]*PackageInfo), @@ -553,47 +676,101 @@ conf: conf, prog: prog, imported: make(map[string]*importInfo), + start: time.Now(), + graph: make(map[string]map[string]bool), } - for path := range conf.ImportPkgs { - info, err := imp.importPackage(path) - if err != nil { - return nil, err // failed to create package + // -- loading proper (concurrent phase) -------------------------------- + + var errpkgs []string // packages that contained errors + + // Load the initially imported packages and their dependencies, + // in parallel. + for _, ii := range imp.loadAll("", conf.ImportPkgs) { + if ii.err != nil { + conf.TypeChecker.Error(ii.err) // failed to create package + errpkgs = append(errpkgs, ii.path) + continue } - prog.Imported[path] = info + prog.Imported[ii.info.Pkg.Path()] = ii.info } - // Now augment those packages that need it. + // Augment the designated initial packages by their tests. + // Dependencies are loaded in parallel. + var xtestPkgs []*build.Package for path, augment := range conf.ImportPkgs { - if augment { - // Find and create the actual package. - bp, err := conf.findSourcePackage(path) - if err != nil { - // "Can't happen" because of previous loop. - return nil, err // package not found - } + if !augment { + continue + } - info := imp.imported[path].info // must be non-nil, see above - files, errs := imp.conf.parsePackageFiles(bp, 't') - for _, err := range errs { - info.appendError(err) - } - typeCheckFiles(info, files...) + bp, err := conf.FindPackage(conf.build(), path) + if err != nil { + // Package not found, or can't even parse package declaration. + // Already reported by previous loop; ignore it. + continue + } + + // Needs external test package? + if len(bp.XTestGoFiles) > 0 { + xtestPkgs = append(xtestPkgs, bp) } - } - for _, create := range conf.CreatePkgs { - path := create.Path - if create.Path == "" && len(create.Files) > 0 { - path = create.Files[0].Name.Name + imp.importedMu.Lock() // (unnecessary, we're sequential here) + info := imp.imported[path].info // must be non-nil, see above + imp.importedMu.Unlock() + + // Parse the in-package test files. + files, errs := imp.conf.parsePackageFiles(bp, 't') + for _, err := range errs { + info.appendError(err) } + + // The test files augmenting package P cannot be imported, + // but may import packages that import P, + // so we must disable the cycle check. + imp.addFiles(info, files, false) + } + + createPkg := func(path string, files []*ast.File, errs []error) { info := imp.newPackageInfo(path) - typeCheckFiles(info, create.Files...) + for _, err := range errs { + info.appendError(err) + } + + // Ad-hoc packages are non-importable, + // so no cycle check is needed. + // addFiles loads dependencies in parallel. + imp.addFiles(info, files, false) prog.Created = append(prog.Created, info) } + // Create packages specified by conf.CreatePkgs. + for _, cp := range conf.CreatePkgs { + files, errs := parseFiles(conf.fset(), conf.build(), nil, ".", cp.Filenames, conf.ParserMode) + files = append(files, cp.Files...) + + path := cp.Path + if path == "" { + if len(files) > 0 { + path = files[0].Name.Name + } else { + path = "(unnamed)" + } + } + createPkg(path, files, errs) + } + + // Create external test packages. + sort.Sort(byImportPath(xtestPkgs)) + for _, bp := range xtestPkgs { + files, errs := imp.conf.parsePackageFiles(bp, 'x') + createPkg(bp.ImportPath+"_test", files, errs) + } + + // -- finishing up (sequential) ---------------------------------------- + if len(prog.Imported)+len(prog.Created) == 0 { - return nil, errors.New("no initial packages were specified") + return nil, errors.New("no initial packages were loaded") } // Create infos for indirectly imported packages. @@ -611,7 +788,6 @@ if !conf.AllowErrors { // Report errors in indirectly imported packages. - var errpkgs []string for _, info := range prog.AllPackages { if len(info.Errors) > 0 { errpkgs = append(errpkgs, info.Pkg.Path()) @@ -633,6 +809,12 @@ return prog, nil } +type byImportPath []*build.Package + +func (b byImportPath) Len() int { return len(b) } +func (b byImportPath) Less(i, j int) bool { return b[i].ImportPath < b[j].ImportPath } +func (b byImportPath) Swap(i, j int) { b[i], b[j] = b[j], b[i] } + // markErrorFreePackages sets the TransitivelyErrorFree flag on all // applicable packages. func markErrorFreePackages(allPackages map[*types.Package]*PackageInfo) { @@ -682,18 +864,6 @@ return &build.Default } -// findSourcePackage locates the specified (possibly empty) package -// using go/build logic. It returns an error if not found. -// -func (conf *Config) findSourcePackage(path string) (*build.Package, error) { - // Import(srcDir="") disables local imports, e.g. import "./foo". - bp, err := conf.build().Import(path, "", 0) - if _, ok := err.(*build.NoGoError); ok { - return bp, nil // empty directory is not an error - } - return bp, err -} - // parsePackageFiles enumerates the files belonging to package path, // then loads, parses and returns them, plus a list of I/O or parse // errors that were encountered. @@ -743,52 +913,150 @@ // // Idempotent. // -func (imp *importer) doImport(imports map[string]*types.Package, path string) (*types.Package, error) { +func (imp *importer) doImport(from *PackageInfo, to string) (*types.Package, error) { // Package unsafe is handled specially, and has no PackageInfo. - if path == "unsafe" { + // TODO(adonovan): move this check into go/types? + if to == "unsafe" { return types.Unsafe, nil } - info, err := imp.importPackage(path) - if err != nil { - return nil, err + imp.importedMu.Lock() + ii := imp.imported[to] + imp.importedMu.Unlock() + if ii == nil { + panic("internal error: unexpected import: " + to) + } + if ii.err != nil { + return nil, ii.err } + if ii.info != nil { + return ii.info.Pkg, nil + } + + // Import of incomplete package: this indicates a cycle. + fromPath := from.Pkg.Path() + if cycle := imp.findPath(to, fromPath); cycle != nil { + cycle = append([]string{fromPath}, cycle...) + return nil, fmt.Errorf("import cycle: %s", strings.Join(cycle, " -> ")) + } + + panic("internal error: import of incomplete (yet acyclic) package: " + fromPath) +} + +// loadAll loads, parses, and type-checks the specified packages in +// parallel and returns their completed importInfos in unspecified order. +// +// fromPath is the import path of the importing package, if it is +// importable, "" otherwise. It is used for cycle detection. +// +func (imp *importer) loadAll(fromPath string, paths map[string]bool) []*importInfo { + result := make([]*importInfo, 0, len(paths)) + for path := range paths { + result = append(result, imp.startLoad(path)) + } + + if fromPath != "" { + // We're loading a set of imports. + // + // We must record graph edges from the importing package + // to its dependencies, and check for cycles. + imp.graphMu.Lock() + deps, ok := imp.graph[fromPath] + if !ok { + deps = make(map[string]bool) + imp.graph[fromPath] = deps + } + for path := range paths { + deps[path] = true + } + imp.graphMu.Unlock() + } + + for _, ii := range result { + if fromPath != "" { + if cycle := imp.findPath(ii.path, fromPath); cycle != nil { + // Cycle-forming import: we must not await its + // completion since it would deadlock. + // + // We don't record the error in ii since + // the error is really associated with the + // cycle-forming edge, not the package itself. + // (Also it would complicate the + // invariants of importPath completion.) + if trace { + fmt.Fprintln(os.Stderr, "import cycle: %q", cycle) + } + continue + } + } + ii.awaitCompletion() - // Update the type checker's package map on success. - imports[path] = info.Pkg + } + return result +} - return info.Pkg, nil +// findPath returns an arbitrary path from 'from' to 'to' in the import +// graph, or nil if there was none. +func (imp *importer) findPath(from, to string) []string { + imp.graphMu.Lock() + defer imp.graphMu.Unlock() + + seen := make(map[string]bool) + var search func(stack []string, importPath string) []string + search = func(stack []string, importPath string) []string { + if !seen[importPath] { + seen[importPath] = true + stack = append(stack, importPath) + if importPath == to { + return stack + } + for x := range imp.graph[importPath] { + if p := search(stack, x); p != nil { + return p + } + } + } + return nil + } + return search(make([]string, 0, 20), from) } -// importPackage imports the package with the given import path, plus -// its dependencies. +// startLoad initiates the loading, parsing and type-checking of the +// specified package and its dependencies, if it has not already begun. // -// On success, it returns a PackageInfo, possibly containing errors. -// importPackage returns an error if it couldn't even create the package. +// It returns an importInfo, not necessarily in a completed state. The +// caller must call awaitCompletion() before accessing its info and err +// fields. +// +// startLoad is concurrency-safe and idempotent. // // Precondition: path != "unsafe". // -func (imp *importer) importPackage(path string) (*PackageInfo, error) { +func (imp *importer) startLoad(path string) *importInfo { + imp.importedMu.Lock() ii, ok := imp.imported[path] if !ok { - // In preorder, initialize the map entry to a cycle - // error in case importPackage(path) is called again - // before the import is completed. - ii = &importInfo{err: fmt.Errorf("import cycle in package %s", path)} + ii = &importInfo{path: path} + ii.complete.L = &ii.mu imp.imported[path] = ii - // Find and create the actual package. - if _, ok := imp.conf.ImportPkgs[path]; ok || imp.conf.SourceImports { - ii.info, ii.err = imp.importFromSource(path) - } else { - ii.info, ii.err = imp.importFromBinary(path) - } - if ii.info != nil { - ii.info.Importable = true - } + go imp.load(path, ii) } + imp.importedMu.Unlock() + + return ii +} - return ii.info, ii.err +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.ImportFromBinary { + info, err = imp.loadFromSource(path) + } else { + info, err = imp.importFromBinary(path) + } + ii.Complete(info, err) } // importFromBinary implements package loading from the client-supplied @@ -800,43 +1068,79 @@ if importfn == nil { importfn = gcimporter.Import } + imp.typecheckerMu.Lock() pkg, err := importfn(imp.conf.TypeChecker.Packages, path) + if pkg != nil { + imp.conf.TypeChecker.Packages[path] = pkg + } + imp.typecheckerMu.Unlock() if err != nil { return nil, err } info := &PackageInfo{Pkg: pkg} + info.Importable = true + imp.typecheckerMu.Lock() imp.prog.AllPackages[pkg] = info + imp.typecheckerMu.Unlock() return info, nil } -// importFromSource implements package loading by parsing Go source files +// loadFromSource implements package loading by parsing Go source files // located by go/build. +// The returned PackageInfo's typeCheck function must be called. // -func (imp *importer) importFromSource(path string) (*PackageInfo, error) { - bp, err := imp.conf.findSourcePackage(path) +func (imp *importer) loadFromSource(path string) (*PackageInfo, error) { + bp, err := imp.conf.FindPackage(imp.conf.build(), path) if err != nil { return nil, err // package not found } - // Type-check the package. - info := imp.newPackageInfo(path) + info := imp.newPackageInfo(bp.ImportPath) + info.Importable = true files, errs := imp.conf.parsePackageFiles(bp, 'g') for _, err := range errs { info.appendError(err) } - typeCheckFiles(info, files...) + + imp.addFiles(info, files, true) + + imp.typecheckerMu.Lock() + imp.conf.TypeChecker.Packages[path] = info.Pkg + imp.typecheckerMu.Unlock() + return info, nil } -// typeCheckFiles adds the specified files to info and type-checks them. -// The order of files determines the package initialization order. -// It may be called multiple times. +// addFiles adds and type-checks the specified files to info, loading +// their dependencies if needed. The order of files determines the +// package initialization order. It may be called multiple times on the +// same package. Errors are appended to the info.Errors field. +// +// cycleCheck determines whether the imports within files create +// dependency edges that should be checked for potential cycles. // -// Errors are stored in the info.Errors field. -func typeCheckFiles(info *PackageInfo, files ...*ast.File) { +func (imp *importer) addFiles(info *PackageInfo, files []*ast.File, cycleCheck bool) { info.Files = append(info.Files, files...) - // Ignore the returned (first) error since we already collect them all. - _ = info.checker.Files(files) + // Ensure the dependencies are loaded, in parallel. + var fromPath string + if cycleCheck { + fromPath = info.Pkg.Path() + } + imp.loadAll(fromPath, scanImports(files)) + + if trace { + fmt.Fprintf(os.Stderr, "%s: start %q (%d)\n", + time.Since(imp.start), info.Pkg.Path(), len(files)) + } + + // Ignore the returned (first) error since we + // already collect them all in the PackageInfo. + info.checker.Files(files) + + if trace { + fmt.Fprintf(os.Stderr, "%s: stop %q\n", + time.Since(imp.start), info.Pkg.Path()) + } } func (imp *importer) newPackageInfo(path string) *PackageInfo { @@ -863,10 +1167,14 @@ if f := imp.conf.TypeCheckFuncBodies; f != nil { tc.IgnoreFuncBodies = !f(path) } - tc.Import = imp.doImport // doImport wraps the user's importfn, effectively + tc.Import = func(_ map[string]*types.Package, to string) (*types.Package, error) { + return imp.doImport(info, to) + } tc.Error = info.appendError // appendError wraps the user's Error function info.checker = types.NewChecker(&tc, imp.conf.fset(), pkg, &info.Info) + imp.typecheckerMu.Lock() imp.prog.AllPackages[pkg] = info + imp.typecheckerMu.Unlock() return info } Index: llgo/trunk/third_party/gotools/go/loader/loader_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/loader/loader_test.go +++ llgo/trunk/third_party/gotools/go/loader/loader_test.go @@ -5,141 +5,404 @@ package loader_test import ( - "bytes" "fmt" "go/build" - "io" - "io/ioutil" - "os" + "reflect" "sort" "strings" + "sync" "testing" - "time" + "llvm.org/llgo/third_party/gotools/go/buildutil" "llvm.org/llgo/third_party/gotools/go/loader" ) -func loadFromArgs(args []string) (prog *loader.Program, rest []string, err error) { - conf := &loader.Config{} - rest, err = conf.FromArgs(args, true) +// TestFromArgs checks that conf.FromArgs populates conf correctly. +// It does no I/O. +func TestFromArgs(t *testing.T) { + type result struct { + Err string + Rest []string + ImportPkgs map[string]bool + CreatePkgs []loader.PkgSpec + } + for _, test := range []struct { + args []string + tests bool + want result + }{ + // Mix of existing and non-existent packages. + { + args: []string{"nosuchpkg", "errors"}, + want: result{ + ImportPkgs: map[string]bool{"errors": false, "nosuchpkg": false}, + }, + }, + // Same, with -test flag. + { + args: []string{"nosuchpkg", "errors"}, + tests: true, + want: result{ + ImportPkgs: map[string]bool{"errors": true, "nosuchpkg": true}, + }, + }, + // Surplus arguments. + { + args: []string{"fmt", "errors", "--", "surplus"}, + want: result{ + Rest: []string{"surplus"}, + ImportPkgs: map[string]bool{"errors": false, "fmt": false}, + }, + }, + // Ad hoc package specified as *.go files. + { + args: []string{"foo.go", "bar.go"}, + want: result{CreatePkgs: []loader.PkgSpec{{ + Filenames: []string{"foo.go", "bar.go"}, + }}}, + }, + // Mixture of *.go and import paths. + { + args: []string{"foo.go", "fmt"}, + want: result{ + Err: "named files must be .go files: fmt", + }, + }, + } { + var conf loader.Config + rest, err := conf.FromArgs(test.args, test.tests) + got := result{ + Rest: rest, + ImportPkgs: conf.ImportPkgs, + CreatePkgs: conf.CreatePkgs, + } + if err != nil { + got.Err = err.Error() + } + if !reflect.DeepEqual(got, test.want) { + t.Errorf("FromArgs(%q) = %+v, want %+v", test.args, got, test.want) + } + } +} + +func TestLoad_NoInitialPackages(t *testing.T) { + var conf loader.Config + + const wantErr = "no initial packages were loaded" + + prog, err := conf.Load() if err == nil { - prog, err = conf.Load() + t.Errorf("Load succeeded unexpectedly, want %q", wantErr) + } else if err.Error() != wantErr { + t.Errorf("Load failed with wrong error %q, want %q", err, wantErr) + } + if prog != nil { + t.Errorf("Load unexpectedly returned a Program") + } +} + +func TestLoad_MissingInitialPackage(t *testing.T) { + var conf loader.Config + conf.Import("nosuchpkg") + conf.Import("errors") + + const wantErr = "couldn't load packages due to errors: nosuchpkg" + + prog, err := conf.Load() + if err == nil { + t.Errorf("Load succeeded unexpectedly, want %q", wantErr) + } else if err.Error() != wantErr { + t.Errorf("Load failed with wrong error %q, want %q", err, wantErr) + } + if prog != nil { + t.Errorf("Load unexpectedly returned a Program") } - return } -func TestLoadFromArgs(t *testing.T) { - // Failed load: bad first import path causes parsePackageFiles to fail. - args := []string{"nosuchpkg", "errors"} - if _, _, err := loadFromArgs(args); err == nil { - t.Errorf("loadFromArgs(%q) succeeded, want failure", args) - } else { - // cannot find package: ok. - } - - // Failed load: bad second import path proceeds to doImport0, which fails. - args = []string{"errors", "nosuchpkg"} - if _, _, err := loadFromArgs(args); err == nil { - t.Errorf("loadFromArgs(%q) succeeded, want failure", args) - } else { - // cannot find package: ok - } - - // Successful load. - args = []string{"fmt", "errors", "--", "surplus"} - prog, rest, err := loadFromArgs(args) +func TestLoad_MissingInitialPackage_AllowErrors(t *testing.T) { + var conf loader.Config + conf.AllowErrors = true + conf.Import("nosuchpkg") + conf.ImportWithTests("errors") + + prog, err := conf.Load() if err != nil { - t.Fatalf("loadFromArgs(%q) failed: %s", args, err) + t.Errorf("Load failed unexpectedly: %v", err) } - if got, want := fmt.Sprint(rest), "[surplus]"; got != want { - t.Errorf("loadFromArgs(%q) rest: got %s, want %s", args, got, want) + if prog == nil { + t.Fatalf("Load returned a nil Program") } - // Check list of Created packages. - var pkgnames []string - for _, info := range prog.Created { - pkgnames = append(pkgnames, info.Pkg.Path()) + if got, want := created(prog), "errors_test"; got != want { + t.Errorf("Created = %s, want %s", got, want) } - // All import paths may contribute tests. - if got, want := fmt.Sprint(pkgnames), "[fmt_test errors_test]"; got != want { - t.Errorf("Created: got %s, want %s", got, want) - } - - // Check set of Imported packages. - pkgnames = nil - for path := range prog.Imported { - pkgnames = append(pkgnames, path) - } - sort.Strings(pkgnames) - // All import paths may contribute tests. - if got, want := fmt.Sprint(pkgnames), "[errors fmt]"; got != want { - t.Errorf("Loaded: got %s, want %s", got, want) + if got, want := imported(prog), "errors"; got != want { + t.Errorf("Imported = %s, want %s", got, want) } +} - // Check set of transitive packages. - // There are >30 and the set may grow over time, so only check a few. - all := map[string]struct{}{} - for _, info := range prog.AllPackages { - all[info.Pkg.Path()] = struct{}{} +func TestCreateUnnamedPackage(t *testing.T) { + var conf loader.Config + conf.CreateFromFilenames("") + prog, err := conf.Load() + if err != nil { + t.Fatalf("Load failed: %v", err) } - want := []string{"strings", "time", "runtime", "testing", "unicode"} - for _, w := range want { - if _, ok := all[w]; !ok { - t.Errorf("AllPackages: want element %s, got set %v", w, all) - } + if got, want := fmt.Sprint(prog.InitialPackages()), "[(unnamed)]"; got != want { + t.Errorf("InitialPackages = %s, want %s", got, want) } } -func TestLoadFromArgsSource(t *testing.T) { - // mixture of *.go/non-go. - args := []string{"testdata/a.go", "fmt"} - prog, _, err := loadFromArgs(args) +func TestLoad_MissingFileInCreatedPackage(t *testing.T) { + var conf loader.Config + conf.CreateFromFilenames("", "missing.go") + + const wantErr = "couldn't load packages due to errors: (unnamed)" + + prog, err := conf.Load() + if prog != nil { + t.Errorf("Load unexpectedly returned a Program") + } if err == nil { - t.Errorf("loadFromArgs(%q) succeeded, want failure", args) - } else { - // "named files must be .go files: fmt": ok + t.Fatalf("Load succeeded unexpectedly, want %q", wantErr) } + if err.Error() != wantErr { + t.Fatalf("Load failed with wrong error %q, want %q", err, wantErr) + } +} - // successful load - args = []string{"testdata/a.go", "testdata/b.go"} - prog, _, err = loadFromArgs(args) +func TestLoad_MissingFileInCreatedPackage_AllowErrors(t *testing.T) { + conf := loader.Config{AllowErrors: true} + conf.CreateFromFilenames("", "missing.go") + + prog, err := conf.Load() if err != nil { - t.Fatalf("loadFromArgs(%q) failed: %s", args, err) + t.Errorf("Load failed: %v", err) } - if len(prog.Created) != 1 { - t.Errorf("loadFromArgs(%q): got %d items, want 1", args, len(prog.Created)) + if got, want := fmt.Sprint(prog.InitialPackages()), "[(unnamed)]"; got != want { + t.Fatalf("InitialPackages = %s, want %s", got, want) } - if len(prog.Created) > 0 { - path := prog.Created[0].Pkg.Path() - if path != "P" { - t.Errorf("loadFromArgs(%q): got %v, want [P]", prog.Created, path) - } +} + +func TestLoad_ParseError(t *testing.T) { + var conf loader.Config + conf.CreateFromFilenames("badpkg", "testdata/badpkgdecl.go") + + const wantErr = "couldn't load packages due to errors: badpkg" + + prog, err := conf.Load() + if prog != nil { + t.Errorf("Load unexpectedly returned a Program") + } + if err == nil { + t.Fatalf("Load succeeded unexpectedly, want %q", wantErr) + } + if err.Error() != wantErr { + t.Fatalf("Load failed with wrong error %q, want %q", err, wantErr) } } -type fakeFileInfo struct{} +func TestLoad_ParseError_AllowErrors(t *testing.T) { + var conf loader.Config + conf.AllowErrors = true + conf.CreateFromFilenames("badpkg", "testdata/badpkgdecl.go") -func (fakeFileInfo) Name() string { return "x.go" } -func (fakeFileInfo) Sys() interface{} { return nil } -func (fakeFileInfo) ModTime() time.Time { return time.Time{} } -func (fakeFileInfo) IsDir() bool { return false } -func (fakeFileInfo) Size() int64 { return 0 } -func (fakeFileInfo) Mode() os.FileMode { return 0644 } + prog, err := conf.Load() + if err != nil { + t.Errorf("Load failed unexpectedly: %v", err) + } + if prog == nil { + t.Fatalf("Load returned a nil Program") + } + if got, want := created(prog), "badpkg"; got != want { + t.Errorf("Created = %s, want %s", got, want) + } -var justXgo = [1]os.FileInfo{fakeFileInfo{}} // ["x.go"] + badpkg := prog.Created[0] + if len(badpkg.Files) != 1 { + t.Errorf("badpkg has %d files, want 1", len(badpkg.Files)) + } + wantErr := "testdata/badpkgdecl.go:1:34: expected 'package', found 'EOF'" + if !hasError(badpkg.Errors, wantErr) { + t.Errorf("badpkg.Errors = %v, want %s", badpkg.Errors, wantErr) + } +} -func fakeContext(pkgs map[string]string) *build.Context { - ctxt := build.Default // copy - ctxt.GOROOT = "/go" - ctxt.GOPATH = "" - ctxt.IsDir = func(path string) bool { return true } - ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { return justXgo[:], nil } - ctxt.OpenFile = func(path string) (io.ReadCloser, error) { - path = path[len("/go/src/"):] - return ioutil.NopCloser(bytes.NewBufferString(pkgs[path[0:1]])), nil +func TestLoad_FromSource_Success(t *testing.T) { + var conf loader.Config + conf.CreateFromFilenames("P", "testdata/a.go", "testdata/b.go") + + prog, err := conf.Load() + if err != nil { + t.Errorf("Load failed unexpectedly: %v", err) + } + if prog == nil { + t.Fatalf("Load returned a nil Program") + } + if got, want := created(prog), "P"; got != want { + t.Errorf("Created = %s, want %s", got, want) } - return &ctxt } +func TestLoad_FromImports_Success(t *testing.T) { + var conf loader.Config + conf.ImportWithTests("fmt") + conf.ImportWithTests("errors") + + prog, err := conf.Load() + if err != nil { + t.Errorf("Load failed unexpectedly: %v", err) + } + if prog == nil { + t.Fatalf("Load returned a nil Program") + } + if got, want := created(prog), "errors_test fmt_test"; got != want { + t.Errorf("Created = %q, want %s", got, want) + } + if got, want := imported(prog), "errors fmt"; got != want { + t.Errorf("Imported = %s, want %s", got, want) + } + // Check set of transitive packages. + // There are >30 and the set may grow over time, so only check a few. + want := map[string]bool{ + "strings": true, + "time": true, + "runtime": true, + "testing": true, + "unicode": true, + } + for _, path := range all(prog) { + delete(want, path) + } + if len(want) > 0 { + t.Errorf("AllPackages is missing these keys: %q", keys(want)) + } +} + +func TestLoad_MissingIndirectImport(t *testing.T) { + pkgs := map[string]string{ + "a": `package a; import _ "b"`, + "b": `package b; import _ "c"`, + } + conf := loader.Config{Build: fakeContext(pkgs)} + conf.Import("a") + + const wantErr = "couldn't load packages due to errors: b" + + prog, err := conf.Load() + if err == nil { + t.Errorf("Load succeeded unexpectedly, want %q", wantErr) + } else if err.Error() != wantErr { + t.Errorf("Load failed with wrong error %q, want %q", err, wantErr) + } + if prog != nil { + t.Errorf("Load unexpectedly returned a Program") + } +} + +func TestLoad_BadDependency_AllowErrors(t *testing.T) { + for _, test := range []struct { + descr string + pkgs map[string]string + wantPkgs string + }{ + + { + descr: "missing dependency", + pkgs: map[string]string{ + "a": `package a; import _ "b"`, + "b": `package b; import _ "c"`, + }, + wantPkgs: "a b", + }, + { + descr: "bad package decl in dependency", + pkgs: map[string]string{ + "a": `package a; import _ "b"`, + "b": `package b; import _ "c"`, + "c": `package`, + }, + wantPkgs: "a b", + }, + { + descr: "parse error in dependency", + pkgs: map[string]string{ + "a": `package a; import _ "b"`, + "b": `package b; import _ "c"`, + "c": `package c; var x = `, + }, + wantPkgs: "a b c", + }, + } { + conf := loader.Config{ + AllowErrors: true, + Build: fakeContext(test.pkgs), + } + conf.Import("a") + + prog, err := conf.Load() + if err != nil { + t.Errorf("%s: Load failed unexpectedly: %v", test.descr, err) + } + if prog == nil { + t.Fatalf("%s: Load returned a nil Program", test.descr) + } + + if got, want := imported(prog), "a"; got != want { + t.Errorf("%s: Imported = %s, want %s", test.descr, got, want) + } + if got := all(prog); strings.Join(got, " ") != test.wantPkgs { + t.Errorf("%s: AllPackages = %s, want %s", test.descr, got, test.wantPkgs) + } + } +} + +func TestCwd(t *testing.T) { + ctxt := fakeContext(map[string]string{"one/two/three": `package three`}) + for _, test := range []struct { + cwd, arg, want string + }{ + {cwd: "/go/src/one", arg: "./two/three", want: "one/two/three"}, + {cwd: "/go/src/one", arg: "../one/two/three", want: "one/two/three"}, + {cwd: "/go/src/one", arg: "one/two/three", want: "one/two/three"}, + {cwd: "/go/src/one/two/three", arg: ".", want: "one/two/three"}, + {cwd: "/go/src/one", arg: "two/three", want: ""}, + } { + conf := loader.Config{ + Cwd: test.cwd, + Build: ctxt, + } + conf.Import(test.arg) + + var got string + prog, err := conf.Load() + if prog != nil { + got = imported(prog) + } + if got != test.want { + t.Errorf("Load(%s) from %s: Imported = %s, want %s", + test.arg, test.cwd, got, test.want) + if err != nil { + t.Errorf("Load failed: %v", err) + } + } + } +} + +// TODO(adonovan): more Load tests: +// +// failures: +// - to parse package decl of *_test.go files +// - to parse package decl of external *_test.go files +// - to parse whole of *_test.go files +// - to parse whole of external *_test.go files +// - to open a *.go file during import scanning +// - to import from binary + +// features: +// - InitialPackages +// - PackageCreated hook +// - TypeCheckFuncBodies hook + func TestTransitivelyErrorFreeFlag(t *testing.T) { // Create an minimal custom build.Context // that fakes the following packages: @@ -157,9 +420,8 @@ "e": `package e; import _ "d"`, } conf := loader.Config{ - AllowErrors: true, - SourceImports: true, - Build: fakeContext(pkgs), + AllowErrors: true, + Build: fakeContext(pkgs), } conf.Import("a") @@ -200,21 +462,23 @@ } } -// Test that both syntax (scan/parse) and type errors are both recorded +// Test that syntax (scan/parse), type, and loader errors are recorded // (in PackageInfo.Errors) and reported (via Config.TypeChecker.Error). func TestErrorReporting(t *testing.T) { pkgs := map[string]string{ - "a": `package a; import _ "b"; var x int = false`, + "a": `package a; import (_ "b"; _ "c"); var x int = false`, "b": `package b; 'syntax error!`, } conf := loader.Config{ - AllowErrors: true, - SourceImports: true, - Build: fakeContext(pkgs), + AllowErrors: true, + Build: fakeContext(pkgs), } + var mu sync.Mutex var allErrors []error conf.TypeChecker.Error = func(err error) { + mu.Lock() allErrors = append(allErrors, err) + mu.Unlock() } conf.Import("a") @@ -226,15 +490,6 @@ t.Fatalf("Load returned nil *Program") } - hasError := func(errors []error, substr string) bool { - for _, err := range errors { - if strings.Contains(err.Error(), substr) { - return true - } - } - return false - } - // TODO(adonovan): test keys of ImportMap. // Check errors recorded in each PackageInfo. @@ -244,6 +499,9 @@ if !hasError(info.Errors, "cannot convert false") { t.Errorf("a.Errors = %v, want bool conversion (type) error", info.Errors) } + if !hasError(info.Errors, "could not import c") { + t.Errorf("a.Errors = %v, want import (loader) error", info.Errors) + } case "b": if !hasError(info.Errors, "rune literal not terminated") { t.Errorf("b.Errors = %v, want unterminated literal (syntax) error", info.Errors) @@ -253,7 +511,159 @@ // Check errors reported via error handler. if !hasError(allErrors, "cannot convert false") || - !hasError(allErrors, "rune literal not terminated") { - t.Errorf("allErrors = %v, want both syntax and type errors", allErrors) + !hasError(allErrors, "rune literal not terminated") || + !hasError(allErrors, "could not import c") { + t.Errorf("allErrors = %v, want syntax, type and loader errors", allErrors) + } +} + +func TestCycles(t *testing.T) { + for _, test := range []struct { + descr string + ctxt *build.Context + wantErr string + }{ + { + "self-cycle", + fakeContext(map[string]string{ + "main": `package main; import _ "selfcycle"`, + "selfcycle": `package selfcycle; import _ "selfcycle"`, + }), + `import cycle: selfcycle -> selfcycle`, + }, + { + "three-package cycle", + fakeContext(map[string]string{ + "main": `package main; import _ "a"`, + "a": `package a; import _ "b"`, + "b": `package b; import _ "c"`, + "c": `package c; import _ "a"`, + }), + `import cycle: c -> a -> b -> c`, + }, + { + "self-cycle in dependency of test file", + buildutil.FakeContext(map[string]map[string]string{ + "main": { + "main.go": `package main`, + "main_test.go": `package main; import _ "a"`, + }, + "a": { + "a.go": `package a; import _ "a"`, + }, + }), + `import cycle: a -> a`, + }, + // TODO(adonovan): fix: these fail + // { + // "two-package cycle in dependency of test file", + // buildutil.FakeContext(map[string]map[string]string{ + // "main": { + // "main.go": `package main`, + // "main_test.go": `package main; import _ "a"`, + // }, + // "a": { + // "a.go": `package a; import _ "main"`, + // }, + // }), + // `import cycle: main -> a -> main`, + // }, + // { + // "self-cycle in augmented package", + // buildutil.FakeContext(map[string]map[string]string{ + // "main": { + // "main.go": `package main`, + // "main_test.go": `package main; import _ "main"`, + // }, + // }), + // `import cycle: main -> main`, + // }, + } { + conf := loader.Config{ + AllowErrors: true, + Build: test.ctxt, + } + var mu sync.Mutex + var allErrors []error + conf.TypeChecker.Error = func(err error) { + mu.Lock() + allErrors = append(allErrors, err) + mu.Unlock() + } + conf.ImportWithTests("main") + + prog, err := conf.Load() + if err != nil { + t.Errorf("%s: Load failed: %s", test.descr, err) + } + if prog == nil { + t.Fatalf("%s: Load returned nil *Program", test.descr) + } + + if !hasError(allErrors, test.wantErr) { + t.Errorf("%s: Load() errors = %q, want %q", + test.descr, allErrors, test.wantErr) + } + } + + // TODO(adonovan): + // - Test that in a legal test cycle, none of the symbols + // defined by augmentation are visible via import. +} + +// ---- utilities ---- + +// Simplifying wrapper around buildutil.FakeContext for single-file packages. +func fakeContext(pkgs map[string]string) *build.Context { + pkgs2 := make(map[string]map[string]string) + for path, content := range pkgs { + pkgs2[path] = map[string]string{"x.go": content} + } + return buildutil.FakeContext(pkgs2) +} + +func hasError(errors []error, substr string) bool { + for _, err := range errors { + if strings.Contains(err.Error(), substr) { + return true + } + } + return false +} + +func keys(m map[string]bool) (keys []string) { + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + return +} + +// Returns all loaded packages. +func all(prog *loader.Program) []string { + var pkgs []string + for _, info := range prog.AllPackages { + pkgs = append(pkgs, info.Pkg.Path()) + } + sort.Strings(pkgs) + return pkgs +} + +// Returns initially imported packages, as a string. +func imported(prog *loader.Program) string { + var pkgs []string + for _, info := range prog.Imported { + pkgs = append(pkgs, info.Pkg.Path()) + } + sort.Strings(pkgs) + return strings.Join(pkgs, " ") +} + +// Returns initially created packages, as a string. +func created(prog *loader.Program) string { + var pkgs []string + for _, info := range prog.Created { + pkgs = append(pkgs, info.Pkg.Path()) } + return strings.Join(pkgs, " ") } Index: llgo/trunk/third_party/gotools/go/loader/stdlib_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/loader/stdlib_test.go +++ llgo/trunk/third_party/gotools/go/loader/stdlib_test.go @@ -40,9 +40,7 @@ ctxt.GOPATH = "" // disable GOPATH conf := loader.Config{Build: &ctxt} for _, path := range buildutil.AllPackages(conf.Build) { - if err := conf.ImportWithTests(path); err != nil { - t.Error(err) - } + conf.ImportWithTests(path) } prog, err := conf.Load() Index: llgo/trunk/third_party/gotools/go/loader/testdata/badpkgdecl.go =================================================================== --- llgo/trunk/third_party/gotools/go/loader/testdata/badpkgdecl.go +++ llgo/trunk/third_party/gotools/go/loader/testdata/badpkgdecl.go @@ -0,0 +1 @@ +// this file has no package decl Index: llgo/trunk/third_party/gotools/go/loader/util.go =================================================================== --- llgo/trunk/third_party/gotools/go/loader/util.go +++ llgo/trunk/third_party/gotools/go/loader/util.go @@ -11,8 +11,10 @@ "go/token" "io" "os" - "path/filepath" + "strconv" "sync" + + "llvm.org/llgo/third_party/gotools/go/buildutil" ) // parseFiles parses the Go source files within directory dir and @@ -26,21 +28,13 @@ if displayPath == nil { displayPath = func(path string) string { return path } } - isAbs := filepath.IsAbs - if ctxt.IsAbsPath != nil { - isAbs = ctxt.IsAbsPath - } - joinPath := filepath.Join - if ctxt.JoinPath != nil { - joinPath = ctxt.JoinPath - } var wg sync.WaitGroup n := len(files) parsed := make([]*ast.File, n) errors := make([]error, n) for i, file := range files { - if !isAbs(file) { - file = joinPath(dir, file) + if !buildutil.IsAbsPath(ctxt, file) { + file = buildutil.JoinPath(ctxt, dir, file) } wg.Add(1) go func(i int, file string) { @@ -86,6 +80,32 @@ return parsed, errors } +// scanImports returns the set of all package import paths from all +// import specs in the specified files. +func scanImports(files []*ast.File) map[string]bool { + imports := make(map[string]bool) + for _, f := range files { + for _, decl := range f.Decls { + if decl, ok := decl.(*ast.GenDecl); ok && decl.Tok == token.IMPORT { + for _, spec := range decl.Specs { + spec := spec.(*ast.ImportSpec) + + // NB: do not assume the program is well-formed! + path, err := strconv.Unquote(spec.Path.Value) + if err != nil { + continue // quietly ignore the error + } + if path == "C" || path == "unsafe" { + continue // skip pseudo packages + } + imports[path] = true + } + } + } + } + return imports +} + // ---------- Internal helpers ---------- // TODO(adonovan): make this a method: func (*token.File) Contains(token.Pos) Index: llgo/trunk/third_party/gotools/go/pointer/analysis.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/analysis.go +++ llgo/trunk/third_party/gotools/go/pointer/analysis.go @@ -255,7 +255,7 @@ // (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()) } } Index: llgo/trunk/third_party/gotools/go/pointer/api.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/api.go +++ llgo/trunk/third_party/gotools/go/pointer/api.go @@ -21,6 +21,12 @@ // Mains contains the set of 'main' packages to analyze // Clients must provide the analysis with at least one // package defining a main() function. + // + // Non-main packages in the ssa.Program that are not + // dependencies of any main package may still affect the + // analysis result, because they contribute runtime types and + // thus methods. + // TODO(adonovan): investigate whether this is desirable. Mains []*ssa.Package // Reflection determines whether to handle reflection Index: llgo/trunk/third_party/gotools/go/pointer/example_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/example_test.go +++ llgo/trunk/third_party/gotools/go/pointer/example_test.go @@ -41,10 +41,10 @@ i.f(x) // dynamic method call } ` - // Construct a loader. - conf := loader.Config{SourceImports: true} + var conf loader.Config - // Parse the input file. + // Parse the input file, a string. + // (Command-line tools should use conf.FromArgs.) file, err := conf.ParseFile("myprog.go", myprog) if err != nil { fmt.Print(err) // parse error Index: llgo/trunk/third_party/gotools/go/pointer/gen.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/gen.go +++ llgo/trunk/third_party/gotools/go/pointer/gen.go @@ -1262,7 +1262,7 @@ // Create nodes and constraints for all methods of all types // that are dynamically accessible via reflection or interfaces. - for _, T := range a.prog.TypesWithMethodSets() { + for _, T := range a.prog.RuntimeTypes() { a.genMethodsOf(T) } Index: llgo/trunk/third_party/gotools/go/pointer/pointer_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/pointer_test.go +++ llgo/trunk/third_party/gotools/go/pointer/pointer_test.go @@ -153,7 +153,7 @@ } func doOneInput(input, filename string) bool { - conf := loader.Config{SourceImports: true} + var conf loader.Config // Parsing. f, err := conf.ParseFile(filename, input) Index: llgo/trunk/third_party/gotools/go/pointer/stdlib_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/stdlib_test.go +++ llgo/trunk/third_party/gotools/go/pointer/stdlib_test.go @@ -35,10 +35,7 @@ // Load, parse and type-check the program. ctxt := build.Default // copy ctxt.GOPATH = "" // disable GOPATH - conf := loader.Config{ - SourceImports: true, - Build: &ctxt, - } + conf := loader.Config{Build: &ctxt} if _, err := conf.FromArgs(buildutil.AllPackages(conf.Build), true); err != nil { t.Errorf("FromArgs failed: %v", err) return Index: llgo/trunk/third_party/gotools/go/pointer/util.go =================================================================== --- llgo/trunk/third_party/gotools/go/pointer/util.go +++ llgo/trunk/third_party/gotools/go/pointer/util.go @@ -50,11 +50,7 @@ return false } -// isInterface reports whether T is an interface type. -func isInterface(T types.Type) bool { - _, ok := T.Underlying().(*types.Interface) - return ok -} +func isInterface(T types.Type) bool { return types.IsInterface(T) } // mustDeref returns the element type of its argument, which must be a // pointer; panic ensues otherwise. Index: llgo/trunk/third_party/gotools/go/ssa/builder.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/builder.go +++ llgo/trunk/third_party/gotools/go/ssa/builder.go @@ -358,7 +358,9 @@ v = fn.addLocal(t, e.Lbrace) } v.Comment = "complit" - b.compLit(fn, v, e, true) // initialize in place + var sb storebuf + b.compLit(fn, v, e, true, &sb) + sb.emit(fn) return &address{addr: v, pos: e.Lbrace, expr: e} case *ast.ParenExpr: @@ -420,15 +422,39 @@ panic(fmt.Sprintf("unexpected address expression: %T", e)) } -// exprInPlace emits to fn code to initialize the lvalue loc with the -// value of expression e. If isZero is true, exprInPlace assumes that loc -// holds the zero value for its type. -// -// This is equivalent to loc.store(fn, b.expr(fn, e)) but may -// generate better code in some cases, e.g. for composite literals -// in an addressable location. +type store struct { + lhs lvalue + rhs Value +} + +type storebuf struct{ stores []store } + +func (sb *storebuf) store(lhs lvalue, rhs Value) { + sb.stores = append(sb.stores, store{lhs, rhs}) +} + +func (sb *storebuf) emit(fn *Function) { + for _, s := range sb.stores { + s.lhs.store(fn, s.rhs) + } +} + +// assign emits to fn code to initialize the lvalue loc with the value +// of expression e. If isZero is true, assign assumes that loc holds +// the zero value for its type. +// +// This is equivalent to loc.store(fn, b.expr(fn, e)), but may generate +// better code in some cases, e.g., for composite literals in an +// addressable location. +// +// If sb is not nil, assign generates code to evaluate expression e, but +// not to update loc. Instead, the necessary stores are appended to the +// storebuf sb so that they can be executed later. This allows correct +// in-place update of existing variables when the RHS is a composite +// literal that may reference parts of the LHS. // -func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr, isZero bool) { +func (b *builder) assign(fn *Function, loc lvalue, e ast.Expr, isZero bool, sb *storebuf) { + // Can we initialize it in place? if e, ok := unparen(e).(*ast.CompositeLit); ok { // A CompositeLit never evaluates to a pointer, // so if the type of the location is a pointer, @@ -436,7 +462,12 @@ if _, ok := loc.(blank); !ok { // avoid calling blank.typ() if isPointer(loc.typ()) { ptr := b.addr(fn, e, true).address(fn) - loc.store(fn, ptr) // copy address + // copy address + if sb != nil { + sb.store(loc, ptr) + } else { + loc.store(fn, ptr) + } return } } @@ -447,14 +478,35 @@ // Can't in-place initialize an interface value. // Fall back to copying. } else { + // x = T{...} or x := T{...} addr := loc.address(fn) - b.compLit(fn, addr, e, isZero) // in place - emitDebugRef(fn, e, addr, true) + if sb != nil { + b.compLit(fn, addr, e, isZero, sb) + } else { + var sb storebuf + b.compLit(fn, addr, e, isZero, &sb) + sb.emit(fn) + } + + // Subtle: emit debug ref for aggregate types only; + // slice and map are handled by store ops in compLit. + switch loc.typ().Underlying().(type) { + case *types.Struct, *types.Array: + emitDebugRef(fn, e, addr, true) + } + return } } } - loc.store(fn, b.expr(fn, e)) // copy value + + // simple case: just copy + rhs := b.expr(fn, e) + if sb != nil { + sb.store(loc, rhs) + } else { + loc.store(fn, rhs) + } } // expr lowers a single-result expression e to SSA form, emitting code @@ -600,7 +652,7 @@ case *types.Basic, *types.Slice, *types.Pointer: // *array x = b.expr(fn, e.X) default: - unreachable() + panic("unreachable") } if e.High != nil { high = b.expr(fn, e.High) @@ -938,7 +990,7 @@ fn.addLocalForIdent(id) } lval := b.addr(fn, id, false) // non-escaping - b.exprInPlace(fn, lval, spec.Values[i], true) + b.assign(fn, lval, spec.Values[i], true, nil) } case len(spec.Values) == 0: @@ -973,37 +1025,33 @@ // func (b *builder) assignStmt(fn *Function, lhss, rhss []ast.Expr, isDef bool) { // Side effects of all LHSs and RHSs must occur in left-to-right order. - var lvals []lvalue - for _, lhs := range lhss { + lvals := make([]lvalue, len(lhss)) + isZero := make([]bool, len(lhss)) + for i, lhs := range lhss { var lval lvalue = blank{} if !isBlankIdent(lhs) { if isDef { if obj := fn.Pkg.info.Defs[lhs.(*ast.Ident)]; obj != nil { fn.addNamedLocal(obj) + isZero[i] = true } } lval = b.addr(fn, lhs, false) // non-escaping } - lvals = append(lvals, lval) + lvals[i] = lval } if len(lhss) == len(rhss) { - // e.g. x, y = f(), g() - if len(lhss) == 1 { - // x = type{...} - // Optimization: in-place construction - // of composite literals. - b.exprInPlace(fn, lvals[0], rhss[0], false) - } else { - // Parallel assignment. All reads must occur - // before all updates, precluding exprInPlace. - var rvals []Value - for _, rval := range rhss { - rvals = append(rvals, b.expr(fn, rval)) - } - for i, lval := range lvals { - lval.store(fn, rvals[i]) - } + // Simple assignment: x = f() (!isDef) + // Parallel assignment: x, y = f(), g() (!isDef) + // or short var decl: x, y := f(), g() (isDef) + // + // In all cases, the RHSs may refer to the LHSs, + // so we need a storebuf. + var sb storebuf + for i := range rhss { + b.assign(fn, lvals[i], rhss[i], isZero[i], &sb) } + sb.emit(fn) } else { // e.g. x, y = pos() tuple := b.exprN(fn, rhss[0]) @@ -1031,22 +1079,32 @@ } // compLit emits to fn code to initialize a composite literal e at -// address addr with type typ, typically allocated by Alloc. +// address addr with type typ. +// // Nested composite literals are recursively initialized in place // where possible. If isZero is true, compLit assumes that addr // holds the zero value for typ. // +// Because the elements of a composite literal may refer to the +// variables being updated, as in the second line below, +// x := T{a: 1} +// x = T{a: x.a} +// all the reads must occur before all the writes. Thus all stores to +// loc are emitted to the storebuf sb for later execution. +// // A CompositeLit may have pointer type only in the recursive (nested) // case when the type name is implicit. e.g. in []*T{{}}, the inner // literal has type *T behaves like &T{}. // In that case, addr must hold a T, not a *T. // -func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero bool) { +func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero bool, sb *storebuf) { typ := deref(fn.Pkg.typeOf(e)) switch t := typ.Underlying().(type) { case *types.Struct: if !isZero && len(e.Elts) != t.NumFields() { - emitMemClear(fn, addr, e.Lbrace) + // memclear + sb.store(&address{addr, e.Lbrace, nil}, + zeroValue(fn, deref(addr.Type()))) isZero = true } for i, e := range e.Elts { @@ -1071,7 +1129,7 @@ } faddr.setType(types.NewPointer(sf.Type())) fn.emit(faddr) - b.exprInPlace(fn, &address{addr: faddr, pos: pos, expr: e}, e, isZero) + b.assign(fn, &address{addr: faddr, pos: pos, expr: e}, e, isZero, sb) } case *types.Array, *types.Slice: @@ -1083,21 +1141,23 @@ alloc := emitNew(fn, at, e.Lbrace) alloc.Comment = "slicelit" array = alloc - isZero = true case *types.Array: at = t array = addr - } - if !isZero && int64(len(e.Elts)) != at.Len() { - emitMemClear(fn, array, e.Lbrace) - isZero = true + if !isZero && int64(len(e.Elts)) != at.Len() { + // memclear + sb.store(&address{array, e.Lbrace, nil}, + zeroValue(fn, deref(array.Type()))) + } } var idx *Const for _, e := range e.Elts { + pos := e.Pos() if kv, ok := e.(*ast.KeyValueExpr); ok { idx = b.expr(fn, kv.Key).(*Const) + pos = kv.Colon e = kv.Value } else { var idxval int64 @@ -1112,30 +1172,44 @@ } iaddr.setType(types.NewPointer(at.Elem())) fn.emit(iaddr) - b.exprInPlace(fn, &address{addr: iaddr, pos: e.Pos(), expr: e}, e, isZero) + if t != at { // slice + // backing array is unaliased => storebuf not needed. + b.assign(fn, &address{addr: iaddr, pos: pos, expr: e}, e, true, nil) + } else { + b.assign(fn, &address{addr: iaddr, pos: pos, expr: e}, e, true, sb) + } } + if t != at { // slice s := &Slice{X: array} s.setPos(e.Lbrace) s.setType(typ) - emitStore(fn, addr, fn.emit(s), e.Lbrace) + sb.store(&address{addr: addr, pos: e.Lbrace, expr: e}, fn.emit(s)) } case *types.Map: m := &MakeMap{Reserve: intConst(int64(len(e.Elts)))} m.setPos(e.Lbrace) m.setType(typ) - emitStore(fn, addr, fn.emit(m), e.Lbrace) + fn.emit(m) for _, e := range e.Elts { e := e.(*ast.KeyValueExpr) - loc := &element{ + loc := element{ m: m, k: emitConv(fn, b.expr(fn, e.Key), t.Key()), t: t.Elem(), pos: e.Colon, } - b.exprInPlace(fn, loc, e.Value, true) + + // We call assign() only because it takes care + // of any &-operation required in the recursive + // case, e.g., + // map[int]*struct{}{0: {}} implies &struct{}{}. + // In-place update is of course impossible, + // and no storebuf is needed. + b.assign(fn, &loc, e.Value, true, nil) } + sb.store(&address{addr: addr, pos: e.Lbrace, expr: e}, m) default: panic("unexpected CompositeLit type: " + t.String()) @@ -2125,24 +2199,12 @@ if isBlankIdent(id) { return // discard } - var fn *Function + fn := pkg.values[pkg.info.Defs[id]].(*Function) if decl.Recv == nil && id.Name == "init" { - pkg.ninit++ - fn = &Function{ - name: fmt.Sprintf("init#%d", pkg.ninit), - Signature: new(types.Signature), - pos: decl.Name.NamePos, - Pkg: pkg, - Prog: pkg.Prog, - syntax: decl, - } - var v Call v.Call.Value = fn v.setType(types.NewTuple()) pkg.init.emit(&v) - } else { - fn = pkg.values[pkg.info.Defs[id]].(*Function) } b.buildFunction(fn) } @@ -2193,7 +2255,7 @@ // that would require package creation in topological order. for name, mem := range p.Members { if ast.IsExported(name) { - p.needMethodsOf(mem.Type()) + p.Prog.needMethodsOf(mem.Type()) } } if p.Prog.mode&LogSource != 0 { @@ -2243,7 +2305,7 @@ } else { lval = blank{} } - b.exprInPlace(init, lval, varinit.Rhs, true) + b.assign(init, lval, varinit.Rhs, true, nil) } else { // n:1 initialization: var x, y := f() tuple := b.exprN(init, varinit.Rhs) @@ -2301,129 +2363,3 @@ panic(fmt.Sprintf("no type for %T @ %s", e, p.Prog.Fset.Position(e.Pos()))) } - -// needMethodsOf ensures that runtime type information (including the -// complete method set) is available for the specified type T and all -// its subcomponents. -// -// needMethodsOf must be called for at least every type that is an -// operand of some MakeInterface instruction, and for the type of -// every exported package member. -// -// Precondition: T is not a method signature (*Signature with Recv()!=nil). -// -// Thread-safe. (Called via emitConv from multiple builder goroutines.) -// -// TODO(adonovan): make this faster. It accounts for 20% of SSA build -// time. Do we need to maintain a distinct needRTTI and methodSets per -// package? Using just one in the program might be much faster. -// -func (p *Package) needMethodsOf(T types.Type) { - p.methodsMu.Lock() - p.needMethods(T, false) - p.methodsMu.Unlock() -} - -// Precondition: T is not a method signature (*Signature with Recv()!=nil). -// Precondition: the p.methodsMu lock is held. -// Recursive case: skip => don't call makeMethods(T). -func (p *Package) needMethods(T types.Type, skip bool) { - // Each package maintains its own set of types it has visited. - if prevSkip, ok := p.needRTTI.At(T).(bool); ok { - // needMethods(T) was previously called - if !prevSkip || skip { - return // already seen, with same or false 'skip' value - } - } - p.needRTTI.Set(T, skip) - - // Prune the recursion if we find a named or *named type - // belonging to another package. - var n *types.Named - switch T := T.(type) { - case *types.Named: - n = T - case *types.Pointer: - n, _ = T.Elem().(*types.Named) - } - if n != nil { - owner := n.Obj().Pkg() - if owner == nil { - return // built-in error type - } - if owner != p.Object { - return // belongs to another package - } - } - - // All the actual method sets live in the Program so that - // multiple packages can share a single copy in memory of the - // symbols that would be compiled into multiple packages (as - // weak symbols). - if !skip && p.Prog.makeMethods(T) { - p.methodSets = append(p.methodSets, T) - } - - // Recursion over signatures of each method. - tmset := p.Prog.MethodSets.MethodSet(T) - for i := 0; i < tmset.Len(); i++ { - sig := tmset.At(i).Type().(*types.Signature) - p.needMethods(sig.Params(), false) - p.needMethods(sig.Results(), false) - } - - switch t := T.(type) { - case *types.Basic: - // nop - - case *types.Interface: - // nop---handled by recursion over method set. - - case *types.Pointer: - p.needMethods(t.Elem(), false) - - case *types.Slice: - p.needMethods(t.Elem(), false) - - case *types.Chan: - p.needMethods(t.Elem(), false) - - case *types.Map: - p.needMethods(t.Key(), false) - p.needMethods(t.Elem(), false) - - case *types.Signature: - if t.Recv() != nil { - panic(fmt.Sprintf("Signature %s has Recv %s", t, t.Recv())) - } - p.needMethods(t.Params(), false) - p.needMethods(t.Results(), false) - - case *types.Named: - // A pointer-to-named type can be derived from a named - // type via reflection. It may have methods too. - p.needMethods(types.NewPointer(T), false) - - // Consider 'type T struct{S}' where S has methods. - // Reflection provides no way to get from T to struct{S}, - // only to S, so the method set of struct{S} is unwanted, - // so set 'skip' flag during recursion. - p.needMethods(t.Underlying(), true) - - case *types.Array: - p.needMethods(t.Elem(), false) - - case *types.Struct: - for i, n := 0, t.NumFields(); i < n; i++ { - p.needMethods(t.Field(i).Type(), false) - } - - case *types.Tuple: - for i, n := 0, t.Len(); i < n; i++ { - p.needMethods(t.At(i).Type(), false) - } - - default: - panic(T) - } -} Index: llgo/trunk/third_party/gotools/go/ssa/builder_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/builder_test.go +++ llgo/trunk/third_party/gotools/go/ssa/builder_test.go @@ -21,7 +21,7 @@ // 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 @@ ` // Create a single-file main package. - var conf loader.Config + conf := loader.Config{ImportFromBinary: true} f, err := conf.ParseFile("", test) if err != nil { t.Error(err) @@ -151,8 +151,8 @@ } } -// TestTypesWithMethodSets tests that Package.TypesWithMethodSets includes all necessary types. -func TestTypesWithMethodSets(t *testing.T) { +// TestRuntimeTypes tests that (*Program).RuntimeTypes() includes all necessary types. +func TestRuntimeTypes(t *testing.T) { tests := []struct { input string want []string @@ -167,7 +167,7 @@ }, // Subcomponents of type of exported package-level var are needed. {`package C; import "bytes"; var V struct {*bytes.Buffer}`, - []string{"*struct{*bytes.Buffer}", "struct{*bytes.Buffer}"}, + []string{"*bytes.Buffer", "*struct{*bytes.Buffer}", "struct{*bytes.Buffer}"}, }, // Subcomponents of type of unexported package-level var are not needed. {`package D; import "bytes"; var v struct {*bytes.Buffer}`, @@ -175,7 +175,7 @@ }, // Subcomponents of type of exported package-level function are needed. {`package E; import "bytes"; func F(struct {*bytes.Buffer}) {}`, - []string{"struct{*bytes.Buffer}"}, + []string{"*bytes.Buffer", "struct{*bytes.Buffer}"}, }, // Subcomponents of type of unexported package-level function are not needed. {`package F; import "bytes"; func f(struct {*bytes.Buffer}) {}`, @@ -187,11 +187,11 @@ }, // ...unless used by MakeInterface. {`package G2; import "bytes"; type x struct{}; func (x) G(struct {*bytes.Buffer}) {}; var v interface{} = x{}`, - []string{"*p.x", "p.x", "struct{*bytes.Buffer}"}, + []string{"*bytes.Buffer", "*p.x", "p.x", "struct{*bytes.Buffer}"}, }, // Subcomponents of type of unexported method are not needed. {`package I; import "bytes"; type X struct{}; func (X) G(struct {*bytes.Buffer}) {}`, - []string{"*p.X", "p.X", "struct{*bytes.Buffer}"}, + []string{"*bytes.Buffer", "*p.X", "p.X", "struct{*bytes.Buffer}"}, }, // Local types aren't needed. {`package J; import "bytes"; func f() { type T struct {*bytes.Buffer}; var t T; _ = t }`, @@ -199,11 +199,11 @@ }, // ...unless used by MakeInterface. {`package K; import "bytes"; func f() { type T struct {*bytes.Buffer}; _ = interface{}(T{}) }`, - []string{"*p.T", "p.T"}, + []string{"*bytes.Buffer", "*p.T", "p.T"}, }, // Types used as operand of MakeInterface are needed. {`package L; import "bytes"; func f() { _ = interface{}(struct{*bytes.Buffer}{}) }`, - []string{"struct{*bytes.Buffer}"}, + []string{"*bytes.Buffer", "struct{*bytes.Buffer}"}, }, // MakeInterface is optimized away when storing to a blank. {`package M; import "bytes"; var _ interface{} = struct{*bytes.Buffer}{}`, @@ -212,7 +212,7 @@ } for _, test := range tests { // Create a single-file main package. - var conf loader.Config + conf := loader.Config{ImportFromBinary: true} f, err := conf.ParseFile("", test.input) if err != nil { t.Errorf("test %q: %s", test.input[:15], err) @@ -226,17 +226,17 @@ continue } prog := ssa.Create(iprog, ssa.SanityCheckFunctions) - mainPkg := prog.Package(iprog.Created[0].Pkg) prog.BuildAll() var typstrs []string - for _, T := range mainPkg.TypesWithMethodSets() { + for _, T := range prog.RuntimeTypes() { typstrs = append(typstrs, T.String()) } sort.Strings(typstrs) if !reflect.DeepEqual(typstrs, test.want) { - t.Errorf("test 'package %s': got %q, want %q", f.Name.Name, typstrs, test.want) + t.Errorf("test 'package %s': got %q, want %q", + f.Name.Name, typstrs, test.want) } } } Index: llgo/trunk/third_party/gotools/go/ssa/create.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/create.go +++ llgo/trunk/third_party/gotools/go/ssa/create.go @@ -8,6 +8,7 @@ // See builder.go for explanation. import ( + "fmt" "go/ast" "go/token" "os" @@ -18,20 +19,6 @@ "llvm.org/llgo/third_party/gotools/go/types/typeutil" ) -// BuilderMode is a bitmask of options for diagnostics and checking. -type BuilderMode uint - -const ( - PrintPackages BuilderMode = 1 << iota // Print package inventory to stdout - PrintFunctions // Print function SSA code to stdout - LogSource // Log source locations as SSA builder progresses - SanityCheckFunctions // Perform sanity checking of function bodies - NaiveForm // Build naïve SSA form: don't replace local loads/stores with registers - BuildSerially // Build packages serially, not in parallel. - GlobalDebug // Enable debug info for all packages - BareInits // Build init functions without guards or calls to dependent inits -) - // Create returns a new SSA Program. An SSA Package is created for // each transitively error-free package of iprog. // @@ -102,10 +89,15 @@ pkg.Members[name] = g case *types.Func: + sig := obj.Type().(*types.Signature) + if sig.Recv() == nil && name == "init" { + pkg.ninit++ + name = fmt.Sprintf("init#%d", pkg.ninit) + } fn := &Function{ name: name, object: obj, - Signature: obj.Type().(*types.Signature), + Signature: sig, syntax: syntax, pos: obj.Pos(), Pkg: pkg, @@ -116,7 +108,7 @@ } pkg.values[obj] = fn - if fn.Signature.Recv() == nil { + if sig.Recv() == nil { pkg.Members[name] = fn // package-level function } @@ -162,9 +154,6 @@ case *ast.FuncDecl: id := decl.Name - if decl.Recv == nil && id.Name == "init" { - return // no object - } if !isBlankIdent(id) { memberFromObject(pkg, pkg.info.Defs[id], decl) } @@ -257,7 +246,7 @@ return p } -// printMu serializes printing of Packages/Functions to stdout +// printMu serializes printing of Packages/Functions to stdout. var printMu sync.Mutex // AllPackages returns a new slice containing all packages in the Index: llgo/trunk/third_party/gotools/go/ssa/emit.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/emit.go +++ llgo/trunk/third_party/gotools/go/ssa/emit.go @@ -208,7 +208,7 @@ val = emitConv(f, val, DefaultType(ut_src)) } - f.Pkg.needMethodsOf(val.Type()) + f.Pkg.Prog.needMethodsOf(val.Type()) mi := &MakeInterface{X: val} mi.setType(typ) return f.emit(mi) @@ -430,12 +430,6 @@ } } -// emitMemClear emits to f code to zero the value pointed to by ptr. -func emitMemClear(f *Function, ptr Value, pos token.Pos) { - // TODO(adonovan): define and use a 'memclr' intrinsic for aggregate types. - emitStore(f, ptr, zeroValue(f, deref(ptr.Type())), pos) -} - // createRecoverBlock emits to f a block of code to return after a // recovered panic, and sets f.Recover to it. // Index: llgo/trunk/third_party/gotools/go/ssa/func.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/func.go +++ llgo/trunk/third_party/gotools/go/ssa/func.go @@ -426,7 +426,7 @@ // Definition must be in an enclosing function; // plumb it through intervening closures. if f.parent == nil { - panic("no Value for type.Object " + obj.Name()) + panic("no ssa.Value for " + obj.String()) } outer := f.parent.lookup(obj, true) // escaping v := &FreeVar{ @@ -464,7 +464,10 @@ // (i.e. from == f.Pkg.Object), they are rendered without the package path. // For example: "IsNaN", "(*Buffer).Bytes", etc. // -// Invariant: all non-synthetic functions have distinct package-qualified names. +// All non-synthetic functions have distinct package-qualified names. +// (But two methods may have the same name "(T).f" if one is a synthetic +// wrapper promoting a non-exported method "f" from another package; in +// that case, the strings are equal but the identifiers "f" are distinct.) // func (f *Function) RelString(from *types.Package) string { // Anonymous? Index: llgo/trunk/third_party/gotools/go/ssa/interp/interp.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/interp.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/interp.go @@ -239,10 +239,10 @@ panic(targetPanic{fr.get(instr.X)}) case *ssa.Send: - fr.get(instr.Chan).(chan value) <- copyVal(fr.get(instr.X)) + fr.get(instr.Chan).(chan value) <- fr.get(instr.X) case *ssa.Store: - *fr.get(instr.Addr).(*value) = copyVal(fr.get(instr.Val)) + store(deref(instr.Addr.Type()), fr.get(instr.Addr).(*value), fr.get(instr.Val)) case *ssa.If: succ := 1 @@ -307,10 +307,11 @@ case *ssa.FieldAddr: x := fr.get(instr.X) + // FIXME wrong! &global.f must not change if we do *global = zero! fr.env[instr] = &(*x.(*value)).(structure)[instr.Field] case *ssa.Field: - fr.env[instr] = copyVal(fr.get(instr.X).(structure)[instr.Field]) + fr.env[instr] = fr.get(instr.X).(structure)[instr.Field] case *ssa.IndexAddr: x := fr.get(instr.X) @@ -325,7 +326,7 @@ } case *ssa.Index: - fr.env[instr] = copyVal(fr.get(instr.X).(array)[asInt(fr.get(instr.Index))]) + fr.env[instr] = fr.get(instr.X).(array)[asInt(fr.get(instr.Index))] case *ssa.Lookup: fr.env[instr] = lookup(instr, fr.get(instr.X), fr.get(instr.Index)) @@ -436,7 +437,7 @@ } else { fn = f } - args = append(args, copyVal(recv.v)) + args = append(args, recv.v) } for _, arg := range call.Args { args = append(args, fr.get(arg)) Index: llgo/trunk/third_party/gotools/go/ssa/interp/interp_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/interp_test.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/interp_test.go @@ -188,7 +188,7 @@ 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 @@ -340,9 +340,7 @@ // CreateTestMainPackage should return nil if there were no tests. func TestNullTestmainPackage(t *testing.T) { var conf loader.Config - if err := conf.CreateFromFilenames("", "testdata/b_test.go"); err != nil { - t.Fatalf("ParseFile failed: %s", err) - } + conf.CreateFromFilenames("", "testdata/b_test.go") iprog, err := conf.Load() if err != nil { t.Fatalf("CreatePackages failed: %s", err) Index: llgo/trunk/third_party/gotools/go/ssa/interp/ops.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/ops.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/ops.go @@ -286,9 +286,7 @@ v = x.lookup(idx.(hashable)) ok = v != nil } - if ok { - v = copyVal(v) - } else { + if !ok { v = zero(instr.X.Type().Underlying().(*types.Map).Elem()) } if instr.CommaOk { @@ -844,7 +842,7 @@ return -x } case token.MUL: - return copyVal(*x.(*value)) // load + return load(deref(instr.X.Type()), x.(*value)) case token.NOT: return !x.(bool) case token.XOR: @@ -891,7 +889,7 @@ err = checkInterface(i, idst, itf) } else if types.Identical(itf.t, instr.AssertedType) { - v = copyVal(itf.v) // extract value + v = itf.v // extract value } else { err = fmt.Sprintf("interface conversion: interface is %s, not %s", itf.t, instr.AssertedType) Index: llgo/trunk/third_party/gotools/go/ssa/interp/reflect.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/reflect.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/reflect.go @@ -31,8 +31,7 @@ var reflectTypesPackage = types.NewPackage("reflect", "reflect") // rtype is the concrete type the interpreter uses to implement the -// reflect.Type interface. Since its type is opaque to the target -// language, we use a types.Basic. +// reflect.Type interface. // // type rtype var rtypeType = makeNamedType("rtype", &opaqueType{nil, "rtype"}) @@ -508,6 +507,29 @@ Members: make(map[string]ssa.Member), } + // Clobber the type-checker's notion of reflect.Value's + // underlying type so that it more closely matches the fake one + // (at least in the number of fields---we lie about the type of + // the rtype field). + // + // We must ensure that calls to (ssa.Value).Type() return the + // fake type so that correct "shape" is used when allocating + // variables, making zero values, loading, and storing. + // + // TODO(adonovan): obviously this is a hack. We need a cleaner + // way to fake the reflect package (almost---DeepEqual is fine). + // One approach would be not to even load its source code, but + // provide fake source files. This would guarantee that no bad + // information leaks into other packages. + if r := i.prog.ImportedPackage("reflect"); r != nil { + rV := r.Object.Scope().Lookup("Value").Type().(*types.Named) + tEface := types.NewInterface(nil, nil).Complete() + rV.SetUnderlying(types.NewStruct([]*types.Var{ + types.NewField(token.NoPos, r.Object, "t", tEface, false), // a lie + types.NewField(token.NoPos, r.Object, "v", tEface, false), + }, nil)) + } + i.rtypeMethods = methodSet{ "Bits": newMethod(i.reflectPackage, rtypeType, "Bits"), "Elem": newMethod(i.reflectPackage, rtypeType, "Elem"), Index: llgo/trunk/third_party/gotools/go/ssa/interp/testdata/complit.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/testdata/complit.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/testdata/complit.go @@ -80,5 +80,89 @@ } } +// Regression test for https://github.com/golang/go/issues/10127: +// composite literal clobbers destination before reading from it. +func init() { + // map + { + type M map[string]int + m := M{"x": 1, "y": 2} + m = M{"x": m["y"], "y": m["x"]} + if m["x"] != 2 || m["y"] != 1 { + panic(fmt.Sprint(m)) + } + + n := M{"x": 3} + m, n = M{"x": n["x"]}, M{"x": m["x"]} // parallel assignment + if got := fmt.Sprint(m["x"], n["x"]); got != "3 2" { + panic(got) + } + } + + // struct + { + type T struct{ x, y, z int } + t := T{x: 1, y: 2, z: 3} + + t = T{x: t.y, y: t.z, z: t.x} // all fields + if got := fmt.Sprint(t); got != "{2 3 1}" { + panic(got) + } + + t = T{x: t.y, y: t.z + 3} // not all fields + if got := fmt.Sprint(t); got != "{3 4 0}" { + panic(got) + } + + u := T{x: 5, y: 6, z: 7} + t, u = T{x: u.x}, T{x: t.x} // parallel assignment + if got := fmt.Sprint(t, u); got != "{5 0 0} {3 0 0}" { + panic(got) + } + } + + // array + { + a := [3]int{0: 1, 1: 2, 2: 3} + + a = [3]int{0: a[1], 1: a[2], 2: a[0]} // all elements + if got := fmt.Sprint(a); got != "[2 3 1]" { + panic(got) + } + + a = [3]int{0: a[1], 1: a[2] + 3} // not all elements + if got := fmt.Sprint(a); got != "[3 4 0]" { + panic(got) + } + + b := [3]int{0: 5, 1: 6, 2: 7} + a, b = [3]int{0: b[0]}, [3]int{0: a[0]} // parallel assignment + if got := fmt.Sprint(a, b); got != "[5 0 0] [3 0 0]" { + panic(got) + } + } + + // slice + { + s := []int{0: 1, 1: 2, 2: 3} + + s = []int{0: s[1], 1: s[2], 2: s[0]} // all elements + if got := fmt.Sprint(s); got != "[2 3 1]" { + panic(got) + } + + s = []int{0: s[1], 1: s[2] + 3} // not all elements + if got := fmt.Sprint(s); got != "[3 4]" { + panic(got) + } + + t := []int{0: 5, 1: 6, 2: 7} + s, t = []int{0: t[0]}, []int{0: s[0]} // parallel assignment + if got := fmt.Sprint(s, t); got != "[5] [3]" { + panic(got) + } + } +} + func main() { } Index: llgo/trunk/third_party/gotools/go/ssa/interp/testdata/coverage.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/testdata/coverage.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/testdata/coverage.go @@ -91,8 +91,8 @@ } pa1 := &[2]string{"foo", "bar"} - pa2 := pa1 // creates an alias - (*pa2)[0] = "wiz" // * required to workaround typechecker bug + pa2 := pa1 // creates an alias + pa2[0] = "wiz" if x := fmt.Sprint(*pa1, *pa2); x != "[wiz bar] [wiz bar]" { panic(x) } @@ -515,3 +515,20 @@ i.f() panic("unreachable") } + +// Regression test for a subtle bug in which copying values would causes +// subcomponents of aggregate variables to change address, breaking +// aliases. +func init() { + type T struct{ f int } + var x T + p := &x.f + x = T{} + *p = 1 + if x.f != 1 { + panic("lost store") + } + if p != &x.f { + panic("unstable address") + } +} Index: llgo/trunk/third_party/gotools/go/ssa/interp/value.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/interp/value.go +++ llgo/trunk/third_party/gotools/go/ssa/interp/value.go @@ -310,43 +310,53 @@ panic(fmt.Sprintf("%T is unhashable", x)) } -// copyVal returns a copy of value v. -// TODO(adonovan): add tests of aliasing and mutation. -func copyVal(v value) value { - if v == nil { - panic("copyVal(nil)") - } - switch v := v.(type) { - case bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, uintptr, float32, float64, complex64, complex128, string, unsafe.Pointer: - return v - case map[value]value: - return v - case *hashmap: - return v - case chan value: - return v - case *value: - return v - case *ssa.Function, *ssa.Builtin, *closure: - return v - case iface: - return v - case []value: - return v - case structure: +// reflect.Value struct values don't have a fixed shape, since the +// payload can be a scalar or an aggregate depending on the instance. +// So store (and load) can't simply use recursion over the shape of the +// rhs value, or the lhs, to copy the value; we need the static type +// information. (We can't make reflect.Value a new basic data type +// because its "structness" is exposed to Go programs.) + +// load returns the value of type T in *addr. +func load(T types.Type, addr *value) value { + switch T := T.Underlying().(type) { + case *types.Struct: + v := (*addr).(structure) a := make(structure, len(v)) - copy(a, v) + for i := range a { + a[i] = load(T.Field(i).Type(), &v[i]) + } return a - case array: + case *types.Array: + v := (*addr).(array) a := make(array, len(v)) - copy(a, v) + for i := range a { + a[i] = load(T.Elem(), &v[i]) + } return a - case tuple: - break - case rtype: - return v + default: + return *addr + } +} + +// store stores value v of type T into *addr. +func store(T types.Type, addr *value, v value) { + switch T := T.Underlying().(type) { + case *types.Struct: + lhs := (*addr).(structure) + rhs := v.(structure) + for i := range lhs { + store(T.Field(i).Type(), &lhs[i], rhs[i]) + } + case *types.Array: + lhs := (*addr).(array) + rhs := v.(array) + for i := range lhs { + store(T.Elem(), &lhs[i], rhs[i]) + } + default: + *addr = v } - panic(fmt.Sprintf("cannot copy %T", v)) } // Prints in the style of built-in println. Index: llgo/trunk/third_party/gotools/go/ssa/lvalue.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/lvalue.go +++ llgo/trunk/third_party/gotools/go/ssa/lvalue.go @@ -29,7 +29,7 @@ type address struct { addr Value pos token.Pos // source position - expr ast.Expr // source syntax [debug mode] + expr ast.Expr // source syntax of the value (not address) [debug mode] } func (a *address) load(fn *Function) Value { Index: llgo/trunk/third_party/gotools/go/ssa/methods.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/methods.go +++ llgo/trunk/third_party/gotools/go/ssa/methods.go @@ -55,44 +55,6 @@ return prog.Method(sel) } -// makeMethods ensures that all concrete methods of type T are -// generated. It is equivalent to calling prog.Method() on all -// members of T.methodSet(), but acquires fewer locks. -// -// It reports whether the type's (concrete) method set is non-empty. -// -// Thread-safe. -// -// EXCLUSIVE_LOCKS_ACQUIRED(prog.methodsMu) -// -func (prog *Program) makeMethods(T types.Type) bool { - if isInterface(T) { - return false // abstract method - } - tmset := prog.MethodSets.MethodSet(T) - n := tmset.Len() - if n == 0 { - return false // empty (common case) - } - - if prog.mode&LogSource != 0 { - defer logStack("makeMethods %s", T)() - } - - prog.methodsMu.Lock() - defer prog.methodsMu.Unlock() - - mset := prog.createMethodSet(T) - if !mset.complete { - mset.complete = true - for i := 0; i < n; i++ { - prog.addMethod(mset, tmset.At(i)) - } - } - - return true -} - // methodSet contains the (concrete) methods of a non-interface type. type methodSet struct { mapping map[string]*Function // populated lazily @@ -135,18 +97,15 @@ return fn } -// TypesWithMethodSets returns a new unordered slice containing all +// RuntimeTypes returns a new unordered slice containing all // concrete types in the program for which a complete (non-empty) // method set is required at run-time. // -// It is the union of pkg.TypesWithMethodSets() for all pkg in -// prog.AllPackages(). -// // Thread-safe. // // EXCLUSIVE_LOCKS_ACQUIRED(prog.methodsMu) // -func (prog *Program) TypesWithMethodSets() []types.Type { +func (prog *Program) RuntimeTypes() []types.Type { prog.methodsMu.Lock() defer prog.methodsMu.Unlock() @@ -159,33 +118,6 @@ return res } -// TypesWithMethodSets returns an unordered slice containing the set -// of all concrete types referenced within package pkg and not -// belonging to some other package, for which a complete (non-empty) -// method set is required at run-time. -// -// A type belongs to a package if it is a named type or a pointer to a -// named type, and the name was defined in that package. All other -// types belong to no package. -// -// A type may appear in the TypesWithMethodSets() set of multiple -// distinct packages if that type belongs to no package. Typical -// compilers emit method sets for such types multiple times (using -// weak symbols) into each package that references them, with the -// linker performing duplicate elimination. -// -// This set includes the types of all operands of some MakeInterface -// instruction, the types of all exported members of some package, and -// all types that are subcomponents, since even types that aren't used -// directly may be derived via reflection. -// -// Callers must not mutate the result. -// -func (pkg *Package) TypesWithMethodSets() []types.Type { - // pkg.methodsMu not required; concurrent (build) phase is over. - return pkg.methodSets -} - // declaredFunc returns the concrete function/method denoted by obj. // Panic ensues if there is none. // @@ -195,3 +127,117 @@ } panic("no concrete method: " + obj.String()) } + +// needMethodsOf ensures that runtime type information (including the +// complete method set) is available for the specified type T and all +// its subcomponents. +// +// needMethodsOf must be called for at least every type that is an +// operand of some MakeInterface instruction, and for the type of +// every exported package member. +// +// Precondition: T is not a method signature (*Signature with Recv()!=nil). +// +// Thread-safe. (Called via emitConv from multiple builder goroutines.) +// +// TODO(adonovan): make this faster. It accounts for 20% of SSA build time. +// +// EXCLUSIVE_LOCKS_ACQUIRED(prog.methodsMu) +// +func (prog *Program) needMethodsOf(T types.Type) { + prog.methodsMu.Lock() + prog.needMethods(T, false) + prog.methodsMu.Unlock() +} + +// Precondition: T is not a method signature (*Signature with Recv()!=nil). +// Recursive case: skip => don't create methods for T. +// +// EXCLUSIVE_LOCKS_REQUIRED(prog.methodsMu) +// +func (prog *Program) needMethods(T types.Type, skip bool) { + // Each package maintains its own set of types it has visited. + if prevSkip, ok := prog.runtimeTypes.At(T).(bool); ok { + // needMethods(T) was previously called + if !prevSkip || skip { + return // already seen, with same or false 'skip' value + } + } + prog.runtimeTypes.Set(T, skip) + + tmset := prog.MethodSets.MethodSet(T) + + if !skip && !isInterface(T) && tmset.Len() > 0 { + // Create methods of T. + mset := prog.createMethodSet(T) + if !mset.complete { + mset.complete = true + n := tmset.Len() + for i := 0; i < n; i++ { + prog.addMethod(mset, tmset.At(i)) + } + } + } + + // Recursion over signatures of each method. + for i := 0; i < tmset.Len(); i++ { + sig := tmset.At(i).Type().(*types.Signature) + prog.needMethods(sig.Params(), false) + prog.needMethods(sig.Results(), false) + } + + switch t := T.(type) { + case *types.Basic: + // nop + + case *types.Interface: + // nop---handled by recursion over method set. + + case *types.Pointer: + prog.needMethods(t.Elem(), false) + + case *types.Slice: + prog.needMethods(t.Elem(), false) + + case *types.Chan: + prog.needMethods(t.Elem(), false) + + case *types.Map: + prog.needMethods(t.Key(), false) + prog.needMethods(t.Elem(), false) + + case *types.Signature: + if t.Recv() != nil { + panic(fmt.Sprintf("Signature %s has Recv %s", t, t.Recv())) + } + prog.needMethods(t.Params(), false) + prog.needMethods(t.Results(), false) + + case *types.Named: + // A pointer-to-named type can be derived from a named + // type via reflection. It may have methods too. + prog.needMethods(types.NewPointer(T), false) + + // Consider 'type T struct{S}' where S has methods. + // Reflection provides no way to get from T to struct{S}, + // only to S, so the method set of struct{S} is unwanted, + // so set 'skip' flag during recursion. + prog.needMethods(t.Underlying(), true) + + case *types.Array: + prog.needMethods(t.Elem(), false) + + case *types.Struct: + for i, n := 0, t.NumFields(); i < n; i++ { + prog.needMethods(t.Field(i).Type(), false) + } + + case *types.Tuple: + for i, n := 0, t.Len(); i < n; i++ { + prog.needMethods(t.At(i).Type(), false) + } + + default: + panic(T) + } +} Index: llgo/trunk/third_party/gotools/go/ssa/mode.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/mode.go +++ llgo/trunk/third_party/gotools/go/ssa/mode.go @@ -0,0 +1,107 @@ +// Copyright 2015 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 ssa + +// This file defines the BuilderMode type and its command-line flag. + +import ( + "bytes" + "flag" + "fmt" +) + +// BuilderMode is a bitmask of options for diagnostics and checking. +type BuilderMode uint + +const ( + PrintPackages BuilderMode = 1 << iota // Print package inventory to stdout + PrintFunctions // Print function SSA code to stdout + LogSource // Log source locations as SSA builder progresses + SanityCheckFunctions // Perform sanity checking of function bodies + NaiveForm // Build naïve SSA form: don't replace local loads/stores with registers + BuildSerially // Build packages serially, not in parallel. + GlobalDebug // Enable debug info for all packages + BareInits // Build init functions without guards or calls to dependent inits +) + +const modeFlagUsage = `Options controlling the SSA builder. +The value is a sequence of zero or more of these letters: +C perform sanity [C]hecking of the SSA form. +D include [D]ebug info for every function. +P print [P]ackage inventory. +F print [F]unction SSA code. +S log [S]ource locations as SSA builder progresses. +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. +I build bare [I]nit functions: no init guards or calls to dependent inits. +` + +// BuilderModeFlag creates a new command line flag of type BuilderMode, +// adds it to the specified flag set, and returns it. +// +// Example: +// var ssabuild = BuilderModeFlag(flag.CommandLine, "ssabuild", 0) +// +func BuilderModeFlag(set *flag.FlagSet, name string, value BuilderMode) *BuilderMode { + set.Var((*builderModeValue)(&value), name, modeFlagUsage) + return &value +} + +type builderModeValue BuilderMode // satisfies flag.Value and flag.Getter. + +func (v *builderModeValue) Set(s string) error { + var mode BuilderMode + for _, c := range s { + switch c { + case 'D': + mode |= GlobalDebug + case 'P': + mode |= PrintPackages + case 'F': + mode |= PrintFunctions + case 'S': + mode |= LogSource | BuildSerially + case 'C': + mode |= SanityCheckFunctions + case 'N': + mode |= NaiveForm + case 'L': + mode |= BuildSerially + default: + return fmt.Errorf("unknown BuilderMode option: %q", c) + } + } + *v = builderModeValue(mode) + return nil +} + +func (v *builderModeValue) Get() interface{} { return BuilderMode(*v) } + +func (v *builderModeValue) String() string { + mode := BuilderMode(*v) + var buf bytes.Buffer + if mode&GlobalDebug != 0 { + buf.WriteByte('D') + } + if mode&PrintPackages != 0 { + buf.WriteByte('P') + } + if mode&PrintFunctions != 0 { + buf.WriteByte('F') + } + if mode&LogSource != 0 { + buf.WriteByte('S') + } + if mode&SanityCheckFunctions != 0 { + buf.WriteByte('C') + } + if mode&NaiveForm != 0 { + buf.WriteByte('N') + } + if mode&BuildSerially != 0 { + buf.WriteByte('L') + } + return buf.String() +} Index: llgo/trunk/third_party/gotools/go/ssa/sanity.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/sanity.go +++ llgo/trunk/third_party/gotools/go/ssa/sanity.go @@ -505,8 +505,13 @@ continue // not all members have typechecker objects } if obj.Name() != name { - panic(fmt.Sprintf("%s: %T.Object().Name() = %s, want %s", - pkg.Object.Path(), mem, obj.Name(), name)) + if obj.Name() == "init" && strings.HasPrefix(mem.Name(), "init#") { + // Ok. The name of a declared init function varies between + // its types.Func ("init") and its ssa.Function ("init#%d"). + } else { + panic(fmt.Sprintf("%s: %T.Object().Name() = %s, want %s", + pkg.Object.Path(), mem, obj.Name(), name)) + } } if obj.Pos() != mem.Pos() { panic(fmt.Sprintf("%s Pos=%d obj.Pos=%d", mem, mem.Pos(), obj.Pos())) Index: llgo/trunk/third_party/gotools/go/ssa/source.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/source.go +++ llgo/trunk/third_party/gotools/go/ssa/source.go @@ -144,7 +144,7 @@ // - e is a reference to nil or a built-in function. // - the value was optimised away. // -// If e is an addressable expression used an an lvalue context, +// If e is an addressable expression used in an lvalue context, // value is the address denoted by e, and isAddr is true. // // The types of e (or &e, if isAddr) and the result are equal Index: llgo/trunk/third_party/gotools/go/ssa/ssa.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/ssa.go +++ llgo/trunk/third_party/gotools/go/ssa/ssa.go @@ -20,7 +20,6 @@ ) // A Program is a partial or complete Go program converted to SSA form. -// type Program struct { Fset *token.FileSet // position information for the files of this Program imported map[string]*Package // all importable Packages, keyed by import path @@ -28,11 +27,12 @@ mode BuilderMode // set of mode bits for SSA construction MethodSets types.MethodSetCache // cache of type-checker's method-sets - methodsMu sync.Mutex // guards the following maps: - methodSets typeutil.Map // maps type to its concrete methodSet - canon typeutil.Map // type canonicalization map - bounds map[*types.Func]*Function // bounds for curried x.Method closures - thunks map[selectionKey]*Function // thunks for T.Method expressions + methodsMu sync.Mutex // guards the following maps: + methodSets typeutil.Map // maps type to its concrete methodSet + runtimeTypes typeutil.Map // types for which rtypes are needed + canon typeutil.Map // type canonicalization map + bounds map[*types.Func]*Function // bounds for curried x.Method closures + thunks map[selectionKey]*Function // thunks for T.Method expressions } // A Package is a single analyzed Go package containing Members for @@ -40,22 +40,23 @@ // declares. These may be accessed directly via Members, or via the // type-specific accessor methods Func, Type, Var and Const. // +// Members also contains entries for "init" (the synthetic package +// initializer) and "init#%d", the nth declared init function, +// and unspecified other things too. +// type Package struct { - Prog *Program // the owning program - Object *types.Package // the type checker's package object for this package - Members map[string]Member // all package members keyed by name - methodsMu sync.Mutex // guards needRTTI and methodSets - methodSets []types.Type // types whose method sets are included in this package - values map[types.Object]Value // package members (incl. types and methods), keyed by object - init *Function // Func("init"); the package's init function - debug bool // include full debug info in this package. + Prog *Program // the owning program + Object *types.Package // the type checker's package object for this package + Members map[string]Member // all package members keyed by name (incl. init and init#%d) + values map[types.Object]Value // package members (incl. types and methods), keyed by object + init *Function // Func("init"); the package's init function + debug bool // include full debug info in this package // The following fields are set transiently, then cleared // after building. - started int32 // atomically tested and set at start of build phase - ninit int32 // number of init functions - info *loader.PackageInfo // package ASTs and type information - needRTTI typeutil.Map // types for which runtime type info is needed + started int32 // atomically tested and set at start of build phase + ninit int32 // number of init functions + info *loader.PackageInfo // package ASTs and type information } // A Member is a member of a Go package, implemented by *NamedConst, @@ -70,7 +71,7 @@ Pos() token.Pos // position of member's declaration, if known Type() types.Type // type of the package member Token() token.Token // token.{VAR,FUNC,CONST,TYPE} - Package() *Package // returns the containing package. (TODO: rename Pkg) + Package() *Package // the containing package } // A Type is a Member of a Package representing a package-level named type. @@ -82,8 +83,8 @@ pkg *Package } -// A NamedConst is a Member of Package representing a package-level -// named constant value. +// A NamedConst is a Member of a Package representing a package-level +// named constant. // // Pos() returns the position of the declaring ast.ValueSpec.Names[*] // identifier. @@ -158,7 +159,6 @@ // corresponds to an ast.Expr; use Function.ValueForExpr // instead. NB: it requires that the function was built with // debug information.) - // Pos() token.Pos } @@ -170,21 +170,21 @@ // does not. // type Instruction interface { - // String returns the disassembled form of this value. e.g. + // String returns the disassembled form of this value. // - // Examples of Instructions that define a Value: - // e.g. "x + y" (BinOp) + // Examples of Instructions that are Values: + // "x + y" (BinOp) // "len([])" (Call) // Note that the name of the Value is not printed. // - // Examples of Instructions that do define (are) Values: - // e.g. "return x" (Return) + // Examples of Instructions that are not Values: + // "return x" (Return) // "*y = x" (Store) // - // (This separation is useful for some analyses which - // distinguish the operation from the value it - // defines. e.g. 'y = local int' is both an allocation of - // memory 'local int' and a definition of a pointer y.) + // (The separation Value.Name() from Value.String() is useful + // for some analyses which distinguish the operation from the + // value it defines, e.g., 'y = local int' is both an allocation + // of memory 'local int' and a definition of a pointer y.) String() string // Parent returns the function to which this instruction @@ -232,7 +232,6 @@ // This position may be used to determine which non-Value // Instruction corresponds to some ast.Stmts, but not all: If // and Jump instructions have no Pos(), for example.) - // Pos() token.Pos } @@ -258,12 +257,12 @@ Referrers() *[]Instruction // nil for non-Values } -// Function represents the parameters, results and code of a function +// Function represents the parameters, results, and code of a function // or method. // // If Blocks is nil, this indicates an external function for which no // Go source code is available. In this case, FreeVars and Locals -// will be nil too. Clients performing whole-program analysis must +// are nil too. Clients performing whole-program analysis must // handle external functions specially. // // Blocks contains the function's control-flow graph (CFG). @@ -278,14 +277,18 @@ // parameters, if any. // // A nested function (Parent()!=nil) that refers to one or more -// lexically enclosing local variables ("free variables") has FreeVar -// parameters. Such functions cannot be called directly but require a +// lexically enclosing local variables ("free variables") has FreeVars. +// Such functions cannot be called directly but require a // value created by MakeClosure which, via its Bindings, supplies // values for these parameters. // // If the function is a method (Signature.Recv() != nil) then the first // element of Params is the receiver parameter. // +// A Go package may declare many functions called "init". +// For each one, Object().Name() returns "init" but Name() returns +// "init#1", etc, in declaration order. +// // Pos() returns the declaring ast.FuncLit.Type.Func or the position // of the ast.FuncDecl.Name, if the function was explicit in the // source. Synthetic wrappers, for which Synthetic != "", may share @@ -323,13 +326,13 @@ lblocks map[*ast.Object]*lblock // labelled blocks } -// An SSA basic block. +// BasicBlock represents an SSA basic block. // // The final element of Instrs is always an explicit transfer of -// control (If, Jump, Return or Panic). +// control (If, Jump, Return, or Panic). // // A block may contain no Instructions only if it is unreachable, -// i.e. Preds is nil. Empty blocks are typically pruned. +// i.e., Preds is nil. Empty blocks are typically pruned. // // BasicBlocks and their Preds/Succs relation form a (possibly cyclic) // graph independent of the SSA Value graph: the control-flow graph or @@ -349,9 +352,9 @@ parent *Function // parent function Instrs []Instruction // instructions in order Preds, Succs []*BasicBlock // predecessors and successors - succs2 [2]*BasicBlock // initial space for Succs. + succs2 [2]*BasicBlock // initial space for Succs dom domInfo // dominator tree info - gaps int // number of nil Instrs (transient). + gaps int // number of nil Instrs (transient) rundefers int // number of rundefers (transient) } @@ -399,11 +402,11 @@ // // The underlying type of a constant may be any boolean, numeric, or // string type. In addition, a Const may represent the nil value of -// any reference type: interface, map, channel, pointer, slice, or +// any reference type---interface, map, channel, pointer, slice, or // function---but not "untyped nil". // // All source-level constant expressions are represented by a Const -// of equal type and value. +// of the same type and value. // // Value holds the exact value of the constant, independent of its // Type(), using the same representation as package go/exact uses for @@ -462,11 +465,12 @@ // Value-defining instructions ---------------------------------------- -// The Alloc instruction reserves space for a value of the given type, +// The Alloc instruction reserves space for a variable of the given type, // zero-initializes it, and yields its address. // // Alloc values are always addresses, and have pointer types, so the -// type of the allocated space is actually indirect(Type()). +// type of the allocated variable is actually +// Type().Underlying().(*types.Pointer).Elem(). // // If Heap is false, Alloc allocates space in the function's // activation record (frame); we refer to an Alloc(Heap=false) as a @@ -474,7 +478,7 @@ // it is executed within the same activation; the space is // re-initialized to zero. // -// If Heap is true, Alloc allocates space in the heap, and returns; we +// If Heap is true, Alloc allocates space in the heap; we // refer to an Alloc(Heap=true) as a "new" alloc. Each new Alloc // returns a different address each time it is executed. // @@ -508,7 +512,7 @@ // during SSA renaming. // // Example printed form: -// t2 = phi [0.start: t0, 1.if.then: t1, ...] +// t2 = phi [0: t0, 1: t1] // type Phi struct { register @@ -518,8 +522,8 @@ // The Call instruction represents a function or method call. // -// The Call instruction yields the function result, if there is -// exactly one, or a tuple (empty or len>1) whose components are +// The Call instruction yields the function result if there is exactly +// one. Otherwise it returns a tuple, the components of which are // accessed via Extract. // // See CallCommon for generic function call documentation. @@ -1361,7 +1365,7 @@ } // Description returns a description of the mode of this call suitable -// for a user interface, e.g. "static method call". +// for a user interface, e.g., "static method call". func (c *CallCommon) Description() string { switch fn := c.Value.(type) { case *Builtin: Index: llgo/trunk/third_party/gotools/go/ssa/ssautil/visit.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/ssautil/visit.go +++ llgo/trunk/third_party/gotools/go/ssa/ssautil/visit.go @@ -41,7 +41,7 @@ } } } - for _, T := range visit.prog.TypesWithMethodSets() { + for _, T := range visit.prog.RuntimeTypes() { mset := visit.prog.MethodSets.MethodSet(T) for i, n := 0, mset.Len(); i < n; i++ { visit.function(visit.prog.Method(mset.At(i))) Index: llgo/trunk/third_party/gotools/go/ssa/stdlib_test.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/stdlib_test.go +++ llgo/trunk/third_party/gotools/go/ssa/stdlib_test.go @@ -10,6 +10,7 @@ // Run with "go test -cpu=8 to" set GOMAXPROCS. import ( + "go/ast" "go/build" "go/token" "runtime" @@ -37,10 +38,7 @@ // Load, parse and type-check the program. ctxt := build.Default // copy ctxt.GOPATH = "" // disable GOPATH - conf := loader.Config{ - SourceImports: true, - Build: &ctxt, - } + conf := loader.Config{Build: &ctxt} if _, err := conf.FromArgs(buildutil.AllPackages(conf.Build), true); err != nil { t.Errorf("FromArgs failed: %v", err) return @@ -82,9 +80,11 @@ allFuncs := ssautil.AllFunctions(prog) // Check that all non-synthetic functions have distinct names. + // Synthetic wrappers for exported methods should be distinct too, + // except for unexported ones (explained at (*Function).RelString). byName := make(map[string]*ssa.Function) for fn := range allFuncs { - if fn.Synthetic == "" { + if fn.Synthetic == "" || ast.IsExported(fn.Name()) { str := fn.String() prev := byName[str] byName[str] = fn Index: llgo/trunk/third_party/gotools/go/ssa/testdata/valueforexpr.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/testdata/valueforexpr.go +++ llgo/trunk/third_party/gotools/go/ssa/testdata/valueforexpr.go @@ -76,11 +76,11 @@ // 1. Slices print( /*@Slice*/ ([]int{})) print( /*@Alloc*/ (&[]int{})) - print(& /*@Alloc*/ ([]int{})) + print(& /*@Slice*/ ([]int{})) sl1 := /*@Slice*/ ([]int{}) sl2 := /*@Alloc*/ (&[]int{}) - sl3 := & /*@Alloc*/ ([]int{}) + sl3 := & /*@Slice*/ ([]int{}) _, _, _ = sl1, sl2, sl3 _ = /*@Slice*/ ([]int{}) @@ -98,18 +98,18 @@ _, _, _ = arr1, arr2, arr3 _ = /*@UnOp*/ ([1]int{}) - _ = /*@Alloc*/ (& /*@Alloc*/ ([1]int{})) // & optimized away + _ = /*@Alloc*/ (& /*@Alloc*/ ([1]int{})) _ = & /*@Alloc*/ ([1]int{}) // 3. Maps type M map[int]int print( /*@MakeMap*/ (M{})) print( /*@Alloc*/ (&M{})) - print(& /*@Alloc*/ (M{})) + print(& /*@MakeMap*/ (M{})) m1 := /*@MakeMap*/ (M{}) m2 := /*@Alloc*/ (&M{}) - m3 := & /*@Alloc*/ (M{}) + m3 := & /*@MakeMap*/ (M{}) _, _, _ = m1, m2, m3 _ = /*@MakeMap*/ (M{}) Index: llgo/trunk/third_party/gotools/go/ssa/testmain.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/testmain.go +++ llgo/trunk/third_party/gotools/go/ssa/testmain.go @@ -12,8 +12,10 @@ "go/ast" "go/token" "os" + "sort" "strings" + "llvm.org/llgo/third_party/gotools/go/exact" "llvm.org/llgo/third_party/gotools/go/types" ) @@ -100,7 +102,7 @@ Prog: prog, Members: make(map[string]Member), values: make(map[types.Object]Value), - Object: types.NewPackage("testmain", "testmain"), + Object: types.NewPackage("test$main", "main"), } // Build package's init function. @@ -118,25 +120,33 @@ } // Initialize packages to test. + var pkgpaths []string for _, pkg := range pkgs { var v Call v.Call.Value = pkg.init v.setType(types.NewTuple()) init.emit(&v) + + pkgpaths = append(pkgpaths, pkg.Object.Path()) } + sort.Strings(pkgpaths) init.emit(new(Return)) init.finishBody() testmain.init = init testmain.Object.MarkComplete() testmain.Members[init.name] = init - main := &Function{ - name: "main", - Signature: new(types.Signature), - Synthetic: "test main function", - Prog: prog, - Pkg: testmain, - } + // For debugging convenience, define an unexported const + // that enumerates the packages. + packagesConst := types.NewConst(token.NoPos, testmain.Object, "packages", tString, + exact.MakeString(strings.Join(pkgpaths, " "))) + memberFromObject(testmain, packagesConst, nil) + + // Create main *types.Func and *ssa.Function + mainFunc := types.NewFunc(token.NoPos, testmain.Object, "main", new(types.Signature)) + memberFromObject(testmain, mainFunc, nil) + main := testmain.Func("main") + main.Synthetic = "test main function" main.startBody() Index: llgo/trunk/third_party/gotools/go/ssa/util.go =================================================================== --- llgo/trunk/third_party/gotools/go/ssa/util.go +++ llgo/trunk/third_party/gotools/go/ssa/util.go @@ -17,10 +17,6 @@ "llvm.org/llgo/third_party/gotools/go/types" ) -func unreachable() { - panic("unreachable") -} - //// AST utilities func unparen(e ast.Expr) ast.Expr { return astutil.Unparen(e) } @@ -41,11 +37,7 @@ return ok } -// isInterface reports whether T's underlying type is an interface. -func isInterface(T types.Type) bool { - _, ok := T.Underlying().(*types.Interface) - return ok -} +func isInterface(T types.Type) bool { return types.IsInterface(T) } // deref returns a pointer's element type; otherwise it returns typ. func deref(typ types.Type) types.Type { Index: llgo/trunk/third_party/gotools/go/types/assignments.go =================================================================== --- llgo/trunk/third_party/gotools/go/types/assignments.go +++ llgo/trunk/third_party/gotools/go/types/assignments.go @@ -45,7 +45,7 @@ // bool, rune, int, float64, complex128 or string respectively, depending // on whether the value is a boolean, rune, integer, floating-point, complex, // or string constant." - if T == nil || isInterface(T) { + if T == nil || IsInterface(T) { if T == nil && x.typ == Typ[UntypedNil] { check.errorf(x.pos(), "use of untyped nil") x.mode = invalid Index: llgo/trunk/third_party/gotools/go/types/call.go =================================================================== --- llgo/trunk/third_party/gotools/go/types/call.go +++ llgo/trunk/third_party/gotools/go/types/call.go @@ -397,7 +397,7 @@ // includes the methods of typ. // Variables are addressable, so we can always take their // address. - if _, ok := typ.(*Pointer); !ok && !isInterface(typ) { + if _, ok := typ.(*Pointer); !ok && !IsInterface(typ) { typ = &Pointer{base: typ} } } Index: llgo/trunk/third_party/gotools/go/types/conversions.go =================================================================== --- llgo/trunk/third_party/gotools/go/types/conversions.go +++ llgo/trunk/third_party/gotools/go/types/conversions.go @@ -54,7 +54,7 @@ // use the default type (e.g., []byte("foo") should report string // not []byte as type for the constant "foo"). // - Keep untyped nil for untyped nil arguments. - if isInterface(T) || constArg && !isConstType(T) { + if IsInterface(T) || constArg && !isConstType(T) { final = defaultType(x.typ) } check.updateExprType(x.expr, final, true) Index: llgo/trunk/third_party/gotools/go/types/expr.go =================================================================== --- llgo/trunk/third_party/gotools/go/types/expr.go +++ llgo/trunk/third_party/gotools/go/types/expr.go @@ -998,6 +998,7 @@ } } if typ == nil { + // TODO(gri) provide better error messages depending on context check.error(e.Pos(), "missing type in composite literal") goto Error } @@ -1057,7 +1058,12 @@ break // cannot continue } // i < len(fields) - etyp := fields[i].typ + fld := fields[i] + if !fld.Exported() && fld.pkg != check.pkg { + check.errorf(x.pos(), "implicit assignment to unexported field %s in %s literal", fld.name, typ) + continue + } + etyp := fld.typ if !check.assignment(x, etyp) { if x.mode != invalid { check.errorf(x.pos(), "cannot use %s as %s value in struct literal", x, etyp) @@ -1089,7 +1095,7 @@ check.error(e.Pos(), "missing key in map literal") continue } - check.expr(x, kv.Key) + check.exprWithHint(x, kv.Key, utyp.key) if !check.assignment(x, utyp.key) { if x.mode != invalid { check.errorf(x.pos(), "cannot use %s as %s key in map literal", x, utyp.key) Index: llgo/trunk/third_party/gotools/go/types/predicates.go =================================================================== --- llgo/trunk/third_party/gotools/go/types/predicates.go +++ llgo/trunk/third_party/gotools/go/types/predicates.go @@ -71,7 +71,8 @@ return ok && t.info&IsConstType != 0 } -func isInterface(typ Type) bool { +// IsInterface reports whether typ is an interface type. +func IsInterface(typ Type) bool { _, ok := typ.Underlying().(*Interface) return ok } Index: llgo/trunk/third_party/gotools/go/types/stmt.go =================================================================== --- llgo/trunk/third_party/gotools/go/types/stmt.go +++ llgo/trunk/third_party/gotools/go/types/stmt.go @@ -456,7 +456,14 @@ check.invalidAST(s.Pos(), "incorrect form of type switch guard") return } - check.recordDef(lhs, nil) // lhs variable is implicitly declared in each cause clause + + if lhs.Name == "_" { + // _ := x.(type) is an invalid short variable declaration + check.softErrorf(lhs.Pos(), "no new variable on left side of :=") + lhs = nil // avoid declared but not used error below + } else { + check.recordDef(lhs, nil) // lhs variable is implicitly declared in each cause clause + } rhs = guard.Rhs[0] @@ -604,60 +611,49 @@ defer check.closeScope() // check expression to iterate over - decl := s.Tok == token.DEFINE var x operand check.expr(&x, s.X) - if x.mode == invalid { - // if we don't have a declaration, we can still check the loop's body - // (otherwise we can't because we are missing the declared variables) - if !decl { - check.stmt(inner, s.Body) - } - return - } // determine key/value types var key, val Type - switch typ := x.typ.Underlying().(type) { - case *Basic: - if isString(typ) { + if x.mode != invalid { + switch typ := x.typ.Underlying().(type) { + case *Basic: + if isString(typ) { + key = Typ[Int] + val = UniverseRune // use 'rune' name + } + case *Array: key = Typ[Int] - val = UniverseRune // use 'rune' name - } - case *Array: - key = Typ[Int] - val = typ.elem - case *Slice: - key = Typ[Int] - val = typ.elem - case *Pointer: - if typ, _ := typ.base.Underlying().(*Array); typ != nil { + val = typ.elem + case *Slice: key = Typ[Int] val = typ.elem - } - case *Map: - key = typ.key - val = typ.elem - case *Chan: - key = typ.elem - val = Typ[Invalid] - if typ.dir == SendOnly { - check.errorf(x.pos(), "cannot range over send-only channel %s", &x) - // ok to continue - } - if s.Value != nil { - check.errorf(s.Value.Pos(), "iteration over %s permits only one iteration variable", &x) - // ok to continue + case *Pointer: + if typ, _ := typ.base.Underlying().(*Array); typ != nil { + key = Typ[Int] + val = typ.elem + } + case *Map: + key = typ.key + val = typ.elem + case *Chan: + key = typ.elem + val = Typ[Invalid] + if typ.dir == SendOnly { + check.errorf(x.pos(), "cannot range over send-only channel %s", &x) + // ok to continue + } + if s.Value != nil { + check.errorf(s.Value.Pos(), "iteration over %s permits only one iteration variable", &x) + // ok to continue + } } } if key == nil { check.errorf(x.pos(), "cannot range over %s", &x) - // if we don't have a declaration, we can still check the loop's body - if !decl { - check.stmt(inner, s.Body) - } - return + // ok to continue } // check assignment to/declaration of iteration variables @@ -665,9 +661,9 @@ // lhs expressions and initialization value (rhs) types lhs := [2]ast.Expr{s.Key, s.Value} - rhs := [2]Type{key, val} + rhs := [2]Type{key, val} // key, val may be nil - if decl { + if s.Tok == token.DEFINE { // short variable declaration; variable scope starts after the range clause // (the for loop opens a new scope, so variables on the lhs never redeclare // previously declared variables) @@ -694,10 +690,15 @@ } // initialize lhs variable - x.mode = value - x.expr = lhs // we don't have a better rhs expression to use here - x.typ = rhs[i] - check.initVar(obj, &x, false) + if typ := rhs[i]; typ != nil { + x.mode = value + x.expr = lhs // we don't have a better rhs expression to use here + x.typ = typ + check.initVar(obj, &x, false) + } else { + obj.typ = Typ[Invalid] + obj.used = true // don't complain about unused variable + } } // declare variables @@ -714,10 +715,12 @@ if lhs == nil { continue } - x.mode = value - x.expr = lhs // we don't have a better rhs expression to use here - x.typ = rhs[i] - check.assignVar(lhs, &x) + if typ := rhs[i]; typ != nil { + x.mode = value + x.expr = lhs // we don't have a better rhs expression to use here + x.typ = typ + check.assignVar(lhs, &x) + } } } Index: llgo/trunk/third_party/gotools/go/types/testdata/constdecl.src =================================================================== --- llgo/trunk/third_party/gotools/go/types/testdata/constdecl.src +++ llgo/trunk/third_party/gotools/go/types/testdata/constdecl.src @@ -20,17 +20,20 @@ } // Identifier and expression arity must match. -const _ /* ERROR "missing init expr for _" */ +// The first error message is produced by the parser. +// In a real-world scenario, the type-checker would not be run +// in this case and the 2nd error message would not appear. +const _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ const _ = 1, 2 /* ERROR "extra init expr 2" */ -const _ /* ERROR "missing init expr for _" */ int +const _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ int const _ int = 1, 2 /* ERROR "extra init expr 2" */ const ( - _ /* ERROR "missing init expr for _" */ + _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ _ = 1, 2 /* ERROR "extra init expr 2" */ - _ /* ERROR "missing init expr for _" */ int + _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ int _ int = 1, 2 /* ERROR "extra init expr 2" */ ) @@ -51,17 +54,17 @@ ) func _() { - const _ /* ERROR "missing init expr for _" */ + const _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ const _ = 1, 2 /* ERROR "extra init expr 2" */ - const _ /* ERROR "missing init expr for _" */ int + const _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ int const _ int = 1, 2 /* ERROR "extra init expr 2" */ const ( - _ /* ERROR "missing init expr for _" */ + _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ _ = 1, 2 /* ERROR "extra init expr 2" */ - _ /* ERROR "missing init expr for _" */ int + _ /* ERROR "missing constant value" */ /* ERROR "missing init expr for _" */ int _ int = 1, 2 /* ERROR "extra init expr 2" */ ) Index: llgo/trunk/third_party/gotools/go/types/testdata/expr3.src =================================================================== --- llgo/trunk/third_party/gotools/go/types/testdata/expr3.src +++ llgo/trunk/third_party/gotools/go/types/testdata/expr3.src @@ -4,6 +4,8 @@ package expr3 +import "time" + func indexes() { _ = 1 /* ERROR "cannot index" */ [0] _ = indexes /* ERROR "cannot index" */ [0] @@ -26,7 +28,7 @@ a0 = a[0] _ = a0 var a1 int32 - a1 = a /* ERROR "cannot assign" */ [1] + a1 = a /* ERROR "cannot assign" */ [1] _ = a1 _ = a[9] @@ -101,7 +103,6 @@ _, ok = m["bar"] _ = ok - var t string _ = t[- /* ERROR "negative" */ 1] _ = t[- /* ERROR "negative" */ 1 :] @@ -185,7 +186,7 @@ _ = T1{a: 0, s: "foo", u: 0, a /* ERROR "duplicate field" */: 10} _ = T1{a: "foo" /* ERROR "cannot convert" */ } _ = T1{c /* ERROR "unknown field" */ : 0} - _ = T1{T0: { /* ERROR "missing type" */ }} + _ = T1{T0: { /* ERROR "missing type" */ }} // struct literal element type may not be elided _ = T1{T0: T0{}} _ = T1{T0 /* ERROR "invalid field name" */ .a: 0} @@ -201,6 +202,15 @@ x int } _ = P /* ERROR "invalid composite literal type" */ {} + + // unexported fields + _ = time.Time{} + _ = time.Time{sec /* ERROR "unknown field" */ : 0} + _ = time.Time{ + 0 /* ERROR implicit assignment to unexported field sec in time.Time literal */, + 0 /* ERROR implicit assignment */ , + nil /* ERROR implicit assignment */ , + } } func array_literals() { @@ -237,7 +247,7 @@ a0 := [...]int{} assert(len(a0) == 0) - + a1 := [...]int{0, 1, 2} assert(len(a1) == 3) var a13 [3]int @@ -245,7 +255,7 @@ a13 = a1 a14 = a1 /* ERROR "cannot assign" */ _, _ = a13, a14 - + a2 := [...]int{- /* ERROR "negative" */ 1: 0} _ = a2 @@ -255,6 +265,15 @@ a4 := [...]complex128{0, 1, 2, 1<<10-2: -1i, 1i, 400: 10, 12, 14} assert(len(a4) == 1024) + // composite literal element types may be elided + type T []int + _ = [10]T{T{}, {}, 5: T{1, 2, 3}, 7: {1, 2, 3}} + a6 := [...]T{T{}, {}, 5: T{1, 2, 3}, 7: {1, 2, 3}} + assert(len(a6) == 8) + + // recursively so + _ = [10][10]T{{}, [10]T{{}}, {{1, 2, 3}}} + // from the spec type Point struct { x, y float32 } _ = [...]Point{Point{1.5, -3.5}, Point{0, 0}} @@ -298,6 +317,13 @@ _ = S0{f /* ERROR "truncated" */ : 0} _ = S0{s /* ERROR "cannot convert" */ : 0} + // composite literal element types may be elided + type T []int + _ = []T{T{}, {}, 5: T{1, 2, 3}, 7: {1, 2, 3}} + _ = [][]int{{1, 2, 3}, {4, 5}} + + // recursively so + _ = [][]T{{}, []T{{}}, {{1, 2, 3}}} } const index2 int = 2 @@ -342,6 +368,31 @@ var value int _ = M1{true: 1, false: 0} _ = M2{nil: 0, &value: 1} + + // composite literal element types may be elided + type T [2]int + _ = map[int]T{0: T{3, 4}, 1: {5, 6}} + + // recursively so + _ = map[int][]T{0: {}, 1: {{}, T{1, 2}}} + + // composite literal key types may be elided + _ = map[T]int{T{3, 4}: 0, {5, 6}: 1} + + // recursively so + _ = map[[2]T]int{{}: 0, {{}}: 1, [2]T{{}}: 2, {T{1, 2}}: 3} + + // composite literal element and key types may be elided + _ = map[T]T{{}: {}, {1, 2}: T{3, 4}, T{4, 5}: {}} + _ = map[T]M0{{} : {}, T{1, 2}: M0{"foo": 0}, {1, 3}: {"foo": 1}} + + // recursively so + _ = map[[2]T][]T{{}: {}, {{}}: {{}, T{1, 2}}, [2]T{{}}: nil, {T{1, 2}}: {{}, {}}} + + // from the spec + type Point struct { x, y float32 } + _ = map[string]Point{"orig": {0, 0}} + _ = map[*Point]string{{0, 0}: "orig"} } var key2 string = "bar" Index: llgo/trunk/third_party/gotools/go/types/testdata/stmt0.src =================================================================== --- llgo/trunk/third_party/gotools/go/types/testdata/stmt0.src +++ llgo/trunk/third_party/gotools/go/types/testdata/stmt0.src @@ -540,6 +540,7 @@ } switch x /* ERROR "declared but not used" */ := x.(type) {} + switch _ /* ERROR "no new variable on left side of :=" */ := x.(type) {} switch x := x.(type) { case int: @@ -784,6 +785,21 @@ for a, a /* ERROR redeclared */ := range []int{1, 2, 3} { _ = a } } +// Test that despite errors in the range clause, +// the loop body is still type-checked (and thus +// errors reported). +func issue10148() { + for y /* ERROR declared but not used */ := range "" { + _ = "" /* ERROR cannot convert */ + 1 + } + for range 1 /* ERROR cannot range over 1 */ { + _ = "" /* ERROR cannot convert */ + 1 + } + for y := range 1 /* ERROR cannot range over 1 */ { + _ = "" /* ERROR cannot convert */ + 1 + } +} + func labels0() { goto L0 goto L1 Index: llgo/trunk/third_party/gotools/go/types/testdata/vardecl.src =================================================================== --- llgo/trunk/third_party/gotools/go/types/testdata/vardecl.src +++ llgo/trunk/third_party/gotools/go/types/testdata/vardecl.src @@ -14,9 +14,12 @@ var _ int var _, _ int -var _ /* ERROR "missing type or init expr" */ -var _ /* ERROR "missing type or init expr" */, _ -var _ /* ERROR "missing type or init expr" */, _, _ +// The first error message is produced by the parser. +// In a real-world scenario, the type-checker would not be run +// in this case and the 2nd error message would not appear. +var _ /* ERROR "missing variable type" */ /* ERROR "missing type or init expr" */ +var _ /* ERROR "missing variable type" */ /* ERROR "missing type or init expr" */, _ +var _ /* ERROR "missing variable type" */ /* ERROR "missing type or init expr" */, _, _ // The initializer must be an expression. var _ = int /* ERROR "not an expression" */ Index: llgo/trunk/update_third_party.sh =================================================================== --- llgo/trunk/update_third_party.sh +++ llgo/trunk/update_third_party.sh @@ -7,7 +7,7 @@ gccrev=216268 gotoolsrepo=https://go.googlesource.com/tools -gotoolsrev=47f2109c640e97025f36c98610bd9782e815012e +gotoolsrev=d4e70101500b43ffe705d4c45e50dd4f1c8e3b2e tempdir=$(mktemp -d /tmp/update_third_party.XXXXXX) gofrontenddir=$tempdir/gofrontend @@ -84,11 +84,6 @@ find third_party/gotools -name '*.go' | xargs sed -i -e \ 's,"golang.org/x/tools/,"llvm.org/llgo/third_party/gotools/,g' -# Until the version skew between the "go" tool and the compiler is resolved, -# we patch out Go 1.4 specific code in go.tools. -sed -i -e '/go1\.4/ d' third_party/gotools/go/exact/go13.go -rm third_party/gotools/go/exact/go14.go - # --------------------- license check --------------------- # We don't want any GPL licensed code without an autoconf/libtool