code review 7023043: go/types: Steps towards removing ast.Object from export...

go/types: Steps towards removing ast.Object from exported API.

- introduced type Method for methods
- renamed StructField -> Field
- removed ObjList
- methods are not sorted anymore in interfaces (for now)

[gri, 2012-12-28 22:00:45]

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

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

[gri, 2012-12-28 22:02:32]

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

Please take another look.

[adonovan, 2012-12-28 22:17:43]

LGTM

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/expr.go
File src/pkg/go/types/expr.go (right):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/expr.go#new...
src/pkg/go/types/expr.go:114: return // keep multiple entries, lookup will only
return the first entry
This will cause identicalMethods to return false; is this what is wanted?

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/predicates.go
File src/pkg/go/types/predicates.go (right):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/predicates....
src/pkg/go/types/predicates.go:211: // same length and corresponding methods
have identical types.
You could optimise this by specializing the common case in which a[i]==b[i] for
all i and falling back to the map-allocating algorithm for the suffix starting
at the first i for which this check fails.

(It's too bad there's no way to test equality of the slices directly as that's
probably the most common case.)

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (left):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go#ol...
src/pkg/go/types/types.go:13: // TODO(gri) Eventually determine what common Type
functionality should be exported.
func String() string 

would save me hours of typing. :)

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (right):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go#ne...
src/pkg/go/types/types.go:179: type Method struct {
Docstring?

[gri, 2012-12-28 22:28:52]

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/expr.go
File src/pkg/go/types/expr.go (right):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/expr.go#new...
src/pkg/go/types/expr.go:114: return // keep multiple entries, lookup will only
return the first entry
On 2012/12/28 22:17:43, adonovan wrote:
> This will cause identicalMethods to return false; is this what is wanted?
good observation - but that's ok. these interfaces are wrong in the first place.

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/predicates.go
File src/pkg/go/types/predicates.go (right):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/predicates....
src/pkg/go/types/predicates.go:211: // same length and corresponding methods
have identical types.
On 2012/12/28 22:17:43, adonovan wrote:
> You could optimise this by specializing the common case in which a[i]==b[i]
for
> all i and falling back to the map-allocating algorithm for the suffix starting
> at the first i for which this check fails.
> 
> (It's too bad there's no way to test equality of the slices directly as that's
> probably the most common case.)

Yes. I have added a TODO. It's good enough for now. Another option would be to
lazily sort the list and remember that it is sorted.

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (left):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go#ol...
src/pkg/go/types/types.go:13: // TODO(gri) Eventually determine what common Type
functionality should be exported.
On 2012/12/28 22:17:43, adonovan wrote:
> func String() string 
> 
> would save me hours of typing. :)

duly noted

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (right):

https://codereview.appspot.com/7023043/diff/8001/src/pkg/go/types/types.go#ne...
src/pkg/go/types/types.go:179: type Method struct {
On 2012/12/28 22:17:43, adonovan wrote:
> Docstring?

Done.

[gri, 2012-12-28 22:30:42]

*** Submitted as https://code.google.com/p/go/source/detail?r=1e331db5cf11 ***

go/types: Steps towards removing ast.Object from exported API.

- introduced type Method for methods
- renamed StructField -> Field
- removed ObjList
- methods are not sorted anymore in interfaces (for now)

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