go/types: compute effective Go version independent of token.Pos

Previously, the Checker.allowVersion method would use a token.Pos
to try to infer which file of the current package the checker
was "in". This proved fragile when type-checking syntax that
had been modified or synthesized and whose positions were invalid.

This change records the effective version in the checker state
(checker.environment.version). Just like other aspects of the
environment, the version changes from one file to the next
and must be saved and restored with each check.later closure.

Similarly, declInfo captures and temporarily reinstates
the effective version when checking each object.

+ Test of position independence in go/types and types2
+ Test of panic avoidance in go/types

Fixes #69477
Fixes #69338

Change-Id: Ic06f9d88151c64a4f7848f8942d08e3c312cdd6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/613735
Reviewed-by: Robert Griesemer <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

src/cmd/compile/internal/types2/api_predicates.go

@@ -19,7 +19,7 @@ func AssertableTo(V *Interface, T Type) bool {
 	if !isValid(T.Underlying()) {
 		return false
 	}
-	return (*Checker)(nil).newAssertableTo(nopos, V, T, nil)
+	return (*Checker)(nil).newAssertableTo(V, T, nil)
 }
 
 // AssignableTo reports whether a value of type V is assignable to a variable
@@ -57,15 +57,15 @@ func Implements(V Type, T *Interface) bool {
 	if !isValid(V.Underlying()) {
 		return false
 	}
-	return (*Checker)(nil).implements(nopos, V, T, false, nil)
+	return (*Checker)(nil).implements(V, T, false, nil)
 }
 
 // Satisfies reports whether type V satisfies the constraint T.
 //
 // The behavior of Satisfies is unspecified if V is Typ[Invalid] or an uninstantiated
 // generic type.
 func Satisfies(V Type, T *Interface) bool {
-	return (*Checker)(nil).implements(nopos, V, T, true, nil)
+	return (*Checker)(nil).implements(V, T, true, nil)
 }
 
 // Identical reports whether x and y are identical types.

src/cmd/compile/internal/types2/api_test.go

@@ -3206,3 +3206,35 @@ func TestAnyHijacking_Check(t *testing.T) {
 		})
 	}
 }
+
+// This test function only exists for go/types.
+// func TestVersionIssue69477(t *testing.T)
+
+// TestVersionWithoutPos is a regression test for issue #69477,
+// in which the type checker would use position information
+// to compute which file it is "in" based on syntax position.
+//
+// As a rule the type checker should not depend on position
+// information for correctness, only for error messages and
+// Object.Pos. (Scope.LookupParent was a mistake.)
+//
+// The Checker now holds the effective version in a state variable.
+func TestVersionWithoutPos(t *testing.T) {
+	f := mustParse("//go:build go1.22\n\npackage p; var _ int")
+
+	// Splice in a decl from another file. Its pos will be wrong.
+	f.DeclList[0] = mustParse("package q; func _(s func(func() bool)) { for range s {} }").DeclList[0]
+
+	// Type check. The checker will consult the effective
+	// version (1.22) for the for-range stmt to know whether
+	// range-over-func are permitted: they are not.
+	// (Previously, no error was reported.)
+	pkg := NewPackage("p", "p")
+	check := NewChecker(&Config{}, pkg, nil)
+	err := check.Files([]*syntax.File{f})
+	got := fmt.Sprint(err)
+	want := "range over s (variable of type func(func() bool)): requires go1.23"
+	if !strings.Contains(got, want) {
+		t.Errorf("check error was %q, want substring %q", got, want)
+	}
+}

src/cmd/compile/internal/types2/call.go

