Note: this poses a rather unfortunate problem for switching GODEBUG values inside tests.

Change https://go.dev/cl/580355 mentions this issue: go/types, types: make any, byte, and rune actual Alias types.

@griesemer @adonovan after staring at the implications of this for a minute, I think the only way to do it properly is to insist that the only way to change GODEBUG=gotypesalias is at startup. There's no way that switching in tests can work when universeAny, universeByte, and universeRune are written to types.Universe and types.Typ.

@timothy-king has created a GODEBUG=gotypesalias=1 builder, and we should use that technique for creating a GODEBUG=gotypesalias=0 builder.

Just noticed that the predeclared types any, byte, and rune in go/types.Universe are not actually of type go/types.Alias. This matters: I noticed it while working on type writing for the x/tools inliner.

Why does it matter exactly? The defined types string and int are not actually represented like other defined types astypes.Named, because they don't have a declaration or TypeName. It seems to me that byte falls into a similar category.

I'll put together a fix. I would not be surprised if additional tests fail as a result of this fix, since this would be the first time we have an Alias type in the universe scope.

What would be the fix? What would the TypeName be for a universal Alias?

@griesemer @adonovan after staring at the implications of this for a minute, I think the only way to do it properly is to insist that the only way to change GODEBUG=gotypesalias is at startup. There's no way that switching in tests can work when universeAny, universeByte, and universeRune are written to types.Universe and types.Typ.

Arguably this is evidence that it's not the right fix.

@timothy-king has created a GODEBUG=gotypesalias=1 builder, and we should use that technique for creating a GODEBUG=gotypesalias=0 builder.

We should flip that builder to "0", since we don't need a "1" builder: it's the default at tip.

Why does it matter exactly?

The spec says that they are aliases, and there are various places where we need to check for distinguished pointers to print the right name. (search for types2?.Universe.Lookup("any") to find these special cases). Several of those places also have apologies that the "hack" wouldn't be necessary with proper alias support. Now we have that support.

It is inconsistent for them not to be Aliases. Why shouldn't we fix this properly?

What would be the fix? What would the TypeName for a universal Alias?

A new TypeName? I don't understand the question.

Arguably this is evidence that it's not the right fix.

I don't see how that follows. We have a global that uses the new API. Any time a global is affected by a GODEBUG value, it seems that such a fix would be required. I don't understand the philosophical point.

A new TypeName? I don't understand the question.

Sorry, I was under the misapprehension that all TypeNames had valid positions, but of course that's not true for all the built-in types. Ignore me.

Thanks. Will wait for @griesemer to opine before spending too much more time here. See the associated CL for the proposed fix. It wasn't hard to get go/types/types2 tests passing, and I suspect just a few more changes are required in the compiler / importers.

Do we want to do this? It's now or never, I suppose.

Aside:

The defined types string and int are not actually represented like other defined types astypes.Named, because they don't have a declaration or TypeName

FWIW, I think that's no problem. Basic types are defined types. But byte and rune are aliases, and suffer from the same loss of information as other aliases.

Put differently, don't we eventually want the definition of TypeName.IsAlias to be _, ok := obj.typ.(*Alias); return ok?

Do we want to do this? It's now or never, I suppose.
Put differently, don't we eventually want the definition of TypeName.IsAlias to be _, ok := obj.typ.(*Alias); return ok?

Yeah, it's definitely a simpler representation. The question is how much churn will it generate. Only experience will tell.

Hmm, well byte and rune are distinguished Basic types, i.e. their type carries their name. Therefore it is less important to make this change for them than for any.

I'm worried about introducing more problems with this than solving.
I think byte and rune can be left alone.
The only other type where it matters is any and I think there it only matters for type equality when any is not at the top level (e.g., func(any) and func(interface{}) must be considered identical). Can we just have a special check for this in the identity check and then remove it once we switch solidly to Alias nodes?

To be clearer about this: we could define any as its own type (definition) but then treat it as identical with interface{}. Maybe the way to do this is to (dynamically) introduce an Alias node (or skip it) depending on the current setting if AliasEnabled.

@griesemer we already have special handling for any throughout the toolchain. If we're going to change the representation of any, it should be to Alias. Storing any in a Named seems quite problematic.

Leaving byte and rune alone sounds fine, since they have existing for a long time and in any case have access to their Name (and Basic types are special anyway). But any is a real outlier. It would be nice not to have to special-case any throughout tools that needs to format or rewrite source code.

@findleyr Sounds good. So maybe any is simply always represented with an Alias node?

So maybe any is simply always represented with an Alias node?

@griesemer that would be problematic for tools that still use gotypesalias=0. I think we need to change the representation based on the GODEBUG environment variable at startup. Perhaps we can just get away with inconsistent behavior if GODEBUG is subsequently modified in tests...

This may also be a problem for bootstrapping.

"Once at startup" seems like the only logical way to choose the representation of a global variable, but many of our tests need to call Setenv to 1 or 0 later on, and this may lead to inconsistency.

Perhaps we need to establish and assume that all our tests are run in both modes, so tests need only skip when run with the opposite mode from what they need.

@findleyr Sounds good. So maybe any is simply always represented with an Alias node?

Perhaps it would be less trouble to make this change later, once we've ditched gotypesalias=0 forever. Then there is no mode-dependence; we've just made the representation of any a little more faithful to the spec.

@adonovan I think we should rip the band-aid off now. The sooner we make this change, the sooner we can avoid workarounds like https://go-review.git.corp.google.com/c/tools/+/580835/4/internal/refactor/inline/inline.go#2874.

That workaround seems a lot simpler to me than the problem of initializing a global based on the value of an environment variable that we expect tests to modify.

@adonovan that is just one of several, and it's likely there are more workarounds for any that don't exist yet should.

I don't see why we should wait, and then have to make yet another breaking change in Go 1.24 or Go 1.25.
Less ergonomic testing seems easier than having to make another breaking alias-related change in a future Go version.

I don't see why we should wait, and then have to make yet another breaking change in Go 1.24 or Go 1.25. Less ergonomic testing seems easier than having to make another breaking alias-related change in a future Go version.

My gut feeling is that the less ergonomic testing could be a major hardship for users of go/types, whereas the specific representation of the type any should be a fairly narrow change.

If I understand correctly, the workaround in the above code is just to detect shadowing declarations of any, which should be very rare. If you had forgotten to add the workaround, you would temporarily have a bug, but this bug is not a new problem due to Aliases, it's one that would have already been present in this tool (and similar ones like rename) due to the lack of aliases. And when Aliases come along, the bug would become fixed.

Decision: we're going to switch the implementation of any based on gotypesalias, and hijack Scope.Lookup to do the right thing based on the current debug value.

I'm doing this in the above CL (as well as adding some safeguards against improper concurrent use), but as always it's proving trickier than one would hope. Should be able to get it working tomorrow morning.

Change https://go.dev/cl/582335 mentions this issue: gopls/internal/golang/completion: fix the isEmptyInterface predicate