code review 6525047: cmd/api: allow extension of interfaces with unexported ...

cmd/api: allow extension of interfaces with unexported methods

Fixes issue 4061.

[rsc, 2012-09-18 18:59:11]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2012-09-18 19:38:46]

LGTM

Thanks.  I kept meaning to do this soon.

On Tue, Sep 18, 2012 at 11:59 AM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> cmd/api: allow extension of interfaces with unexported methods
>
> Fixes issue 4061.
>
> Please review this at
http://codereview.appspot.com/**6525047/<http://codereview.appspot.com/6525047/>
>
> Affected files:
>   M api/go1.txt
>   M src/cmd/api/goapi.go
>   M src/cmd/api/testdata/src/pkg/**p1/golden.txt
>   M src/cmd/api/testdata/src/pkg/**p1/p1.go
>
>
> Index: api/go1.txt
> ==============================**==============================**=======
> --- a/api/go1.txt
> +++ b/api/go1.txt
> @@ -2811,7 +2811,7 @@
>  pkg go/ast, type CompositeLit struct, Lbrace token.Pos
>  pkg go/ast, type CompositeLit struct, Rbrace token.Pos
>  pkg go/ast, type CompositeLit struct, Type Expr
> -pkg go/ast, type Decl interface { End, Pos }
> +pkg go/ast, type Decl interface, unexported methods
>  pkg go/ast, type Decl interface, End() token.Pos
>  pkg go/ast, type Decl interface, Pos() token.Pos
>  pkg go/ast, type DeclStmt struct
> @@ -2824,7 +2824,7 @@
>  pkg go/ast, type Ellipsis struct, Elt Expr
>  pkg go/ast, type EmptyStmt struct
>  pkg go/ast, type EmptyStmt struct, Semicolon token.Pos
> -pkg go/ast, type Expr interface { End, Pos }
> +pkg go/ast, type Expr interface, unexported methods
>  pkg go/ast, type Expr interface, End() token.Pos
>  pkg go/ast, type Expr interface, Pos() token.Pos
>  pkg go/ast, type ExprStmt struct
> @@ -2971,13 +2971,13 @@
>  pkg go/ast, type SliceExpr struct, Low Expr
>  pkg go/ast, type SliceExpr struct, Rbrack token.Pos
>  pkg go/ast, type SliceExpr struct, X Expr
> -pkg go/ast, type Spec interface { End, Pos }
> +pkg go/ast, type Spec interface, unexported methods
>  pkg go/ast, type Spec interface, End() token.Pos
>  pkg go/ast, type Spec interface, Pos() token.Pos
>  pkg go/ast, type StarExpr struct
>  pkg go/ast, type StarExpr struct, Star token.Pos
>  pkg go/ast, type StarExpr struct, X Expr
> -pkg go/ast, type Stmt interface { End, Pos }
> +pkg go/ast, type Stmt interface, unexported methods
>  pkg go/ast, type Stmt interface, End() token.Pos
>  pkg go/ast, type Stmt interface, Pos() token.Pos
>  pkg go/ast, type StructType struct
> @@ -5458,7 +5458,7 @@
>  pkg reflect, type StructField struct, Tag StructTag
>  pkg reflect, type StructField struct, Type Type
>  pkg reflect, type StructTag string
> -pkg reflect, type Type interface { Align, AssignableTo, Bits, ChanDir,
> Elem, Field, FieldAlign, FieldByIndex, FieldByName, FieldByNameFunc,
> Implements, In, IsVariadic, Key, Kind, Len, Method, MethodByName, Name,
> NumField, NumIn, NumMethod, NumOut, Out, PkgPath, Size, String }
> +pkg reflect, type Type interface, unexported methods
>  pkg reflect, type Type interface, Align() int
>  pkg reflect, type Type interface, AssignableTo(Type) bool
>  pkg reflect, type Type interface, Bits() int
> @@ -7608,7 +7608,7 @@
>  pkg syscall (darwin-386), type RouteMessage struct
>  pkg syscall (darwin-386), type RouteMessage struct, Data []byte
>  pkg syscall (darwin-386), type RouteMessage struct, Header RtMsghdr
> -pkg syscall (darwin-386), type RoutingMessage interface {}
> +pkg syscall (darwin-386), type RoutingMessage interface, unexported
> methods
>  pkg syscall (darwin-386), type RtMetrics struct
>  pkg syscall (darwin-386), type RtMetrics struct, Expire int32
>  pkg syscall (darwin-386), type RtMetrics struct, Filler [4]uint32
> @@ -9427,7 +9427,7 @@
>  pkg syscall (darwin-386-cgo), type RouteMessage struct
>  pkg syscall (darwin-386-cgo), type RouteMessage struct, Data []byte
>  pkg syscall (darwin-386-cgo), type RouteMessage struct, Header RtMsghdr
> -pkg syscall (darwin-386-cgo), type RoutingMessage interface {}
> +pkg syscall (darwin-386-cgo), type RoutingMessage interface, unexported
> methods
>  pkg syscall (darwin-386-cgo), type RtMetrics struct
>  pkg syscall (darwin-386-cgo), type RtMetrics struct, Expire int32
>  pkg syscall (darwin-386-cgo), type RtMetrics struct, Filler [4]uint32
> @@ -11249,7 +11249,7 @@
>  pkg syscall (darwin-amd64), type RouteMessage struct
>  pkg syscall (darwin-amd64), type RouteMessage struct, Data []byte
>  pkg syscall (darwin-amd64), type RouteMessage struct, Header RtMsghdr
> -pkg syscall (darwin-amd64), type RoutingMessage interface {}
> +pkg syscall (darwin-amd64), type RoutingMessage interface, unexported
> methods
>  pkg syscall (darwin-amd64), type RtMetrics struct
>  pkg syscall (darwin-amd64), type RtMetrics struct, Expire int32
>  pkg syscall (darwin-amd64), type RtMetrics struct, Filler [4]uint32
> @@ -13075,7 +13075,7 @@
>  pkg syscall (darwin-amd64-cgo), type RouteMessage struct
>  pkg syscall (darwin-amd64-cgo), type RouteMessage struct, Data []byte
>  pkg syscall (darwin-amd64-cgo), type RouteMessage struct, Header RtMsghdr
> -pkg syscall (darwin-amd64-cgo), type RoutingMessage interface {}
> +pkg syscall (darwin-amd64-cgo), type RoutingMessage interface, unexported
> methods
>  pkg syscall (darwin-amd64-cgo), type RtMetrics struct
>  pkg syscall (darwin-amd64-cgo), type RtMetrics struct, Expire int32
>  pkg syscall (darwin-amd64-cgo), type RtMetrics struct, Filler [4]uint32
> @@ -15046,7 +15046,7 @@
>  pkg syscall (freebsd-386), type RouteMessage struct
>  pkg syscall (freebsd-386), type RouteMessage struct, Data []byte
>  pkg syscall (freebsd-386), type RouteMessage struct, Header RtMsghdr
> -pkg syscall (freebsd-386), type RoutingMessage interface {}
> +pkg syscall (freebsd-386), type RoutingMessage interface, unexported
> methods
>  pkg syscall (freebsd-386), type RtMetrics struct
>  pkg syscall (freebsd-386), type RtMetrics struct, Expire uint32
>  pkg syscall (freebsd-386), type RtMetrics struct, Filler [3]uint32
> @@ -17022,7 +17022,7 @@
>  pkg syscall (freebsd-amd64), type RouteMessage struct
>  pkg syscall (freebsd-amd64), type RouteMessage struct, Data []byte
>  pkg syscall (freebsd-amd64), type RouteMessage struct, Header RtMsghdr
> -pkg syscall (freebsd-amd64), type RoutingMessage interface {}
> +pkg syscall (freebsd-amd64), type RoutingMessage interface, unexported
> methods
>  pkg syscall (freebsd-amd64), type RtMetrics struct
>  pkg syscall (freebsd-amd64), type RtMetrics struct, Expire uint64
>  pkg syscall (freebsd-amd64), type RtMetrics struct, Filler [3]uint64
> @@ -30123,7 +30123,7 @@
>  pkg syscall, type RawSockaddrInet4 struct, Port uint16
>  pkg syscall, type Rusage struct
>  pkg syscall, type Signal int
> -pkg syscall, type Sockaddr interface {}
> +pkg syscall, type Sockaddr interface, unexported methods
>  pkg syscall, type SockaddrInet4 struct
>  pkg syscall, type SockaddrInet4 struct, Addr [4]byte
>  pkg syscall, type SockaddrInet4 struct, Port int
> Index: src/cmd/api/goapi.go
> ==============================**==============================**=======
> --- a/src/cmd/api/goapi.go
> +++ b/src/cmd/api/goapi.go
> @@ -892,15 +892,18 @@
>         sig  string // "([]byte) (int, error)", from funcSigString
>  }
>
> -// interfaceMethods returns the expanded list of methods for an interface.
> +// interfaceMethods returns the expanded list of exported methods for an
> interface.
> +// The boolean complete reports whether the list contains all methods
> (that is, the
> +// interface has no unexported methods).
>  // pkg is the complete package name ("net/http")
>  // iname is the interface name.
> -func (w *Walker) interfaceMethods(pkg, iname string) (methods []method) {
> +func (w *Walker) interfaceMethods(pkg, iname string) (methods []method,
> complete bool) {
>         t, ok := w.interfaces[pkgSymbol{pkg, iname}]
>         if !ok {
>                 log.Fatalf("failed to find interface %s.%s", pkg, iname)
>         }
>
> +       complete = true
>         for _, f := range t.Methods.List {
>                 typ := f.Type
>                 switch tv := typ.(type) {
> @@ -912,6 +915,8 @@
>                                                 name: mname.Name,
>                                                 sig:  w.funcSigString(ft),
>                                         })
> +                               } else {
> +                                       complete = false
>                                 }
>                         }
>                 case *ast.Ident:
> @@ -927,7 +932,9 @@
>                                 log.Fatalf("unexported embedded interface
> %q in exported interface %s.%s; confused",
>                                         embedded, pkg, iname)
>                         }
> -                       methods = append(methods, w.interfaceMethods(pkg,
> embedded)...)
> +                       m, c := w.interfaceMethods(pkg, embedded)
> +                       methods = append(methods, m...)
> +                       complete = complete && c
>                 case *ast.SelectorExpr:
>                         lhs := w.nodeString(tv.X)
>                         rhs := w.nodeString(tv.Sel)
> @@ -935,7 +942,9 @@
>                         if !ok {
>                                 log.Fatalf("can't resolve selector %q in
> interface %s.%s", lhs, pkg, iname)
>                         }
> -                       methods = append(methods, w.interfaceMethods(fpkg,
> rhs)...)
> +                       m, c := w.interfaceMethods(fpkg, rhs)
> +                       methods = append(methods, m...)
> +                       complete = complete && c
>                 default:
>                         log.Fatalf("unknown type %T in interface field",
> typ)
>                 }
> @@ -945,14 +954,28 @@
>
>  func (w *Walker) walkInterfaceType(name string, t *ast.InterfaceType) {
>         methNames := []string{}
> -
>         pop := w.pushScope("type " + name + " interface")
> -       for _, m := range w.interfaceMethods(w.**curPackageName, name) {
> +       methods, complete := w.interfaceMethods(w.**curPackageName, name)
> +       for _, m := range methods {
>                 methNames = append(methNames, m.name)
>                 w.emitFeature(fmt.Sprintf("%s%**s", m.name, m.sig))
>         }
> +       if !complete {
> +               // The method set has unexported methods, so all the
> +               // implementations are provided by the same package,
> +               // so the method set can be extended. Instead of recording
> +               // the full set of names (below), record only that there
> were
> +               // unexported methods. (If the interface shrinks, we will
> notice
> +               // because a method signature emitted during the last loop,
> +               // will disappear.)
> +               w.emitFeature("unexported methods")
> +       }
>         pop()
>
> +       if !complete {
> +               return
> +       }
> +
>         sort.Strings(methNames)
>         if len(methNames) == 0 {
>                 w.emitFeature(fmt.Sprintf("**type %s interface {}", name))
> Index: src/cmd/api/testdata/src/pkg/**p1/golden.txt
> ==============================**==============================**=======
> --- a/src/cmd/api/testdata/src/**pkg/p1/golden.txt
> +++ b/src/cmd/api/testdata/src/**pkg/p1/golden.txt
> @@ -37,7 +37,7 @@
>  pkg p1, type Error interface { Error, Temporary }
>  pkg p1, type Error interface, Error() string
>  pkg p1, type Error interface, Temporary() bool
> -pkg p1, type I interface { Get, GetNamed, Name, PackageTwoMeth, Set }
> +pkg p1, type I interface, unexported methods
>  pkg p1, type I interface, Get(string) int64
>  pkg p1, type I interface, GetNamed(string) int64
>  pkg p1, type I interface, Name() string
> @@ -46,6 +46,11 @@
>  pkg p1, type MyInt int
>  pkg p1, type Namer interface { Name }
>  pkg p1, type Namer interface, Name() string
> +pkg p1, type Public interface { X, Y }
> +pkg p1, type Public interface, X()
> +pkg p1, type Public interface, Y()
> +pkg p1, type Private interface, unexported methods
> +pkg p1, type Private interface, X()
>  pkg p1, type S struct
>  pkg p1, type S struct, Public *int
>  pkg p1, type S struct, PublicTime time.Time
> Index: src/cmd/api/testdata/src/pkg/**p1/p1.go
> ==============================**==============================**=======
> --- a/src/cmd/api/testdata/src/**pkg/p1/p1.go
> +++ b/src/cmd/api/testdata/src/**pkg/p1/p1.go
> @@ -78,6 +78,16 @@
>         private()
>  }
>
> +type Public interface {
> +       X()
> +       Y()
> +}
> +
> +type Private interface {
> +       X()
> +       y()
> +}
> +
>  type Error interface {
>         error
>         Temporary() bool
>
>
>

[rsc, 2012-09-18 19:57:06]

*** Submitted as http://code.google.com/p/go/source/detail?r=248e11862ed5 ***

cmd/api: allow extension of interfaces with unexported methods

Fixes issue 4061.

R=golang-dev, bradfitz
CC=golang-dev
http://codereview.appspot.com/6525047