go/types: API allows instantiation of instantiated type

[griesemer]

Consider: https://go.dev/play/p/SiV_jaPuzNr
Re-Instantiation of the instantiated type of x in the code works unexpectedly.
Re-Instantiation of the instantiated type of f in the code fails as expected.

This seems inconsistent. Either we permit both or neither.

@findleyr for input

[findleyr]

This is just a bug: there are no type params to substitute in the double-instantiated instance. The underlying is the same as the first instance.
https://go.dev/play/p/6WDvAzt01tY

It seems we are just missing a check that there are no type arguments.

[KirtanSoni]

May I propose a PR or a Plan on this?

[obeyda]

Hi,
It looks like the double instantiation check was removed in CL 333589 when the instantiation logic was moved from subst.go to instantiate.go.

Current state:

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

Lines 128 to 131 in 1763ee1

	switch orig := orig.(type) {
	case *Named:
	res = check.newNamedInstance(pos, orig, targs, expanding) // substituted lazily

I suggest to fix this by reintroducing the check in instantiate.go. For example:

 case *Named:
+    // Verify type parameter count for named types
+    tparams := orig.TypeParams()
+    if !check.validateTArgLen(pos, orig.obj.Name(), tparams.Len(), len(targs)) {
+        return Typ[Invalid]
+    }
+    if tparams.Len() == 0 {
+        // No type parameters to substitute (orig is not generic or already instantiated)
+        return orig  // nothing to do
+    }
     res = check.newNamedInstance(pos, orig, targs, expanding) // substituted lazily

This should restore the original behavior and prevent double instantiation. Happy to work on a CL if this approach looks acceptable.

[gopherbot]

Change https://go.dev/cl/666115 mentions this issue: go/types, types2: add missing generic type test to Checker.Instantiate

[gopherbot]

Change https://go.dev/cl/666435 mentions this issue: go/types, types2: add tests for duplicate instantiation