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

Closed
mdempsky opened this issue Sep 12, 2019 · 2 comments
Closed

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

mdempsky opened this issue Sep 12, 2019 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mdempsky
Copy link
Member

mdempsky 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
Copy link
Member Author

mdempsky 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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 16, 2019
@toothrot toothrot added this to the Go1.14 milestone Sep 16, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@mdempsky
Copy link
Member Author

We have Type.LinkString and Type.NameString now, which have better documentation on their semantics. I think this can be closed.

@golang golang locked and limited conversation to collaborators Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants