code review 4314054: go/types: New Go type hierarchy implementation for AST.

src/pkg/go/types/types.go

 // Copyright 2011 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// PACKAGE UNDER CONSTRUCTION.
+// PACKAGE UNDER CONSTRUCTION. ANY AND ALL PARTS MAY CHANGE.
 // The types package declares the types used to represent Go types.
 //
 package types
 
 import "go/ast"
 
 
 // All types implement the Type interface.

[rsc, 2011/04/07 16:27:19]

Is the intent to expand this?  If so would the isType()
method then become redundant?  

[gri, 2011/04/08 02:41:32]

For now this is just here to ensure that we only store types into a Type.

 type Type interface {
         isType()
 }
 
 
 // All concrete types embed ImplementsType which
 // ensures that all types implement the Type interface.
 type ImplementsType struct{}
 
 func (t *ImplementsType) isType() {}
 
 
 // A Basic represents a (unnamed) basic type.
-// Predeclared types have a basic type as underlying type.
 type Basic struct {
         ImplementsType
         // TODO(gri) need a field specifying the exact basic type
 }
 
 
 // An Array represents an array type [Len]Elt.
 type Array struct {
         ImplementsType
         Len uint64
         Elt Type
 }
 
 
-// A Slice represents a slice type []Elt or ...Elt.
+// A Slice represents a slice type []Elt.
 type Slice struct {
         ImplementsType
-        Ellipsis bool // true for "...Elt" types

[rsc, 2011/04/07 16:27:19]

The ... is a property of the function type, not a slice type.
I think it's a mistake to stick it here, because any code
that wants to deal with slices is going to have to look at it
and think "hmm, what's going on with this?".  I suggest adding

IsVariadic bool

to the Func instead.

[gri, 2011/04/08 02:41:32]

I am not convinced. ... types behave likes slices inside functions - there is no
difference. Only when passing to the function one has to special case. Seems
like that's one place compared to all the places where a ... type is used like a
slice.

[rsc, 2011/04/08 03:25:44]

Right.  These are arguments for making it a property of the
function and not distinguishing in the slice type itself.
That is, if you parse func(string, ...int), then I am
suggesting to construct something like:

FuncType{
    In: []Type{
        String,
        Slice{Elt: Int},
    },
    IsVariadic: true,
}

The result of parsing func(string, []int) would be identical,
except that it would have IsVariadic: false in the FuncType
(not in the type of the final input parameter).

The "property of the Slice" approach as written in the CL right now
is more suited to the way ... used to be defined, where you could write

func f(args ...int) {
    g(args)
}

func g(args ...int) {}

Then it really did matter that args had this magic "..." bit,
and you needed to know that bit when type-checking its use in the
call to g, so it made sense to be part of args's Slice type.
(This is what 6g used to do.)

But when we changed Go to make args ...int indistinguishable
inside the function from a []int, that "bit in the Slice type"
approach stopped making sense.  In every way, inside the body of f,
args has type []int, not ...int.  So its Slice should be the same
as the Slice for []int.

As one concrete example of how the current code would complicate
things, consider

func f(args ...int) {
    x := args
}

To type check the assignment, the natural thing to do is say
that x's Type = args's Type.  In the "Ellipsis inside Slice"
representation, this assignment is wrong.  The ellipsis bool
must be cleared, introducing a special case any time you 
copy the type of an expression.  (Or it introduces a special
case when computing the type of an expression that refers
to a parameter, if you want to push it down one level.
Somewhere, the Ellipsis bool has to get cleared during the
Slice's traveling from being the type of args to being the
type of x.)

The only time when the type checking algorithm needs to
know that f is variadic is while checking a call to f
or comparing two function types for equality.
Making IsVariadic a property of the FuncType handles both
these uses as well or better than making it a property of
the final argument type, and it avoids the special cases
such as the one involving assignment above.

[gri, 2011/04/08 04:08:59]

ah yes. the assignment property makes it obvious.

-        Elt      Type
+        Elt Type
 }
 
 
 // A Struct represents a struct type struct{...}.
 type Struct struct {
         ImplementsType
-        Fields *ast.Scope // always present

[rsc, 2011/04/07 16:27:19]

I'm surprised to see the ast mixed in here.
I think it would be clearer to separate them
completely, introducing a type Field and making
this []*Field.  It makes sense for the type tree
to have references back to the original ast, but
this representation implies that the original
ast then has references back to this package,
because otherwise you couldn't find out the
type of a field.  I think you'll have more flexibility
if they're not so tightly coupled, the same way we
have more flexibility when we avoid cyclic dependencies.

If this is just a placeholder for something else later,
I'd suggest leaving it out for now to make sure it
doesn't get depended on.

Same applies to the other uses of *ast.Scope here.

[gri, 2011/04/08 02:41:32]

It's a placeholder. Removed for now.

-        // TODO(gri) need to remember order of fields for
-        //           struct type comparisons.
+        // TODO(gri) need to remember fields.
 }
 
 
 // A Pointer represents a pointer type *Base.
 type Pointer struct {
         ImplementsType
         Base Type

[rsc, 2011/04/07 16:27:19]

can this be Elt?

Reflect uses the same name for all of these (Elem)
and it would be nice if the mental translation was
1:1 (Elem -> Elt).

It would be even nicer if this package used the same
speling as reflect (Ptr, Elem) but I'm not going to
push for that.

[gri, 2011/04/08 02:41:32]

It's really not an element. It's base type in the spec. The reflect names are
somewhat based on making the API small. Is it really important?

The AST uses Elt instead of Elem.

[rsc, 2011/04/08 03:25:44]

No, just a would be nice.  Not a big deal.
Sure.

 }
 
 
 // A Func represents a function type func(...) (...).
 type Func struct {
         ImplementsType
-        Params *ast.Scope // always present

[rsc, 2011/04/07 16:27:19]

This is even more surprising as a scope, since
the names involved are not relevant.  I expected
something like

In []Type
Out []Type
IsVariadic bool

[gri, 2011/04/08 02:41:32]

There's a TODO below. This was a placeholder. removed for now.

-        // TODO(gri) need to remember parameter order,
-        //           number of results, receiver, etc.
+        IsVariadic bool
+        // TODO(gri) need to remember parameters.
 }
 
 
 // An Interface represents an interface type interface{...}.
 type Interface struct {
         ImplementsType
-        Methods *ast.Scope // always present

[rsc, 2011/04/07 16:27:19]

Again.

[gri, 2011/04/08 02:41:32]

Done.

+        // TODO(gri) need to remember methods.
 }
 
 
 // A Map represents a map type map[Key]Elt.
 type Map struct {
         ImplementsType
         Key, Elt Type
 }
 
 
 // A Chan represents a channel type chan Elt, <-chan Elt, or chan<-Elt.
 type Chan struct {
         ImplementsType
         Dir ast.ChanDir
         Elt Type
 }
 
 
 // A Name represents a named type as declared in a type declaration.
 type Name struct {
         ImplementsType
         Underlying Type        // nil if not fully declared
         Obj        *ast.Object // corresponding declared object
-        Scope      *ast.Scope  // associated methods (and fields, if any); or nil

[rsc, 2011/04/07 16:27:19]

Again.

[gri, 2011/04/08 02:41:32]

Done.

+        // TODO(gri) need to remember fields and methods.
+}
+
+
+// If typ is a pointer type, Deref returns the pointer's base type;
+// otherwise it returns typ.
+func Deref(typ Type) Type {
+        if typ, ok := typ.(*Pointer); ok {
+                return typ.Base
+        }
+        return typ
 }
 
 
 // Underlying returns the underlying type of a type.
 func Underlying(typ Type) Type {
-        if typ, isName := typ.(*Name); isName {
-                return typ.Underlying
+        if typ, ok := typ.(*Name); ok {
+                utyp := typ.Underlying
+                if _, ok := utyp.(*Basic); ok {
+                        return typ
+                }
+                return utyp
+
         }
         return typ
 }