Kill off the second pass

Author: jakebailey

compiler/compiler.go

@@ -92,22 +92,21 @@ type compilerContext struct {
 	pkg              *types.Package
 	packageDir       string // directory for this package
 	runtimePkg       *types.Package
-	localTypeNames   map[*types.TypeName]string
+	localTypeNames   typeutil.Map // *types.Named (synthetic local from generic instantiation) -> string
 }
 
 // newCompilerContext returns a new compiler context ready for use, most
 // importantly with a newly created LLVM context and module.
 func newCompilerContext(moduleName string, machine llvm.TargetMachine, config *Config, dumpSSA bool) *compilerContext {
 	c := &compilerContext{
-		Config:         config,
-		DumpSSA:        dumpSSA,
-		difiles:        make(map[string]llvm.Metadata),
-		ditypes:        make(map[types.Type]llvm.Metadata),
-		machine:        machine,
-		targetData:     machine.CreateTargetData(),
-		functionInfos:  map[*ssa.Function]functionInfo{},
-		astComments:    map[string]*ast.CommentGroup{},
-		localTypeNames: map[*types.TypeName]string{},
+		Config:        config,
+		DumpSSA:       dumpSSA,
+		difiles:       make(map[string]llvm.Metadata),
+		ditypes:       make(map[types.Type]llvm.Metadata),
+		machine:       machine,
+		targetData:    machine.CreateTargetData(),
+		functionInfos: map[*ssa.Function]functionInfo{},
+		astComments:   map[string]*ast.CommentGroup{},
 	}
 
 	c.ctx = llvm.NewContext()

compiler/interface.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"go/token"
 	"go/types"
+	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -583,8 +584,19 @@ var basicTypeNames = [...]string{
 //
 // isLocal is true when the type is declared inside a function body.
 // Such types need a per-declaration (or per instantiation) suffix
-// because their printed names are not unique; scanLocalTypes assigns
-// the suffix and stores the result in c.localTypeNames.
+// because their printed names are not unique.
+//
+// Ordinary function-local types (TypeName.Parent() != nil) are
+// disambiguated lazily from their declaration position: every
+// declaration in the package has a distinct (file, line, column)
+// triple, and the position is taken un-//line-adjusted so it is
+// stable across builds. Such types are only nameable inside their
+// declaring package, so the name does not need to agree with anything
+// computed in another package.
+//
+// Synthetic locals (TypeName.Parent() == nil), produced by generic
+// instantiation, are pre-registered by scanLocalTypes because their
+// names must agree across packages that materialize the same instance.
 func (c *compilerContext) getTypeCodeName(t types.Type) (name string, isLocal bool) {
 	switch t := types.Unalias(t).(type) {
 	case *types.Named:
@@ -593,14 +605,19 @@ func (c *compilerContext) getTypeCodeName(t types.Type) (name string, isLocal bo
 			// Package-scope or builtin: the printed name is unique.
 			return "named:" + t.String(), false
 		}
-		// Function-local type. Both ordinary locals (Parent() != nil)
-		// and synthetic locals from generic instantiation
-		// (Parent() == nil) are pre-registered by scanLocalTypes.
-		n, ok := c.localTypeNames[tn]
-		if !ok {
-			panic("compiler: local type " + tn.Name() + " was not registered by scanLocalTypes")
+		if tn.Parent() != nil {
+			// Ordinary function-local type. Use the un-//line-adjusted
+			// declaration position as the disambiguator.
+			pos := c.program.Fset.PositionFor(tn.Pos(), false)
+			return "named:" + t.String() + "$" + filepath.Base(pos.Filename) + ":" + strconv.Itoa(pos.Line) + ":" + strconv.Itoa(pos.Column), true
 		}
-		return "named:" + n, true
+		// Synthetic local from generic instantiation: must have been
+		// pre-registered by scanLocalTypes.
+		v := c.localTypeNames.At(t)
+		if v == nil {
+			panic("compiler: synthetic local type " + tn.Name() + " was not registered by scanLocalTypes")
+		}
+		return "named:" + v.(string), true
 	case *types.Array:
 		s, isLocal := c.getTypeCodeName(t.Elem())
 		return "array:" + strconv.FormatInt(t.Len(), 10) + ":" + s, isLocal
@@ -686,41 +703,40 @@ func (c *compilerContext) getTypeCodeName(t types.Type) (name string, isLocal bo
 	}
 }
 
-// scanLocalTypes assigns names to every function-local named type in
-// the package and stores them in c.localTypeNames. Two flavors are
-// handled:
+// scanLocalTypes assigns names to every synthetic *types.TypeName
+// (TypeName.Parent() == nil) reachable from this package and stores
+// them in c.localTypeNames.
 //
-//  1. Synthetic TypeNames produced by generic instantiation
-//     (TypeName.Parent() == nil). Two instantiations of the same
-//     generic function (e.g. F[int] and F[string]) produce TypeNames
-//     with the same printed name and the same source position, so
-//     each one is named with the enclosing instance's RelString as
-//     prefix. RelString encodes the type arguments, matching Go's
-//     runtime behavior, where F[int].Inner and F[string].Inner are
-//     distinct types even when Inner does not mention the type
-//     parameter.
+// Synthetic TypeNames are produced by generic instantiation: two
+// instantiations of the same generic function (e.g. F[int] and
+// F[string]) produce TypeNames with the same printed name and the
+// same source position, so each is named with the enclosing
+// instance's RelString as prefix. RelString encodes the type
+// arguments, matching Go's runtime behavior, where F[int].Inner and
+// F[string].Inner are distinct types even when Inner does not mention
+// the type parameter.
 //
-//  2. Ordinary function-local TypeNames (TypeName.Parent() != nil).
-//     Each one is named with its declaring function's RelString plus
-//     a per-function counter assigned in source order. This mirrors
-//     the ·N suffix the standard Go compiler uses for such types and
-//     is robust against //line directives that would otherwise make a
-//     file:line:column suffix non-unique.
+// A given instance may be materialized by several packages (the body
+// of F[int] is compiled in every package that calls F[int]); its
+// reflect/types.type:* global has LinkOnceODRLinkage and is merged by
+// name at link time. The chosen name therefore depends only on
+// intrinsic SSA properties (RelString and the raw token.Pos used as a
+// sort key), so any package compiling the same instance produces the
+// same identifier.
 //
-// Names depend only on intrinsic SSA properties (RelString and the
-// raw token.Pos used as a sort key), so any package compiling the
-// same function or instance produces the same identifier.
+// Ordinary function-local TypeNames (TypeName.Parent() != nil) are
+// not handled here: they are nameable only inside their declaring
+// package, and getTypeCodeName derives a stable per-declaration name
+// for them directly from their source position.
 func (c *compilerContext) scanLocalTypes(ssaPkg *ssa.Package) {
-	// Pass 1: locate every generic instance reachable from this
-	// package (including instances declared in imported packages and
-	// any function reached through an instance subtree). Synthetic
-	// TypeNames are produced by instantiation, so we need the call
-	// graph to find them all.
+	// Locate every generic instance reachable from this package
+	// (including instances declared in imported packages and any
+	// function reached through an instance subtree).
 	var instances []*ssa.Function
-	seenInstWalk := map[*ssa.Function]bool{}
-	var instWalk func(fn *ssa.Function, inInstance bool)
-	instWalk = func(fn *ssa.Function, inInstance bool) {
-		if fn == nil || seenInstWalk[fn] {
+	seen := map[*ssa.Function]bool{}
+	var walk func(fn *ssa.Function, inInstance bool)
+	walk = func(fn *ssa.Function, inInstance bool) {
+		if fn == nil || seen[fn] {
 			return
 		}
 		// fn belongs to an instance subtree if it is itself an
@@ -736,13 +752,13 @@ func (c *compilerContext) scanLocalTypes(ssaPkg *ssa.Package) {
 		if fn.Blocks == nil && fn.AnonFuncs == nil {
 			return
 		}
-		seenInstWalk[fn] = true
+		seen[fn] = true
 		isInInstance := inInstance || isInstanceRoot
 		if isInInstance {
 			instances = append(instances, fn)
 		}
 		for _, anon := range fn.AnonFuncs {
-			instWalk(anon, isInInstance)
+			walk(anon, isInInstance)
 		}
 		var ops [10]*ssa.Value
 		for _, b := range fn.Blocks {
@@ -752,7 +768,7 @@ func (c *compilerContext) scanLocalTypes(ssaPkg *ssa.Package) {
 						continue
 					}
 					if callee, ok := (*op).(*ssa.Function); ok {
-						instWalk(callee, isInInstance)
+						walk(callee, isInInstance)
 					}
 				}
 			}
@@ -761,97 +777,46 @@ func (c *compilerContext) scanLocalTypes(ssaPkg *ssa.Package) {
 	for _, member := range ssaPkg.Members {
 		switch m := member.(type) {
 		case *ssa.Function:
-			instWalk(m, false)
+			walk(m, false)
 		case *ssa.Type:
 			mset := c.program.MethodSets.MethodSet(m.Type())
 			for i := 0; i < mset.Len(); i++ {
-				instWalk(c.program.MethodValue(mset.At(i)), false)
+				walk(c.program.MethodValue(mset.At(i)), false)
 			}
 			pmset := c.program.MethodSets.MethodSet(types.NewPointer(m.Type()))
 			for i := 0; i < pmset.Len(); i++ {
-				instWalk(c.program.MethodValue(pmset.At(i)), false)
+				walk(c.program.MethodValue(pmset.At(i)), false)
 			}
 		}
 	}
 
-	// Pass 2: collect every non-instance function defined in this
-	// package together with its closures. Ordinary function-local
-	// TypeNames are scoped to their declaring function (and visible
-	// in nested closures only), so the declaring function is always
-	// somewhere in this lexical tree. Following callees here would
-	// be wrong: an instantiated function whose substituted signature
-	// mentions the local type would otherwise race with the actual
-	// declaring function for ownership.
-	var packageFuncs []*ssa.Function
-	var collect func(fn *ssa.Function)
-	collect = func(fn *ssa.Function) {
-		if fn == nil || fn.Pkg != ssaPkg {
-			return
-		}
-		if len(fn.TypeArgs()) > 0 {
-			// Generic instances are handled by pass 1 (their local
-			// types are synthetic).
-			return
-		}
-		if fn.Blocks == nil && fn.AnonFuncs == nil {
-			return
-		}
-		packageFuncs = append(packageFuncs, fn)
-		for _, anon := range fn.AnonFuncs {
-			collect(anon)
-		}
-	}
-	for _, member := range ssaPkg.Members {
-		switch m := member.(type) {
-		case *ssa.Function:
-			collect(m)
-		case *ssa.Type:
-			mset := c.program.MethodSets.MethodSet(m.Type())
-			for i := 0; i < mset.Len(); i++ {
-				collect(c.program.MethodValue(mset.At(i)))
-			}
-			pmset := c.program.MethodSets.MethodSet(types.NewPointer(m.Type()))
-			for i := 0; i < pmset.Len(); i++ {
-				collect(c.program.MethodValue(pmset.At(i)))
-			}
-		}
-	}
-
-	// Registration is first-writer-wins, so visit each list in a
+	// Registration is first-writer-wins (a synthetic TypeName may be
+	// reachable from several instances), so visit instances in a
 	// deterministic order. Pos() is a defensive tiebreaker.
-	sortFns := func(fns []*ssa.Function) {
-		sort.Slice(fns, func(i, j int) bool {
-			ri, rj := fns[i].RelString(nil), fns[j].RelString(nil)
-			if ri != rj {
-				return ri < rj
-			}
-			return fns[i].Pos() < fns[j].Pos()
-		})
-	}
-	sortFns(instances)
-	sortFns(packageFuncs)
+	sort.Slice(instances, func(i, j int) bool {
+		ri, rj := instances[i].RelString(nil), instances[j].RelString(nil)
+		if ri != rj {
+			return ri < rj
+		}
+		return instances[i].Pos() < instances[j].Pos()
+	})
 	for _, fn := range instances {
-		c.registerLocalTypes(fn, true)
-	}
-	for _, fn := range packageFuncs {
-		c.registerLocalTypes(fn, false)
+		c.registerSyntheticLocalTypes(fn)
 	}
 }
 
-// registerLocalTypes walks every type reachable from fn's body and
-// records each function-local TypeName whose Parent() matches the
-// synthetic flag (Parent() == nil for synthetic, != nil otherwise) in
-// c.localTypeNames. Each TypeName is named with fn.RelString as the
-// owning function plus a per-function counter assigned in source order.
+// registerSyntheticLocalTypes walks every type reachable from fn's
+// body and records each synthetic *types.Named (TypeName.Parent() ==
+// nil) in c.localTypeNames. Each is named with fn.RelString as the
+// owning function plus a per-function counter assigned in source
+// order.
 //
-// First-writer-wins: a TypeName already present in c.localTypeNames
-// is left alone. The slot is reserved with an empty string during
-// collection so later registerLocalTypes calls (within the same
-// scanLocalTypes invocation) skip it; the final name is filled in
-// after sorting, before scanLocalTypes returns and any
-// getTypeCodeName lookups happen.
-func (c *compilerContext) registerLocalTypes(fn *ssa.Function, synthetic bool) {
-	var found []*types.TypeName
+// First-writer-wins: a *types.Named already present in
+// c.localTypeNames is left alone, so a synthetic type reachable from
+// several instances keeps the name assigned by the first (in
+// scanLocalTypes' deterministic order).
+func (c *compilerContext) registerSyntheticLocalTypes(fn *ssa.Function) {
+	var found []*types.Named
 	seen := map[types.Type]bool{}
 	var visit func(t types.Type)
 	visit = func(t types.Type) {
@@ -864,10 +829,14 @@ func (c *compilerContext) registerLocalTypes(fn *ssa.Function, synthetic bool) {
 			visit(types.Unalias(t))
 		case *types.Named:
 			tn := t.Obj()
-			if tn.Pkg() != nil && (tn.Parent() == nil) == synthetic {
-				if _, ok := c.localTypeNames[tn]; !ok {
-					c.localTypeNames[tn] = ""
-					found = append(found, tn)
+			if tn.Pkg() != nil && tn.Parent() == nil {
+				if c.localTypeNames.At(t) == nil {
+					// Reserve the slot so later calls within this
+					// scanLocalTypes invocation skip it; the final
+					// name is filled in after sorting, before any
+					// getTypeCodeName lookups happen.
+					c.localTypeNames.Set(t, "")
+					found = append(found, t)
 				}
 			}
 			targs := t.TypeArgs()
@@ -906,10 +875,10 @@ func (c *compilerContext) registerLocalTypes(fn *ssa.Function, synthetic bool) {
 				visit(t.At(i).Type())
 			}
 		case *types.Interface:
-			// A local type can be reachable only through a local
-			// interface's method signature, so descend into them.
-			// getTypeCodeName encodes those signatures into the
-			// interface's identifier, and the seen map breaks
+			// A synthetic local type can be reachable only through a
+			// local interface's method signature, so descend into
+			// them. getTypeCodeName encodes those signatures into
+			// the interface's identifier, and the seen map breaks
 			// cycles formed by methods that mention the interface
 			// itself.
 			for i := 0; i < t.NumMethods(); i++ {
@@ -946,11 +915,11 @@ func (c *compilerContext) registerLocalTypes(fn *ssa.Function, synthetic bool) {
 	// that is stable across builds and unaffected by //line directives
 	// (which only adjust the human-facing position from Fset.Position).
 	sort.Slice(found, func(i, j int) bool {
-		return found[i].Pos() < found[j].Pos()
+		return found[i].Obj().Pos() < found[j].Obj().Pos()
 	})
 	enclosing := fn.RelString(nil)
-	for i, tn := range found {
-		c.localTypeNames[tn] = enclosing + "." + tn.Name() + "$" + strconv.Itoa(i+1)
+	for i, named := range found {
+		c.localTypeNames.Set(named, enclosing+"."+named.Obj().Name()+"$"+strconv.Itoa(i+1))
 	}
 }