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

go/types: types.aliases should be exported #22628

Closed
willfaught opened this issue Nov 8, 2017 · 9 comments
Closed

go/types: types.aliases should be exported #22628

willfaught opened this issue Nov 8, 2017 · 9 comments

Comments

@willfaught
Copy link
Contributor

@willfaught willfaught commented Nov 8, 2017

To be clear, this is:

$ go doc -u go/types aliases
var aliases = [...]*Basic{
	{Byte, IsInteger | IsUnsigned, "byte"},
	{Rune, IsInteger, "rune"},
}

Otherwise, there's no pre-defined way to get a Basic value whose String() is "rune". This means you can't get a type, convert it into an equivalent representation, and then convert it back into an equivalent types.Type with the same String(). Simulating the byte/rune types with Named types doesn't work in general because byte/rune can be underlying types, and Named.SetUnderlying will panic if you use a Named argument.

It's implied that all the Basic values are in Typ, but the Basic value you get for byte/rune types isn't. The Typ documentation should warn about this.

@willfaught willfaught changed the title go/types: types.aliases should be elements of types.Typ go/types: types.aliases should be exported Nov 8, 2017
@mvdan
Copy link
Member

@mvdan mvdan commented Nov 8, 2017

Loading

@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Nov 8, 2017

You can use types.Universe.Lookup("rune").Type(): https://play.golang.org/p/4saiCBPITd

Loading

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 8, 2017

That was fast :)

Loading

@mvdan mvdan closed this Nov 8, 2017
@willfaught
Copy link
Contributor Author

@willfaught willfaught commented Nov 8, 2017

Thanks for the workaround. However, the documentation point has yet to be addressed.

Loading

@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Nov 8, 2017

Robert, how about something like this?

// The Typ array contains the canonical Basic type for a given kind.
//
// Because the aliases "rune" and "byte" existed prior to the addition
// of type aliases to the language, there is a single kind for Rune and Int32,
// and a single kind for Byte and Uint8. To obtain the Basic type
// representing one of the named aliases, look it up by name using
// Universe.Lookup(name).Type().
var Typ = ...

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 8, 2017

Sure, sounds good.

What's also missing is a doc string for Universe (and Unsafe, while there).

Care to send a CL? Or should I do it?

Loading

@mvdan mvdan reopened this Nov 8, 2017
@mvdan
Copy link
Member

@mvdan mvdan commented Nov 8, 2017

Apologies for closing early - had not noticed the point on the docs.

Loading

@willfaught
Copy link
Contributor Author

@willfaught willfaught commented Nov 8, 2017

Some suggestions for the doc wording:

The Typ array

Typ is a slice, technically. The doc comment should start with the declaration's name.

there is a single kind for Rune and Int32, and a single kind for Byte and Uint8

It would perhaps be clearer to say that Rune and Int32 are equal, and Byte and Uint8 are equal, and thus in Typ, Rune and Byte correspond with the Basics for Int32 and Uint8, respectively.

To obtain the Basic type representing one of the named aliases

It might be unclear how these Basics don't represent the alias types, since they're technically equal and interchangeable from a type checking point of view. Perhaps it would help to point out how the names for those Basics won't match what's assumed for the Rune and Byte kinds.


Altogether, perhaps something like this:

// Typ contains the canonical Basic for a BasicKind. Since Byte and Rune
// are aliases, the Basics they correspond to do not have matching names.
// To get the Basics for them with matching names, use Universe.Lookup(name).Type().
var Typ = []*Basic{ ...

In any case, thanks for documenting it!

Loading

@griesemer griesemer self-assigned this Dec 20, 2017
@griesemer griesemer added this to the Go1.10 milestone Dec 20, 2017
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 20, 2017

Change https://golang.org/cl/85075 mentions this issue: go/types: document Typ slice

Loading

@gopherbot gopherbot closed this in 5ad3c1c Dec 20, 2017
@golang golang locked and limited conversation to collaborators Dec 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants