Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: interface methods #39799

Open
carnott-snap opened this issue Jun 24, 2020 · 17 comments
Open

proposal: Go 2: interface methods #39799

carnott-snap opened this issue Jun 24, 2020 · 17 comments

Comments

@carnott-snap
Copy link

@carnott-snap carnott-snap commented Jun 24, 2020

background

Currently one cannot create a method set on an interface and instead must:

package sort

type Interface interface {
        // ... methods elided ...
}

func Reverse(i Interface) Interface {
        // ... implementation ...
}

But this leads to two problems; firstly call site readability, as can be seen in errors.Is:

errors.Is(Check(), ErrFailure)

// ... versus ...

Check().Is(ErrFailure)

Secondly, package namespace cluttering, as can be seen in context.WithXxx funcs, leads to messy call site usage:

package context

func WithCancel(parent Context) (ctx Context, cancel CancelFunc)
func WithDeadline(parent Context, d time.Time) (Context, CancelFunc)
func WithTimeout(parent Context, timeout time.Duration) (Context, CancelFunc)
import (
        "context"
        "time"
)

func current(ctx context.Context) {
        ctx, cancel := context.WithTimeout(ctx, time.Minute)
        defer cancel()
        // ...
}

// ... versus ...

func new(ctx context.Context) {
        ctx, cancel := ctx.Timeout(time.Minute)
        defer cancel()
        // ...
}

proposal

Allow for method sets to be defined against interface types:

package sort

func (i Interface) Reverse() Interface {
        // ... implementation ...
}

concerns

As with all language changes, this has implications. First, interfaces can now implicitly implement other interfaces:

type Any interface{}
func (Any) Do() {}
type Doer interface { Do() }
var _ Doer = Any(nil)

This to me seems like a feature, but could have unintended consequences.

Second, there are some odd side effects wrt type assertions:

type Unit struct{}

func main() {
        u := Unit{}
        u.(Any).(Doer).Do() // valid
     // u.(Doer).Do()       // invalid
}

This is derived from the fact that the interface, not the concrete type owns those methods. However, this too could be a desirable feature, like how custom types work:

func (Unit) Do() type Unit struct{}

func main() {
        s := struct{}{}
     // s.Do()       // invalid
        Unit(s).Do() // valid
}

considerations

Should should pointer methods be allowed:

func (i *Interface) Reverse() {
        // like the previous, but this one uses side effects
        // ... implementation ...
}

My gut says, yes this is an important use case, but I do not have data. Furthermore, it would make the method set definition consistent: any type can have a pointer or value method defined.

alternatives

Some of the need for this disappears with generics, but only if method sets are allowed to be generic, which they are currently not.

@gopherbot gopherbot added this to the Proposal milestone Jun 24, 2020
@gopherbot gopherbot added the Proposal label Jun 24, 2020
@ianlancetaylor ianlancetaylor changed the title proposal: interface methods proposal: Go 2: interface methods Jun 24, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 24, 2020

Looks similar to #23185.

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 24, 2020

From #23185:

func (i Interface) Inc() {
  i.Add(1)
}

While we could disallowing this, from an call site perspective, we would still retain access to all the methods, and it would feel like a hybrid of custom types, read type One Two and anonymous fields. The interface method set is default, with fallback to wrapping method set, and access the hidden methods would require type assertions:

var i Interface = Num(5)
i.Inc()       // Interface.Inc
i.(Num).Inc() // Num.Inc

That being said, an implementation of this proposal could suffer from the same type stack mentioned in #23185, and I agree seems problematic without adding meaningful value. It is poorly described above in the concerns section of the description, and I will likely take another stab at clarifying things, but I would be amenable to removing the feature. This would add the restriction that interface method sets cannot implement interfaces. This simplifies the implementation, and mental gymnastics around constructing a type stack, but has the caveat that only the base type's methods, something that could be confusing or obscured.

That being said, a type stack is not explicitly required to make this work. One could calculate, at compile time, the equivalency set of paths between a type and interface. If cached, this may not have too large an impact on compile time, but would need to be done per symbol, since private interfaces only affect package local symbols. There is also a question of what packages should be interrogated. Restricting it to things imported by the file would lead to _ imports, but using all the packages in the compiler would scale poorly and lead to heterogeneous behaviour when new things get imported. Plus all of this would give goimports/gopls a headache. On top of this it could also lead to tricky bugs where you did not realise you actually did implement a given interface, but some package built the adaptor, so it compiled.

My intentions are to allow for flow style apis and reduce package clutter, so neither this restriction, nor suggesting that method sets would be better, are really at issue with the spirit of this proposal.

@martisch
Copy link
Member

@martisch martisch commented Jun 24, 2020

That being said, a type stack is not explicitly required to make this work. One could calculate, at compile time, the equivalency set of paths between a type and interface.

