go/types: inconsistency in recorded types for instantiations

[findleyr]

This issue describes what is arguably a bug in the go/types API, which we may or may not be able to fix.

Specifically, consider the following generic declaration and instantiations:

func f[P, Q any](P, Q) {}

func _() {
  f(0, "")              // 0 type arguments provided
  f[int](0, "")         // 1 type argument provided
  f[int, string](0, "") // 2 type arguments provided
}

In this example, we record in types.Info.Types the type func(int, string) for the expressions f[int] and f[int, string], but for all f identifiers we record the type func[P, Q any](P, Q).

This is inconsistent, and in hindsight clearlyprobably wrong: types.Info.Types should record the effective type of each node in the syntax tree. In this case, the Fun expression of the call expression must have type func(int, string). types.Info.Types should never contain generic types.

There is some discussion of this at #47916 (comment), where I agreed with @mdempsky that it makes most sense to record the instance for the identifier. Unfortunately, that is not what was implemented, most likely as an artifact of the implementation. Mea culpa.

Now that new forms of inference are being implemented, it is harder to preserve this inconsistency. Can we fix this bug so that we record the instance for f, or must we preserve this behavior as a historical artifact?

On the one hand, this bug has existed for over a year. On the other hand, the current behavior is inconsistent, and likely to cause bugs for tool authors.

CC @griesemer @mdempsky @adonovan @dominikh

[mdempsky]

Is the question here just what Type should be recorded in Info.Types for the f identifier in f(0, "")? I.e., whether it should be the generic type of uninstantiated f, or the inferred type from its usage?

Personally, I'm fine either way. Regardless of what Types[f] holds, it's possible to explicitly get the uninstatiated or instantiated type with minimal extra work:

• For the uninstantiated type, notice that the ast.Expr is an *ast.Ident, look it up in Info.Uses, and get the Origin Object's Type.
• For the instantiated type, notice that the ast.Expr is an *ast.Ident, look it up in Info.Instances.

FWIW, I think golang.org/cl/492455 probably wouldn't have been necessary if Types[f] held the instantiated type.

As for the cases where f is explicitly instantiated with one or more type arguments, I'd still lean towards thinking those Types[f] entries should have the uninstantiated generic type. I can't tell whether you're suggesting they should be changed too.

[findleyr]

Is the question here just what Type should be recorded in Info.Types for the f identifier in f(0, "")
I can't tell whether you're suggesting they should be changed too.

Yes, I would argue that the f identifier in f[int] and f[int, string] should also be the instance. As well as in the explicit instantiation f[int, string] (without a call expression).

[griesemer]

At the moment (with pending CL 494116), we're close to what is being asked here, with the exception @mdempsky is pointing out:

As for the cases where f is explicitly instantiated with one or more type arguments, I'd still lean towards thinking those Types[f] entries should have the uninstantiated generic type. I can't tell whether you're suggesting they should be changed too.

[mdempsky]

Yes, I would argue that the f identifier in f[int] and f[int, string] should also be the instance.

Can you elaborate why? f[int] is only valid because f has generic type. It seems inconsistent to me to record the non-generic type instead.

I understand and see the tension in the implicit instantiation case: there are two interesting types (uninstantiated and inferred instantiation), and only one of them can be recorded in Info.Types.

But for explicit instantiations, I think the identifier should have the generic type and the IndexExpr/IndexListExpr should have the instantiated type. The index expression is the operation that transformed it from generic to instantiated type. It's not like f[int][int] is valid.

[findleyr]

Can you elaborate why? f[int] is only valid because f has generic type. It seems inconsistent to me to record the non-generic type instead.

Hmm, point taken about f[int][int]. My rationale was that it doesn't really make sense to record any generic types in types.Info.Types, since this map records the types of expressions, and expressions cannot be generic. However, in that case the correct course of action was probably not to record anything for the f in f[int, string], instead relying on types.Info.Instances in types.Info.TypeOf. But I think it's too late to make that change now. It's also more likely that people are relying on the current behavior when inspecting an index expression. Perhaps we should just change this in the case of f(0, "").

[griesemer]

@findleyr Is there anything left to do here?

[findleyr]

If we now record the correct type for call.Fun, I think that is good enough.