go icon indicating copy to clipboard operation
go copied to clipboard

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

Open adonovan opened this issue 1 year ago • 1 comments

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 https://github.com/golang/go/issues/68918.

adonovan avatar Aug 27 '24 18:08 adonovan

cc @griesemer @findleyr

prattmic avatar Aug 27 '24 19:08 prattmic

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

gopherbot avatar Oct 02 '24 00:10 gopherbot

@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.

griesemer avatar Oct 02 '24 00:10 griesemer

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

gopherbot avatar Oct 02 '24 16:10 gopherbot

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 avatar Oct 02 '24 20:10 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.

adonovan avatar Oct 02 '24 20:10 adonovan

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

gopherbot avatar Oct 02 '24 20:10 gopherbot