Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile/internal/types: add Type.UniqueString method #34250

Open
mdempsky opened this issue Sep 12, 2019 · 1 comment

Comments

@mdempsky
Copy link
Member

commented Sep 12, 2019

Currently, Type has three methods for converting into a string: String, LongString, and ShortString.

However, none of them uniquely identify the type. That is, there's currently no FooString such that t1.FooString() == t1.FooString() if and only if Identical(t1, t2).

I also suspect that many (if not all) of the current uses of Type.{Short,Long}String would be better served by a Type.UniqueString method.

Incidentally, ShortString and LongString are oddly named: LongString qualifies identifiers with merely the package name, whereas ShortString uses the full package path (thus generating longer strings than LongString normally does).

Currently, ShortString seems like the closest contender for UniqueString, but there are at least few issues I've noticed so far:

  1. For methods, ShortString includes a "method(ReceverType) " prefix, which doesn't affect type identity.
  2. For structs with blank fields, ShortString prints the field name as "_", whereas blank needs to be package-qualified because the package where it appeared affects type identity.
    This seems to be because of the if s.Name == "_" { return "_" } special case in sconv.
  3. For structs that embed type aliases, we currently lose the aliased name (e.g., given type myint = int, then struct { myint } prints as simply struct { int }). There's no Go syntax to handle this, so probably we'll need to invent something like struct { myint = int }. (We can't format as struct { myint int }, because that's another distinct type.)

(I suspect the latter two can be turned into contrived linker failures today.)

@mdempsky

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

Ugh, the other caveat to ShortString is that for localpkg symbols, we qualify with "" as the package path.

This works is okay for string comparisons within a single compilation unit, but it doesn't provide stable string descriptions across compilation units. So it can't be used for type hashes (at least not the ones used for type switch tables), and probably not for linker symbols. (I forget how/when exactly the linker replaces "".)

It's quite annoying that we can't rely on myimportpath. :(

@toothrot toothrot added this to the Go1.14 milestone Sep 16, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.