Some cleanup

Author: grantnelson-wf

build/build.go

@@ -766,8 +766,7 @@ type Session struct {
 	// up to date with input sources and dependencies. In the -w ("watch") mode
 	// must be cleared upon entering watching.
 	UpToDateArchives map[string]*compiler.Archive
-
-	Watcher *fsnotify.Watcher
+	Watcher          *fsnotify.Watcher
 }
 
 // NewSession creates a new GopherJS build session.
@@ -949,9 +948,7 @@ func (s *Session) getSortedSources() []*sources.Sources {
 	for _, srcs := range s.sources {
 		allSources = append(allSources, srcs)
 	}
-	sort.Slice(allSources, func(i, j int) bool {
-		return allSources[i].ImportPath < allSources[j].ImportPath
-	})
+	sources.SortedSourcesSlice(allSources)
 	return allSources
 }
 
@@ -1122,12 +1119,14 @@ func (s *Session) loadPackages(pkg *PackageData) (*sources.Sources, error) {
 
 func (s *Session) compilePackages(rootSrcs *sources.Sources, tContext *types.Context) (*compiler.Archive, error) {
 	for _, srcs := range s.sources {
-		s.compilePackage(srcs, tContext)
+		if _, err := s.compilePackage(srcs, tContext); err != nil {
+			return nil, err
+		}
 	}
 
 	rootArchive, ok := s.UpToDateArchives[rootSrcs.ImportPath]
 	if !ok {
-		// This is simply confirmation that the root package is in the sources map and got compiled.
+		// This is confirmation that the root package is in the sources map and got compiled.
 		return nil, fmt.Errorf(`root package %q not found`, rootSrcs.ImportPath)
 	}
 	return rootArchive, nil
@@ -1154,6 +1153,7 @@ func (s *Session) compilePackage(srcs *sources.Sources, tContext *types.Context)
 	}
 
 	s.UpToDateArchives[srcs.ImportPath] = archive
+
 	return archive, nil
 }
 

compiler/compiler_test.go