Is that sufficient when plugins and/or reflect can make new types and interfaces the compiler does not know about appear at runtime?

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 24, 2020

My assumption was that a statically compiled language could not do such trickery. I would be very curious to see samples for these features, because it sounds like you are implying Go has a dynamic type system.

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 24, 2020

Gut reaction, if we already do this, it means we keep around all the compile time data for interface implementation, or at least if reflect is imported. Thus, we would need to keep around all the same equivalency set data, probably costly depending on the implementation?

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 24, 2020

What would such a method mean if the dynamic type of the interface value also implements a method with the same signature?

What would happen if a value that does not implement the method is assigned to some other interface type that does not provide an implementation? Would its dynamic (concrete) type change by virtue of being assigned from one interface to another? (Interface values do not change in such a way today.)

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 24, 2020

What would such a method mean if the dynamic type of the interface value also implements a method with the same signature?

To confirm this is the situation you are referring to:

type Interface interface {
  Add(x int)
  Inc()
}

func (i Interface) Inc() {
  i.Add(1)
}

The simplest option is to simply reject this at compile time, not sure if this is best, but we would likely want to reject all method name collisions.

Otherwise, I would have the the Interface.Inc method shadow underlying.Inc, since you can still get access to the latter:

type Num int
func (n *Num) Inc() { *n += 1 }
func (n *Num) Add(x int) { *n += x }

func main() {
  var i Interface = Num(5)
  i.(Num).Inc()                 // assuming Num is exported
  i.(interface{ Inc() }).Inc() // otherwise an anonymous type cannot contain a method set
}

What would happen if a value that does not implement the method is assigned to some other interface type that does not provide an implementation? Would its dynamic (concrete) type change by virtue of being assigned from one interface to another? (Interface values do not change in such a way today.)

In short, I do not (think I) care about the path taken here. If it is beneficial, we could implement a full type stack such that u.(Any).(Doer).Do(), see above, is valid, but u.(Doer).Do() is not. However, I see that this has non-trivial implications.

If this feature is too complex to implement or reason about we can disallow it. Thus dynamic (concrete) type would not change, and u.(Any).(Doer).Do() is invalid unless Unit implements interface{ Do() } itself.

For my concerns, this nuance is not important, I am happy with either solution or something else, say generic methods: see above.

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 25, 2020

If the implementations of the interface can't implement the method, and instances of the interface don't retain the method, then it seems like this proposal is just syntactic sugar for calling global functions as if they were methods.

I've used languages that aggressively push global names over to method syntax (particularly Rust), and I don't like it: it's less verbose, but as a reader the “uniform” method syntax makes it much harder for me to figure out which calls are actually defined on the type vs. which ones are free functions defined elsewhere.

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 25, 2020

If the implementations of the interface can't implement the method, and instances of the interface don't retain the method, then it seems like this proposal is just syntactic sugar for calling global functions as if they were methods.

Correct, the focus of my interest in on the syntax sugar. Since method sets do other things, it must be resolved if/how those features are treated too, but your intuition is correct.

I've used languages that aggressively push global names over to method syntax (particularly Rust), and I don't like it:

I am a little unclear what you are trying to say here. It sounds like you agree that method syntax sugar is better than global funcs, is this correct?

ctx, cancel := context.Background().Timeout(time.Minute)
if err.Is(io.EOF) { /* ... */ }

// are prefered to the current

ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
if errors.Is(err, io.EOF) { /* ... */ }

[I]t's less verbose, but as a reader the “uniform” method syntax makes it much harder for me to figure out which calls are actually defined on the type vs. which ones are free functions defined elsewhere.

Again, not sure where your confusion stems from. I agree finding a reference in Rust is harder, but that may have to do with default trait implementations, operator overloads, and the general increase in complexity of their type system; examples would be welcome. In my previous example it should be trivial to find a given definition:

// context.Background is a package func
// it returns a context.Context, thus we can look for context.Context.Timeout
// it returns context.Context and context.CancelFunc
ctx, cancel := context.Background().Timeout(time.Minute)

// type builtin.error has a defined Is method
// it returns a bool
if err.Is(io.EOF) { /* ... */ }
@bcmills
Copy link
Member

@bcmills bcmills commented Jun 25, 2020

It sounds like you agree that method syntax sugar is better than global funcs, is this correct?

No, I strenuously disagree.

When I see context.WithTimeout(ctx, time.Minute), then I know that there is only one possible definition of WithTimeout and I can find it in the context package. (I can run 'go doc context.WithTimeout` — I don't need a fancy IDE hook to figure it out for me.)

When I see ctx.Timeout(time.Minute), now I have to do some analysis: what is the type of ctx, and is the method defined on that type, or on the underlying type?

Compare https://doc.rust-lang.org/rust-by-example/trait.html:

    let mut dolly: Sheep = …;

    dolly.talk();
    dolly.shear();
    dolly.talk();