@@ -87,7 +87,7 @@ func (check *Checker) funcInst(T *target, pos syntax.Pos, x *operand, inst *synt
 		var params []*Var
 		var reverse bool
 		if T != nil && sig.tparams != nil {
-			if !versionErr && !check.allowVersion(instErrPos, go1_21) {
+			if !versionErr && !check.allowVersion(go1_21) {
 				if inst != nil {
 					check.versionErrorf(instErrPos, go1_21, "partially instantiated function in assignment")
 				} else {
@@ -372,7 +372,7 @@ func (check *Checker) genericExprList(elist []syntax.Expr) (resList []*operand,
 	// nor permitted. Checker.funcInst must infer missing type arguments in that case.
 	infer := true // for -lang < go1.21
 	n := len(elist)
-	if n > 0 && check.allowVersion(elist[0], go1_21) {
+	if n > 0 && check.allowVersion(go1_21) {
 		infer = false
 	}
 
@@ -542,7 +542,7 @@ func (check *Checker) arguments(call *syntax.CallExpr, sig *Signature, targs []T
 	// collect type parameters of callee
 	n := sig.TypeParams().Len()
 	if n > 0 {
-		if !check.allowVersion(call.Pos(), go1_18) {
+		if !check.allowVersion(go1_18) {
 			if iexpr, _ := call.Fun.(*syntax.IndexExpr); iexpr != nil {
 				check.versionErrorf(iexpr, go1_18, "function instantiation")
 			} else {

src/cmd/compile/internal/types2/check.go

@@ -56,6 +56,7 @@ type exprInfo struct {
 type environment struct {
 	decl          *declInfo                 // package-level declaration whose init expression/function body is checked
 	scope         *Scope                    // top-most scope for lookups
+	version       goVersion                 // current accepted language version; changes across files
 	pos           syntax.Pos                // if valid, identifiers are looked up as if at position pos (used by Eval)
 	iota          constant.Value            // value of iota in a constant declaration; nil otherwise
 	errpos        syntax.Pos                // if valid, identifier position of a constant with inherited initializer
@@ -90,8 +91,9 @@ type dotImportKey struct {
 
 // An action describes a (delayed) action.
 type action struct {
-	f    func()      // action to be executed
-	desc *actionDesc // action description; may be nil, requires debug to be set
+	version goVersion   // applicable language version
+	f       func()      // action to be executed
+	desc    *actionDesc // action description; may be nil, requires debug to be set
 }
 
 // If debug is set, describef sets a printf-formatted description for action a.
@@ -119,10 +121,9 @@ type Checker struct {
 	ctxt *Context // context for de-duplicating instances
 	pkg  *Package
 	*Info
-	version goVersion              // accepted language version
-	nextID  uint64                 // unique Id for type parameters (first valid Id is 1)
-	objMap  map[Object]*declInfo   // maps package-level objects and (non-interface) methods to declaration info
-	impMap  map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
+	nextID uint64                 // unique Id for type parameters (first valid Id is 1)
+	objMap map[Object]*declInfo   // maps package-level objects and (non-interface) methods to declaration info
+	impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
 	// see TODO in validtype.go
 	// valids  instanceLookup      // valid *Named (incl. instantiated) types per the validType check
 
@@ -140,7 +141,7 @@ type Checker struct {
 	// (initialized by Files, valid only for the duration of check.Files;
 	// maps and lists are allocated on demand)
 	files         []*syntax.File             // list of package files
-	versions      map[*syntax.PosBase]string // maps files to version strings (each file has an entry); shared with Info.FileVersions if present
+	versions      map[*syntax.PosBase]string // maps files to version strings (each file has an entry); shared with Info.FileVersions if present; may be unaltered Config.GoVersion
 	imports       []*PkgName                 // list of imported packages
 	dotImportMap  map[dotImportKey]*PkgName  // maps dot-imported objects to the package they were dot-imported through
 	brokenAliases map[*TypeName]bool         // set of aliases with broken (not yet determined) types
@@ -219,7 +220,7 @@ func (check *Checker) rememberUntyped(e syntax.Expr, lhs bool, mode operandMode,
 // via action.describef for debugging, if desired.
 func (check *Checker) later(f func()) *action {
 	i := len(check.delayed)
-	check.delayed = append(check.delayed, action{f: f})
+	check.delayed = append(check.delayed, action{version: check.version, f: f})
 	return &check.delayed[i]
 }
 
@@ -268,13 +269,12 @@ func NewChecker(conf *Config, pkg *Package, info *Info) *Checker {
 	// (previously, pkg.goVersion was mutated here: go.dev/issue/61212)
 
 	return &Checker{
-		conf:    conf,
-		ctxt:    conf.Context,
-		pkg:     pkg,
-		Info:    info,
-		version: asGoVersion(conf.GoVersion),
-		objMap:  make(map[Object]*declInfo),
-		impMap:  make(map[importKey]*Package),
+		conf:   conf,
+		ctxt:   conf.Context,
+		pkg:    pkg,
+		Info:   info,
+		objMap: make(map[Object]*declInfo),
+		impMap: make(map[importKey]*Package),
 	}
 }
 
@@ -321,10 +321,10 @@ func (check *Checker) initFiles(files []*syntax.File) {
 	}
 	check.versions = versions
 
-	pkgVersionOk := check.version.isValid()
-	if pkgVersionOk && len(files) > 0 && check.version.cmp(go_current) > 0 {
+	pkgVersion := asGoVersion(check.conf.GoVersion)
+	if pkgVersion.isValid() && len(files) > 0 && pkgVersion.cmp(go_current) > 0 {
 		check.errorf(files[0], TooNew, "package requires newer Go version %v (application built with %v)",
-			check.version, go_current)
+			pkgVersion, go_current)
 	}
 
 	// determine Go version for each file
@@ -479,6 +479,7 @@ func (check *Checker) processDelayed(top int) {
 	// are processed in a delayed fashion) that may
 	// add more actions (such as nested functions), so
 	// this is a sufficiently bounded process.
+	savedVersion := check.version
 	for i := top; i < len(check.delayed); i++ {
 		a := &check.delayed[i]
 		if check.conf.Trace {
@@ -488,13 +489,15 @@ func (check *Checker) processDelayed(top int) {
 				check.trace(nopos, "-- delayed %p", a.f)
 			}
 		}
-		a.f() // may append to check.delayed
+		check.version = a.version // reestablish the effective Go version captured earlier
+		a.f()                     // may append to check.delayed
 		if check.conf.Trace {
 			fmt.Println()
 		}
 	}
 	assert(top <= len(check.delayed)) // stack must not have shrunk
 	check.delayed = check.delayed[:top]
+	check.version = savedVersion
 }
 
 // cleanup runs cleanup for all collected cleaners.

src/cmd/compile/internal/types2/conversions.go

@@ -200,7 +200,7 @@ func (x *operand) convertibleTo(check *Checker, T Type, cause *string) bool {
 		switch a := Tu.(type) {
 		case *Array:
 			if Identical(s.Elem(), a.Elem()) {
-				if check == nil || check.allowVersion(x, go1_20) {
+				if check == nil || check.allowVersion(go1_20) {
 					return true
 				}
 				// check != nil
@@ -213,7 +213,7 @@ func (x *operand) convertibleTo(check *Checker, T Type, cause *string) bool {
 		case *Pointer:
 			if a, _ := under(a.Elem()).(*Array); a != nil {
 				if Identical(s.Elem(), a.Elem()) {
-					if check == nil || check.allowVersion(x, go1_17) {
+					if check == nil || check.allowVersion(go1_17) {
 						return true
 					}
 					// check != nil

src/cmd/compile/internal/types2/decl.go

@@ -169,9 +169,7 @@ func (check *Checker) objDecl(obj Object, def *TypeName) {
 	defer func(env environment) {
 		check.environment = env
 	}(check.environment)
-	check.environment = environment{
-		scope: d.file,
-	}
+	check.environment = environment{scope: d.file, version: d.version}
 
 	// Const and var declarations must not have initialization
 	// cycles. We track them by remembering the current declaration

src/cmd/compile/internal/types2/infer.go

@@ -110,7 +110,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type,
 	// Unify parameter and argument types for generic parameters with typed arguments
 	// and collect the indices of generic parameters with untyped arguments.
 	// Terminology: generic parameter = function parameter with a type-parameterized type
-	u := newUnifier(tparams, targs, check.allowVersion(pos, go1_21))
+	u := newUnifier(tparams, targs, check.allowVersion(go1_21))
 
 	errorf := func(tpar, targ Type, arg *operand) {
 		// provide a better error message if we can

src/cmd/compile/internal/types2/instantiate.go

@@ -82,6 +82,8 @@ func Instantiate(ctxt *Context, orig Type, targs []Type, validate bool) (Type, e
 // must be non-nil.
 //
 // For Named types the resulting instance may be unexpanded.
+//
+// check may be nil (when not type-checking syntax); pos is used only only if check is non-nil.
 func (check *Checker) instance(pos syntax.Pos, orig genericType, targs []Type, expanding *Named, ctxt *Context) (res Type) {
 	// The order of the contexts below matters: we always prefer instances in the
 	// expanding instance context in order to preserve reference cycles.
@@ -198,6 +200,7 @@ func (check *Checker) validateTArgLen(pos syntax.Pos, name string, want, got int
 	panic(fmt.Sprintf("%v: %s", pos, msg))
 }
 
+// check may be nil; pos is used only if check is non-nil.
 func (check *Checker) verify(pos syntax.Pos, tparams []*TypeParam, targs []Type, ctxt *Context) (int, error) {
 	smap := makeSubstMap(tparams, targs)
 	for i, tpar := range tparams {
@@ -209,7 +212,7 @@ func (check *Checker) verify(pos syntax.Pos, tparams []*TypeParam, targs []Type,
 		// the parameterized type.
 		bound := check.subst(pos, tpar.bound, smap, nil, ctxt)
 		var cause string
-		if !check.implements(pos, targs[i], bound, true, &cause) {
+		if !check.implements(targs[i], bound, true, &cause) {
 			return i, errors.New(cause)
 		}
 	}
@@ -222,7 +225,7 @@ func (check *Checker) verify(pos syntax.Pos, tparams []*TypeParam, targs []Type,
 //
 // If the provided cause is non-nil, it may be set to an error string
 // explaining why V does not implement (or satisfy, for constraints) T.
-func (check *Checker) implements(pos syntax.Pos, V, T Type, constraint bool, cause *string) bool {
+func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool {
 	Vu := under(V)
 	Tu := under(T)
 	if !isValid(Vu) || !isValid(Tu) {
@@ -295,7 +298,7 @@ func (check *Checker) implements(pos syntax.Pos, V, T Type, constraint bool, cau
 		// so that ordinary, non-type parameter interfaces implement comparable.
 		if constraint && comparableType(V, true /* spec comparability */, nil, nil) {
 			// V is comparable if we are at Go 1.20 or higher.
-			if check == nil || check.allowVersion(atPos(pos), go1_20) { // atPos needed so that go/types generate passes
+			if check == nil || check.allowVersion(go1_20) {
 				return true
 			}
 			if cause != nil {

src/cmd/compile/internal/types2/literals.go

@@ -16,7 +16,7 @@ import (
 // literal is not compatible with the current language version.
 func (check *Checker) langCompat(lit *syntax.BasicLit) {
 	s := lit.Value
-	if len(s) <= 2 || check.allowVersion(lit, go1_13) {
+	if len(s) <= 2 || check.allowVersion(go1_13) {
 		return
 	}
 	// len(s) > 2

src/cmd/compile/internal/types2/lookup.go

@@ -6,10 +6,7 @@
 
 package types2
 
-import (
-	"bytes"
-	"cmd/compile/internal/syntax"
-)
+import "bytes"
 
 // Internal use of LookupFieldOrMethod: If the obj result is a method
 // associated with a concrete (non-interface) type, the method's signature
@@ -531,14 +528,14 @@ func (check *Checker) assertableTo(V, T Type, cause *string) bool {
 // in constraint position (we have not yet defined that behavior in the spec).
 // The underlying type of V must be an interface.
 // If the result is false and cause is not nil, *cause is set to the error cause.
-func (check *Checker) newAssertableTo(pos syntax.Pos, V, T Type, cause *string) bool {
+func (check *Checker) newAssertableTo(V, T Type, cause *string) bool {
 	// no static check is required if T is an interface
 	// spec: "If T is an interface type, x.(T) asserts that the
 	//        dynamic type of x implements the interface T."
 	if IsInterface(T) {
 		return true
 	}
-	return check.implements(pos, T, V, false, cause)
+	return check.implements(T, V, false, cause)
 }
 
 // deref dereferences typ if it is a *Pointer (but not a *Named type

src/cmd/compile/internal/types2/operand.go

@@ -307,7 +307,7 @@ func (x *operand) assignableTo(check *Checker, T Type, cause *string) (bool, Cod
 	// Also handle the case where T is a pointer to an interface so that we get
 	// the Checker.implements error cause.
 	if _, ok := Tu.(*Interface); ok && Tp == nil || isInterfacePtr(Tu) {
-		if check.implements(x.Pos(), V, T, false, cause) {
+		if check.implements(V, T, false, cause) {
 			return true, 0
 		}
 		// V doesn't implement T but V may still be assignable to T if V
@@ -322,7 +322,7 @@ func (x *operand) assignableTo(check *Checker, T Type, cause *string) (bool, Cod
 
 	// If V is an interface, check if a missing type assertion is the problem.
 	if Vi, _ := Vu.(*Interface); Vi != nil && Vp == nil {
-		if check.implements(x.Pos(), T, V, false, nil) {
+		if check.implements(T, V, false, nil) {
 			// T implements V, so give hint about type assertion.
 			if cause != nil {
 				*cause = "need type assertion"