code review 9303048: go.exp/go/types: more API tweaks

go.exp/go/types: more API tweaks

- Expose some internal functionality via types.Type
  per discussion: Builtins, Underlying, Deref.
- Removed QualifiedIdent and relatives.
- Document existing API better.

[gri, 2013-05-15 23:04:14]

Hello adonovan@google.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.exp

[adonovan, 2013-05-16 17:04:30]

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go
File go/types/types.go (right):

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode12
go/types/types.go:12: // Underlying returns the underlying type of a type.
Explain what this means?

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode15
go/types/types.go:15: // For a pointer type (or a named type denoting a pointer
type),
"denoting" isn't a spec word.  Is "whose underlying type is a pointer" too
verbose?

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode25
go/types/types.go:25: // and some others. What is the design principle?
Prefer not to add methods if they could in principle be external.  Also, avoid
predicates like IsIdentical to avoid the binary method problem.

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode218
go/types/types.go:218: // TODO(gri): Do we keep ForEach or should we abandon it
in favor or Len and At?
I say abandon, for now.

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode287
go/types/types.go:287: type Builtin struct {
Thanks!

https://codereview.appspot.com/9303048/diff/6016/ssa/util.go
File ssa/util.go (right):

https://codereview.appspot.com/9303048/diff/6016/ssa/util.go#newcode49
ssa/util.go:49: func underlyingType(typ types.Type) types.Type {
You could delete this if you like.  Or leave that for me.
Ditto indirect() and deref() below.

[adonovan, 2013-05-16 17:04:46]

LGTM

[gri, 2013-05-16 23:27:23]

PTAL

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go
File go/types/types.go (right):

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode12
go/types/types.go:12: // Underlying returns the underlying type of a type.
On 2013/05/16 17:04:30, adonovan wrote:
> Explain what this means?

Nah. This is a language term defined in the spec.

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode15
go/types/types.go:15: // For a pointer type (or a named type denoting a pointer
type),
On 2013/05/16 17:04:30, adonovan wrote:
> "denoting" isn't a spec word.  Is "whose underlying type is a pointer" too
> verbose?

"denote" is used on purpose in the spec, see e.g.:
http://tip.golang.org/ref/spec#Declarations_and_scope
A declaration is used to have an identifier denote a type.

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode25
go/types/types.go:25: // and some others. What is the design principle?
On 2013/05/16 17:04:30, adonovan wrote:
> Prefer not to add methods if they could in principle be external.  Also, avoid
> predicates like IsIdentical to avoid the binary method problem.

ACK.

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode218
go/types/types.go:218: // TODO(gri): Do we keep ForEach or should we abandon it
in favor or Len and At?
On 2013/05/16 17:04:30, adonovan wrote:
> I say abandon, for now.

ACK. Perhaps next CL.

https://codereview.appspot.com/9303048/diff/6016/go/types/types.go#newcode287
go/types/types.go:287: type Builtin struct {
On 2013/05/16 17:04:30, adonovan wrote:
> Thanks!
ACK.

https://codereview.appspot.com/9303048/diff/6016/ssa/util.go
File ssa/util.go (right):

https://codereview.appspot.com/9303048/diff/6016/ssa/util.go#newcode49
ssa/util.go:49: func underlyingType(typ types.Type) types.Type {
On 2013/05/16 17:04:30, adonovan wrote:
> You could delete this if you like.  Or leave that for me.
> Ditto indirect() and deref() below.

Done.

[adonovan, 2013-05-17 01:51:03]

LGTM

Great!

[gri, 2013-05-17 05:09:16]

*** Submitted as
https://code.google.com/p/go/source/detail?r=8633f918ffec&repo=exp ***

go.exp/go/types: more API tweaks

- Expose some internal functionality via types.Type
  per discussion: Builtins, Underlying, Deref.
- Removed QualifiedIdent and relatives.
- Document existing API better.

R=adonovan
CC=golang-dev
https://codereview.appspot.com/9303048