go/types: inner CompositeLit T{{x}} has no type

[adonovan]

When the type checker encounters an ill-typed composite literal, it does not descend into any inner composite literals (because it has no type to propagate downwards) so they have no recorded type.

Minimal repro in the types playground: var _ = T{{x}}. Observe that the outer has type Invalid, the inner no type at all.

This is the root cause of #68918.

[prattmic]

cc @griesemer @findleyr

[gopherbot]

Change https://go.dev/cl/616616 mentions this issue: go/types, types: always record a type for inner composite literals

[griesemer]

@adonovan FWIW, the above CL addresses this particular issue. Note though that while in T{{x}} both the outer composite literal T{{x}} and the inner {x} now have an (invalid) type recorded with them, the x still won't have a type. This is because in Checker.recordTypeAndValue we don't record anything for operands that are invalid. We could change that but that may possibly have repercussions for tools working with incorrect programs (they may see invalid types where before there were no types).

More generally, it may be very difficult to impossible to provide even invalid types for expressions that cannot be properly type checked by the type checker. It seems that this may be better handled on the tools side, because the work only needs to be done if one expects to deal with incorrect programs in the first place.

[gopherbot]

Change https://go.dev/cl/617475 mentions this issue: gopls/internal/test/marker: update regression test issue68918.txt

[adonovan]

More generally, it may be very difficult to impossible to provide even invalid types for expressions that cannot be properly type checked by the type checker. It seems that this may be better handled on the tools side, because the work only needs to be done if one expects to deal with incorrect programs in the first place.

I was going to disagree, as I had believed that it was (modulo bugs) an invariant that every Expr had a type, but I just audited x/tools and found that nearly 100% of places that look at Info.Types handle a missing entry, or are dominated by a check that type-checking was error-free. This is too high to be a coincidence. Furthermore, I notice in the types playground that it is easy to make an ill-formed expression that has no type. So, I think we have generally been conservative in our tools, and that I made a mistake in filing this issue. The "workaround" in gopls was in fact just the correct defensive code.

Sorry for taking up your time. I do think it would be valuable to record the non-invariant at Info.Types.

[adonovan]

I do think it would be valuable to record the non-invariant at Info.Types.

Huh, we already do:

type Info struct {
	// Types maps expressions to their types, and for constant
	// expressions, also their values. Invalid expressions are
	// omitted. [...]
	Types map[ast.Expr]TypeAndValue

How embarrassing.

[gopherbot]

Change https://go.dev/cl/617416 mentions this issue: x/tools: be defensive after types.Info.Types[expr] lookups