spec: Interface Types defines embedding as method enumeration, but private methods are treated differently

[gopherbot]

by Zteve.Powell:

Section http://golang.org/ref/spec#Interface_types in golang spec says:

    An interface may use an interface type name T in place of a method specification. 
    The effect, called embedding an interface, is equivalent to enumerating the methods of T explicitly in the interface.

and no mention is made of private methods (beginning with a lowercase letter). These are
*not* treated as equivalent to being enumerated explicitly by the compiler.  The
compiler prevents implementation of these methods outside of the package the embedded
interface is defined in.

Demonstrate issue
----------------
1. Define an interface Base in pkga with an initial-lower-case method (lower), and an
initial-upper-case method (Upper).
2. Define an interface Outer in pkgb which embeds pkga.Base.
3. Define methods (receiver type OB struct{}, say) with the names and signatures of
lower and Upper in pkgb.
4. In a func, declare (var x OB;) and (var y Outer;) and assign x to y (x = y;)
5. go build

What happened?
-------------
Error from compiler along the lines of:
# iftest/pkgb
./pkgb.go:17: cannot use x (type OB) as type Outer in assignment:
    OB does not implement Outer (missing pkga.lower method)
        have lower()
        want pkga.lower()

What should have happened instead?
-------------------------------
According to the *spec* we should have compiled cleanly.

Additional information
-------------------
See
http://www.google.com/url?q=http%3A%2F%2Fwww.onebigfluke.com%2F2014%2F04%2Fgos-power-is-in-emergent-behavior.html&;sa=D&sntz=1&usg=AFQjCNGPJvFRYNQdzvUgsJ6I8YkmLEJ4Rg

and https://groups.google.com/forum/#!topic/golang-nuts/9AsBWYd6AY0

for the provenance of this issue.

[ianlancetaylor]

Comment 1:

Labels changed: added repo-main.

Owner changed to @griesemer.

Status changed to Accepted.

[griesemer]

Comment 2:

Fair enough. The wording in the spec is misleading.

[rsc]

Comment 3:

Labels changed: added release-go1.4.

[gopherbot]

Comment 4 by Zteve.Powell:

It could be argued that the compiler is wrong; or that both are wrong.  IMO the
spec/compiler should either: disallow lower-case (private) methods in interfaces
(preventing the exploitation of this accidental behaviour), or else introduce a defined
semantics for such methods, which allows implementation of private methods (in the Outer
interface package). I do not think the present behaviour is desirable, or very useful.

[griesemer]

Comment 5:

While the spec may be unclear and/or misleading in this case, the implemented semantics
is well established and understood. Any change affecting the implemented semantics is
tantamount to a language change which might break existing code (these are not corner
cases, it is a feature and people rely on the fact that they can define interfaces that
cannot be implemented external to a package).
But I agree that the spec should be clearer.

[gopherbot]

Comment 6 by glyn.normington:

I'm not sure it is fair to call this particular behaviour of the code a feature. Can we
at least agree to admit that it's a bit of a hack made possible only by the unplanned
implications of earlier decisions? In fact the feature might do more harm than good: in
what circumstances should the provider of an interface be able to prevent the user from
using mocked or stubbed implementations in their testing?

[gopherbot]

Comment 7 by Zteve.Powell:

| the implemented semantics is well established and understood.
I'm not sure how it would be understood. It isn't in the spec, nor in the Go Language
book 'Programming in Go' (2012), so perhaps people didn't stumble upon this before then.
It can only be understood by experimentation; followed by dreaming up a possible
application.  Which, by the way, doesn't sound too useful to me.  If it were so useful,
it should have a clear marker in the source code, and therefore in the language. As it
stands you have to be aware of this glitch, recognise it in the source and then reason
as to why it is there.  A 'private' keyword or some way of indicating restricted
implementation types would be better.
| Any change affecting the implemented semantics is tantamount to a language change
which might break
| existing code
Yes. But then, how would this ever be false?  There can be no code that relies upon
behaviour that differs from the 'implemented semantics', can there? I think this is a
general argument that is essentially:
"It does what it does, and regardless of the language specification or design, we can
never change what it does."
This is of course not the case. Otherwise bugs would not be fixed! (Who says they are
bugs, by the way--the spec?  Nah!)
I think we should at least debate this, rather than dismiss it as 'working as
implemented'.

[ianlancetaylor]

Comment 8:

The feature of using a non-exported method in an interface, which ensures that all
implementations of the interface are within the same package, is by design, is
well-understood, is used by standard Go packages.  It is not an accident.

[gopherbot]

Comment 9 by Zteve.Powell:

| The feature of using a non-exported method in an interface, which ensures that all
implementations of the
| interface are within the same package, is by design, is well-understood, is used by
standard Go packages.
| It is not an accident.
I thought this was something that surprised Robert.
| (Robert Griesemer is an employee of Google and one of the initial designers of the Go
programming
| language.) - Wikipedia
Presumably this was a bit of the design that got past him. Possibly he only read the
spec.
I'm sure it is well understood, by those close to the language implementation, or
implementing standard Go packages.
I can see it might be useful for built-in or critical system/language packages, but for
normal development, it impedes testability.  I cannot stub the use of an interface in
another component/package if it has a restricted implementation. Which then makes it
hard for me to see how a tool like mockgen can cope with it -- unless it finds a way
round it. (One which is well understood, designed in, and undocumented, no doubt.)

[ianlancetaylor]

Comment 10:

> I thought this was something that surprised Robert.
Robert can speak for himself but I believe you have misread his comment.
> I can see it might be useful for built-in or critical system/language packages, but
for normal development, it impedes testability.
Tests are normally in the same package and can therefore use the methods.
In any case, if you find that private methods are a hindrance, don't use them.