When I see dolly.shear(), do I look for a definition (and documentation) on the Sheep type, or on the Animal trait? what about dolly.talk()?

In contrast, with the equivalent explicit calls (which I believe are also valid in Rust):

    let mut dolly: Sheep = …;

    Animal::talk(dolly);
    Sheep::shear(dolly);
    Animal::talk(dolly);

Now I know exactly where to look for the documentation of each of those functions.

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 25, 2020

No, I strenuously disagree.

Apologies for misreading you.

When I see context.WithTimeout(ctx, time.Minute), then I know that there is only one possible definition of WithTimeout and I can find it in the context package. (I can run 'go doc context.WithTimeout` — I don't need a fancy IDE hook to figure it out for me.)

When I see ctx.Timeout(time.Minute), now I have to do some analysis: what is the type of ctx, and is the method defined on that type, or on the underlying type?

While I follow your argument, I exclusively develop in vim without hooks (both Rust and Go), and have yet to have an issue with this. Maybe I have just wrapped my head around the locations that these things can live and tend to guess correct most times? Regardless, this is an extant issue with method sets and flow style apis; consider the following:

x := http.NewRequest(http.MethodGet, "https://example.com", nil).WithContext(context.Background())

What is the type of x? You would do the same lookup:

  • look at the return type of http.NewRequest: http.Request
  • look in the method set of http.Request for WithContext: http.Request.WithContext
  • look at the return type of http.Request.WithContext: http.Request

This is the same if the http.Request is a value, pointer, or interface.

If we allow for method set collisions, which I am not advocating for, it would still be obvious (since I assume the interface method set beats the interface method); go doc could even help with that:

type Interface interface { Add(x int); Inc() }
func (i Interface) Inc() { i.Add(1) }

type Num int
func (n *Num) Inc() { *n += 1 }
func (n *Num) Add(x int) { *n += x }

func New() Interface { return Num(5) }

func main() { New().Inc() }
$ go doc Interface.Inc
func (i Interface) Inc()

Otherwise if there is no method set, you would get the interface method:

$ go doc Interface.Inc
func Inc()

This also ignores the case of name collisions that are not type compatible, but with all this complexity, I think that disallowing this at compile time is fine. It does leave the "look in two places" issue, but this is already present for anonymous struct fields.


When I see dolly.shear(), do I look for a definition (and documentation) on the Sheep type, or on the Animal trait? what about dolly.talk()?

Again, I think this speaks more to the complexities of the rust type system, not flow style apis. It is possible to have only one valid place for a symbol to be defined.

In contrast, with the equivalent explicit method calls (which I believe are also valid in Rust):

    let mut dolly: Sheep = …;

    Animal::talk(dolly);
    Sheep::shear(dolly);
    Animal::talk(dolly);

Now I know exactly where to look for the documentation of each of those functions.

Similarly, you can do this, but having the code read left to right makes it more legible to me:

x := http.Request.WithContext(http.NewRequest(http.MethodGet, "https://example.com", nil), context.Background())

Are you also implicitly arguing that method sets should require explicit, qualified references?

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 25, 2020

Are you also implicitly arguing that method sets should require explicit, qualified references?

Yes..? But that's probably not feasible for Go at this point.

(I would like methods with standardized semantics to be qualified with the package, or perhaps the type, in which they are defined, so that instead of implementing func (r MyReceiver) Read([]byte) (int, error), you would implement func (r MyReceiver) io.Read([]byte) (int, error).)

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 25, 2020

Regardless, this is an extant issue with method sets and flow style apis

Oh, absolutely. I don't like flow-style APIs either. 🙂
(I much prefer the “New function with an Options struct” pattern.)

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jun 26, 2020

(I would like methods with standardized semantics to be qualified with the package, or perhaps the type, in which they are defined, so that instead of implementing func (r MyReceiver) Read([]byte) (int, error), you would implement func (r MyReceiver) io.Read([]byte) (int, error).)

This is well into "new issue" territory, but how would you call func (r MyReceiver) io.Read([]byte) (int, error)?

io.Read(MyReceiver{}, nil)
xxx.MyReceiver.io.Read(MyReceiver{}, nil)

Oh, absolutely. I don't like flow-style APIs either. slightly_smiling_face
(I much prefer the “New function with an Options struct” pattern.)

While I have come to enjoy the ...Options interface pattern, I think there are places where flow apis have real readability benefits.

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 29, 2020

This is well into "new issue" territory, but how would you call func (r MyReceiver) io.Read([]byte) (int, error)?

Not sure — I haven't worked through the details yet, I just have a high-level sketch.

Probably:

n, err := io.Read(r, p)

but perhaps:

n, err := r.io.Read(p)
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 21, 2020

Per discussion above, this particular proposal is a likely decline. Leaving open for four weeks for final comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.