go/types, types2: remove special case for external types in validType

griesemer · griesemer · commit 9dfd458e64a2 · 2022-01-24T21:27:22.000Z
Because validType doesn't modify global state anymore, there's no need to ignore imported types. When we start tracking type parameters, we need to include imported types because they may contribute to cycles that invalidate a type. This CL effectively reverts CL 202483 (issue golang#35049, which doesn't apply anymore because we don't change the state of imported objects). Preparation for fixing issue golang#48962. For golang#35049. For golang#48962. Change-Id: I06f15575ad197375c74ffd09c222250610186b15 Reviewed-on: https://go-review.googlesource.com/c/go/+/378675 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/src/cmd/compile/internal/types2/validtype.go b/src/cmd/compile/internal/types2/validtype.go
@@ -60,12 +60,6 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
 		// will terminate.
 		t = t.orig
 
-		// don't touch the type if it is from a different package or the Universe scope
-		// (doing so would lead to a race condition - was issue #35049)
-		if t.obj.pkg != check.pkg {
-			return valid
-		}
-
 		// don't report a 2nd error if we already know the type is invalid
 		// (e.g., if a cycle was detected earlier, via under).
 		if t.underlying == Typ[Invalid] {
@@ -76,17 +70,25 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
 		switch check.infoMap[t] {
 		case unknown:
 			check.infoMap[t] = marked
-			check.infoMap[t] = check.validType0(t.fromRHS, append(path, t.obj)) // only types of current package added to path
+			check.infoMap[t] = check.validType0(t.fromRHS, append(path, t.obj))
 		case marked:
 			// cycle detected
 			for i, tn := range path {
+				// Even though validType now can hande cycles through external
+				// types, we can't have cycles through external types because
+				// no such types are detected yet.
+				// TODO(gri) Remove this check once we can detect such cycles,
+				//           and adjust cycleError accordingly.
 				if t.obj.pkg != check.pkg {
 					panic("type cycle via package-external type")
 				}
 				if tn == t.obj {
 					check.cycleError(path[i:])
 					check.infoMap[t] = invalid
-					t.underlying = Typ[Invalid]
+					// don't modify imported types (leads to race condition, see #35049)
+					if t.obj.pkg == check.pkg {
+						t.underlying = Typ[Invalid]
+					}
 					return invalid
 				}
 			}
diff --git a/src/go/types/validtype.go b/src/go/types/validtype.go
@@ -60,12 +60,6 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
 		// will terminate.
 		t = t.orig
 
-		// don't touch the type if it is from a different package or the Universe scope
-		// (doing so would lead to a race condition - was issue #35049)
-		if t.obj.pkg != check.pkg {
-			return valid
-		}
-
 		// don't report a 2nd error if we already know the type is invalid
 		// (e.g., if a cycle was detected earlier, via under).
 		if t.underlying == Typ[Invalid] {
@@ -76,17 +70,25 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
 		switch check.infoMap[t] {
 		case unknown:
 			check.infoMap[t] = marked
-			check.infoMap[t] = check.validType0(t.fromRHS, append(path, t.obj)) // only types of current package added to path
+			check.infoMap[t] = check.validType0(t.fromRHS, append(path, t.obj))
 		case marked:
 			// cycle detected
 			for i, tn := range path {
+				// Even though validType now can hande cycles through external
+				// types, we can't have cycles through external types because
+				// no such types are detected yet.
+				// TODO(gri) Remove this check once we can detect such cycles,
+				//           and adjust cycleError accordingly.
 				if t.obj.pkg != check.pkg {
 					panic("type cycle via package-external type")
 				}
 				if tn == t.obj {
 					check.cycleError(path[i:])
 					check.infoMap[t] = invalid
-					t.underlying = Typ[Invalid]
+					// don't modify imported types (leads to race condition, see #35049)
+					if t.obj.pkg == check.pkg {
+						t.underlying = Typ[Invalid]
+					}
 					return invalid
 				}
 			}