[gopherbot]

Comment 11 by Zteve.Powell:

In
http://www.google.com/url?q=http%3A%2F%2Fwww.onebigfluke.com%2F2014%2F04%2Fgos-power-is-in-emergent-behavior.html&sa=D&sntz=1&usg=AFQjCNGPJvFRYNQdzvUgsJ6I8YkmLEJ4Rg
Brett Slatkin suggests that Robert was surprised.
| Tests are normally in the same package and can therefore use the methods.
I think you misunderstand me: if a user of the package wants to be tested, it cannot
stub an interface which has a private method.  The tests impeded are for code that
*uses* the constrained interface, not the implementation of the constrained interface.
This also implies that the advice 'not to use it' is unhelpful, too.  If others have
used it already I cannot easily test my code that calls them without invoking them. And
I cannot modify their packages. I am forced to wrap the interface with another and
delegate, or some such unnatural act. Hence my reference to mockgen.

[griesemer]

Comment 12:

Just to make myself absolutely clear:
1) This is a well-understood language feature and not an accident, as iant already
emphasized again.
1) No, I was not surprised by it; I only said that the spec should  be clearer. There
are a lot of language mechanisms where we (the designers and implementers) have a very
clear notion and understanding, but where we didn't do a good job of writing it down
clearly. This is why we are still refining the spec. Except for pathological or academic
corner cases which are irrelevant in practice but matter for an implementation, we do
not make changes to the language. This is _not_ a pathological case, and we are not
going to change it. I will try to make the spec clearer, though.
2) In case there's any doubt: While I didn't write the compiler, I did write go/types
which requires complete understanding of the subtle issues of interfaces, exports and
non-exported names, specifically with respect to struct fields and method names. So, I
can assure you that I am very familiar with the issues. In fact, go/types was written
with the understanding of the spec in mind and exposed several (corner-case) bugs in the
compilers.
Please let's stop this discussion. Your point (spec is unclear) is taken, and I will fix
it. Thanks.

[gopherbot]

Comment 13:

CL https://golang.org/cl/149010043 mentions this issue.

[griesemer]

Comment 14:

This issue was closed by revision bb29c5a1ed872b770ff5203d8a0109a49e6d1db.

Status changed to Fixed.

[gopherbot]

Comment 15 by Zteve.Powell:

AFAICT the revision to the spec does not clarify the issue or fix this bug in the spec.
Non-exported methods of embedded interfaces are *not* treated the same as exported
methods: the compiler refuses to allow you to implement them for the outer interface: it
associates the original interface package with the non-exported method, which means you
cannot implement the outer interface.
At the very least the spec should say this, but I still consider this to be a language
bug (or anomaly, if you wish).

[gopherbot]

Comment 16 by Zteve.Powell:

AFAICT the revision to the spec does not clarify the issue or fix this "bug" in the spec.
Non-exported methods of embedded interfaces are *not* treated the same as exported
methods: although I can implement the outer (embedding) interface, the resulting
implementation is not type-compatible with the inner interface. It would be, if the
Exported and non-Exported methods were treated the same way.
I think the spec should say something about type compatibility in this case, but no
mention is made of this.
I still consider this to be a language bug (or anomaly, if you wish) because it makes
stubbing or mocking interfaces that exploit the restriction impossible: one cannot
create a type-compatible implementation of an interface with a non-exported method
outside of the implementing package.

[ianlancetaylor]

Comment 17:

From http://golang.org/ref/spec#Uniqueness_of_identifiers , two non-exported identifiers
in different packages are considered to be different even if they are spelled the same.
> one cannot create a type-compatible implementation of an interface with
> a non-exported method outside of the implementing package.
Yes.  That is how the language is supposed to behave.

[gopherbot]

Comment 18 by Zteve.Powell:

> From http://golang.org/ref/spec#Uniqueness_of_identifiers , two non-exported
identifiers in different
> packages are considered to be different even if they are spelled the same.
Yes, although I wonder what is meant by 'identifier' after reading this. [Aren't every
pair of identifiers from distinct packages considered to be different? How would they
not be?]
> That is how the language is supposed to behave.
OK. I think it is silly (because of a mocking requirement and what possible benefit is
it?), but OK: it is supposed to behave like that. I can accept that.
This issue was accepted as requiring a 'clarification' in the spec for this behaviour,
because the spec doesn't say anything about it.
The revision that closed this issue makes no reference at all to this behaviour.
I think the spec should say how the language is *supposed to behave*. Don't you?

[griesemer]

Comment 19:

When it comes to equality of identifiers, the section Ian mentioned (uniqueness of
identifiers) _defines_ when two identifiers are the same or not. And that's what it
comes down to for matching interfaces with implementations (and other interfaces).
The spec has been corrected to not use the word "enumerate" to explain embedding
anymore, because that was clearly misleading as you have pointed out. It now uses the
word "add" (of methods) which is exactly what is meant, and how embedding of interfaces
is implemented: The methods of embedded interfaces are simply added to the embedding
interface (unless there's a conflict in which case an error is reported - and that's
covered by the requirement that interface methods must be unqiue).
The spec also mentions now that all _exported_and_imported_ methods of an embedded
interface are added, exactly to make this behavior of embedding extra clear.
The fact that one cannot necessarily implement such an interface is a consequence of the
uniqueness requirement, and it's an intended and welcome side effect (and Ian has
pointed that out already as well). It permits the definition of interfaces that can only
implemented by using objects of the package that defines that interface. 
I'm considering this issue closed at this point.