@@ -704,9 +704,7 @@ func compileProject(t *testing.T, root *packages.Package, minify bool) map[strin
 	for _, srcs := range allSrcs {
 		sortedSources = append(sortedSources, srcs)
 	}
-	sort.Slice(sortedSources, func(i, j int) bool {
-		return sortedSources[i].ImportPath < sortedSources[j].ImportPath
-	})
+	sources.SortedSourcesSlice(sortedSources)
 	PrepareAllSources(sortedSources, importer, tContext)
 
 	archives := map[string]*Archive{}
@@ -733,6 +731,13 @@ func newTime(seconds float64) time.Time {
 func reloadCompiledProject(t *testing.T, archives map[string]*Archive, rootPkgPath string) map[string]*Archive {
 	t.Helper()
 
+	// TODO(grantnelson-wf): The tests using this function are out-of-date
+	// since they are testing the old archive caching that has been disabled.
+	// At some point, these tests should be updated to test any new caching
+	// mechanism that is implemented or removed. As is this function is faking
+	// the old recursive archive loading that is no longer used since it
+	// doesn't allow cross package analysis for generings.
+
 	buildTime := newTime(5.0)
 	serialized := map[string][]byte{}
 	for path, a := range archives {

compiler/internal/analysis/info.go

@@ -58,10 +58,12 @@ type Info struct {
 	funcLitInfos  map[*ast.FuncLit][]*FuncInfo
 	InitFuncInfo  *FuncInfo // Context for package variable initialization.
 
-	infoImporter InfoImporter // For functions from other packages.
+	infoImporter InfoImporter // To get `Info` for other packages.
 	allInfos     []*FuncInfo
 }
 
+// InfoImporter is used to get the `Info` for another package.
+// The path is the resolved import path of the package to get the `Info` for.
 type InfoImporter func(path string) (*Info, error)
 
 func (info *Info) newFuncInfo(n ast.Node, obj types.Object, typeArgs typesutil.TypeList, resolver *typeparams.Resolver) *FuncInfo {

compiler/internal/analysis/info_test.go

@@ -1576,7 +1576,7 @@ func TestBlocking_IsImportBlocking_ForwardInstances(t *testing.T) {
 }
 
 func TestBlocking_IsImportBlocking_BackwardInstances(t *testing.T) {
-	// This is tests propagation of information across package boundaries.
+	// This tests propagation of information across package boundaries.
 	// `FooBaz` has no instances in it until it is referenced in the `test` package.
 	// That instance information needs to propagate back across the package
 	// boundary to the `other` package. The information for `BazBlocker` and

compiler/package.go

@@ -150,11 +150,18 @@ type flowData struct {
 	endCase   int
 }
 
+// PrepareAllSources prepares all sources for compilation by
+// parsing go linknames, type checking, sorting, simplifying, and
+// performing cross package analysis.
+// The results are stored in the provided sources.
+//
+// All sources must be given at the same time for cross package analysis to
+// work correctly. For consistency, the sources should be sorted by import path.
 func PrepareAllSources(allSources []*sources.Sources, importer sources.Importer, tContext *types.Context) error {
 	// This will be performed recursively for all dependencies so
-	// most of these prepare calls will be no-ops. Since not all packages will
-	// be recursively reached via the root source, we need to prepare them
-	// all here.
+	// most of these prepare calls will be no-ops.
+	// Since some packages might not be recursively reached via the root source,
+	// e.g. runtime, we need to try to prepare them all here.
 	for _, srcs := range allSources {
 		if err := srcs.Prepare(importer, sizes32, tContext); err != nil {
 			return err

compiler/sources/sources.go

@@ -44,19 +44,19 @@ type Sources struct {
 	JSFiles []jsFile.JSFile
 
 	// TypeInfo is the type information this package.
-	// This is nil until PrepareInfo is called.
+	// This is nil until Prepare is called.
 	TypeInfo *analysis.Info
 
 	// Instances is the type parameters instances for this package.
-	// This is nil until PrepareInfo is called.
+	// This is nil until Prepare is called.
 	Instances *typeparams.PackageInstanceSets
 
 	// Package is the type-PrepareInfo package.
-	// This is nil until PrepareInfo is called.
+	// This is nil until Prepare is called.
 	Package *types.Package
 
 	// GoLinknames is the set of Go linknames for this package.
-	// This is nil until PrepareInfo is called.
+	// This is nil until Prepare is called.
 	GoLinknames []linkname.GoLinkname
 }
 
@@ -67,14 +67,13 @@ type Importer func(path, srcDir string) (*Sources, error)
 // determining the type information, go linknames, etc.
 //
 // The importer function is used to import the sources of other packages
-// that are imported by this package being prepared. If the other sources
-// are not prepared when returned by the importer, that package will be
-// prepared as well before continuing on with the current package.
+// that the package being prepared depends on. If the other sources
+// are not prepared when returned by the importer, then that package
+// will be prepared before continuing on with the current package.
 // This is where the recursive nature of the Prepare function comes in.
 //
 // Note that at the end of this call the analysis information
-// has NOT been propagated across packages yet
-// and the source files have not been simplified yet.
+// has NOT been propagated across packages yet.
 func (s *Sources) Prepare(importer Importer, sizes types.Sizes, tContext *types.Context) error {
 	// Skip if the sources have already been prepared.
 	if s.isPrepared() {
@@ -113,6 +112,12 @@ func (s *Sources) Prepare(importer Importer, sizes types.Sizes, tContext *types.
 	return nil
 }
 
+// isPrepared returns true if this sources have had Prepare called on it.
+//
+// This can not determine if the type information has been propagated
+// across packages yet, but usually would only be called prior to that.
+// For the source to be fully prepared for compilation, the type information
+// must be propagated across packages as well.
 func (s *Sources) isPrepared() bool {
 	return s.TypeInfo != nil && s.Package != nil
 }
@@ -132,8 +137,8 @@ func (s *Sources) sort() {
 // Note this function mutates the original Files slice.
 // This must be called after TypeCheck and before analyze since
 // this will change the pointers in the AST, for example the pointers
-// to function literals will change making it impossible to find them
-// in the type information if analyze is called first.
+// to function literals will change, making it impossible to find them
+// in the type information, if analyze is called first.
 func (s *Sources) simplify(typesInfo *types.Info) {
 	for i, file := range s.Files {
 		s.Files[i] = astrewrite.Simplify(file, typesInfo, false)
@@ -143,7 +148,7 @@ func (s *Sources) simplify(typesInfo *types.Info) {
 // typeCheck the sources. Returns information about declared package types and
 // type information for the supplied AST.
 //
-// This must be called prior to simplify.
+// This must be called prior to simplify to get the types.Info used by simplify.
 func (s *Sources) typeCheck(importer Importer, sizes types.Sizes, tContext *types.Context) (*types.Info, error) {
 	const errLimit = 10 // Max number of type checking errors to return.
 
@@ -195,7 +200,9 @@ func (s *Sources) typeCheck(importer Importer, sizes types.Sizes, tContext *type
 // analyze will determine the type parameters instances, blocking,
 // and other type information for the package.
 //
-// This must be called after to simplify.
+// This must be called after to simplify to ensure the pointers
+// in the AST are still valid.
+//
 // Note that at the end of this call the analysis information
 // has NOT been propagated across packages yet.
 func (s *Sources) analyze(typesInfo *types.Info, importer Importer, tContext *types.Context) {
@@ -222,7 +229,6 @@ func (s *Sources) analyze(typesInfo *types.Info, importer Importer, tContext *ty
 // parseGoLinknames extracts all //go:linkname compiler directive from the sources.
 //
 // This will set the GoLinknames field on the Sources struct.
-// This must be called prior to simplify.
 func (s *Sources) parseGoLinknames() error {
 	goLinknames := []linkname.GoLinkname{}
 	var errs errorList.ErrorList
@@ -290,13 +296,22 @@ func (pi *packageImporter) Import(path string) (*types.Package, error) {
 		return nil, err
 	}
 
-	if !srcs.isPrepared() {
-		err := srcs.Prepare(pi.importer, pi.sizes, pi.tContext)
-		if err != nil {
-			pi.Errors = pi.Errors.AppendDistinct(err)
-			return nil, err
-		}
+	// If the source hasn't been prepared yet, prepare it now.
+	// This will recursively prepare all of it's dependencies too.
+	// If the source is already prepared, this will be a no-op.
+	err = srcs.Prepare(pi.importer, pi.sizes, pi.tContext)
+	if err != nil {
+		pi.Errors = pi.Errors.AppendDistinct(err)
+		return nil, err
 	}
 
 	return srcs.Package, nil
 }
+
+// SortedSourcesSlice in place sorts the given slice of Sources by ImportPath.
+// This will not change the order of the files within any Sources.
+func SortedSourcesSlice(sourcesSlice []*Sources) {
+	sort.Slice(sourcesSlice, func(i, j int) bool {
+		return sourcesSlice[i].ImportPath < sourcesSlice[j].ImportPath
+	})
+}