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: spec: allow explicit conversion from function to 1-method interface #47487

Open
Merovius opened this issue Jul 31, 2021 · 134 comments
Open

Comments

@Merovius
Copy link
Contributor

Merovius commented Jul 31, 2021

This is forked from #21670. I'd be fine having this closed as a duplicate, but some people suggested that it was different enough to warrant its own tracking issue. As the title suggests, I would like to be able to convert function values to single-method interfaces, if their respective signatures match.

Motivation

My motivation is the same as that proposal. I don't want to have to repeatedly create a type of the form

type FooerFunc func(A, B) (C, D)

func (f FooerFunc) Foo(a A, b B) (C, D) {
    return f(a, b)
}

to use a closure to implement an interface. #21670 mentions http.HandlerFunc, but for me the cases that comes up most often are io.Reader and io.Writer, which I want to wrap with some trivial behavior. For example, if I want an io.Writer, which counts the numbers of bytes written, I can write

type countingWriter struct {
    w io.Writer
    n int64
}

func (w *countingWriter) Write(p []byte) (n int, err error) {
    n, err = w.w.Write(p)
    w.n += int64(n)
    return n, err
}

func main() {
    cw := &countingWriter{w: os.Stdout}
    // write things to cw
    fmt.Println(cw.n, "bytes written")
}

However, this makes the code look more heavyweight than it is, by adding a separate type, when really, the only really relevant line is w.n += int64(n). It also looks like it breaks encapsulation. (*countingWriter).n is used both by methods of the type and to communicate the end-result.

Compare this to

func main() {
    var N int64
    cw := io.Writer(func(p []byte) (n int, err error) {
        n, err = os.Stdout.Write(p)
        N += int64(n)
        return n, err
    })
    // write things to cw
    fmt.Println(N, "bytes written")
}

Here, the usage of a closure removes the whole consideration of encapsulation. It's not just less code - it's also code that is more localized and makes the interactions clearer.

We can get part of that currently, by implementing a type like http.HandlerFunc. But especially if it's only used once, that smells of over-abstraction and still has the same boiler-plate.

Proposal

I propose to add a bullet to the list of allowed conversions of a non-constant value to a type, saying (roughly):

T is an interface type containing exactly one method and x's type is identical to the type of the method value of Ts single method. The resulting T will call x to implement its method.

One way to implement this, would be for the compiler to automatically generate a (virtual) defined type, with an arbitrary unexported name in the runtime package.

Discussion

  • The main difference to proposal: Go 2: Have functions auto-implement interfaces with only a single method of that same signature #21670 is that they propose assignability, while I propose convertibility. This is specifically to address a common concern about that proposal: io.Reader and io.Writer (for example) use the same signature for their method and it seems unsafe to have a function implement both, implicitly. This proposal addresses that concern, by requiring an explicit type-conversion - the programmer has to explicitly say if the function is intended to be an io.Reader or an io.Writer.
  • The proposal suggests to generate a virtual defined type, so that no changes to tooling, reflect (except to allow the conversion itself) or the use of type-assertions is needed.
    • This handles the same as if, say, the io package defined an unexported type readerFunc func(p []byte) (int, error) implementing io.Reader and a func ReaderFunc(f func(p []byte) (int, error)) Reader { return readerFunc(f) }.
    • We don't have the mismatch of having methods on non-defined types: It's not func([]byte) (int, error) that carries the method, but runtime.io_reader.
    • type-assertion/switches to unpack the func are not possible, as the type is unexported. That is a plus, because again, the func type can't carry methods so it would be strange to use it in a type-assertion.
    • However, it is already possible to transform from var r io.Reader to a func([]byte) (int, error), by simply evaluating r.Read.
  • The proposal suggest to put the generated type in the runtime package, because it seems like if two different packages do a func->interface conversion for the same interface, they should use the same type. I don't think the difference is observable though (the types are not comparable) so we could also put the type in the current package, similar to how closures turn up as pkgpath.func1 in stack-traces.
  • In var f func(); g := Fooer(f).Foo, g should not be nil, even though f is. Method expressions are never nil, currently, and we should maintain this invariant. This requires that the compiler generates wrapper-methods.

Further, more general discussion can be found in #21670 - some arguments applying to that proposal, also apply here. As I said, I'm fine with this being closed as a duplicate, if it's deemed to similar - but I wanted to explicitly file this to increase the chance of moving forward.

@gopherbot gopherbot added this to the Proposal milestone Jul 31, 2021
@ianlancetaylor ianlancetaylor changed the title Proposal: Allow conversions of function values to single-method interfaces of the same signature. proposal: Go 2: allow conversions of function values to single-method interfaces of the same signature Aug 1, 2021
@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Aug 1, 2021
@zephyrtronium
Copy link
Contributor

zephyrtronium commented Aug 1, 2021

The proposal suggests to generate a virtual defined type, so that no changes to tooling, reflect or the use of type-assertions is needed.

To be pedantic, this would require also changing package reflect, so that Value.Convert can convert function values to interface types when the language allows it. I don't remember whether this was mentioned in #21670.

@Merovius
Copy link
Contributor Author

Merovius commented Aug 1, 2021

@zephyrtronium You are correct, forgot to mention that, fixed.

@D1CED
Copy link

D1CED commented Aug 1, 2021

If we already take two steps in this direction why not take the third one with #25860? Is there a good reason to artificially limit this feature to single method interfaces (for e.g. simpler implementation)?

@beoran
Copy link

beoran commented Aug 1, 2021

When looking at the closure example above, I have to admit that in this case, this syntactic sugar would really help readability, because there is no need to go and read what the countingWriter is and does. All while the current level of type safety is maintained. So, I have to admit this makes me change my mind about this proposal.

However as you state :

This handles the same as if, say, the io package defined an unexported type readerFunc func(p []byte) (int, error) implementing io.Reader and a func ReaderFunc(f func(p []byte) (int, error)) Reader { return readerFunc(f) }

This points me to something I think is important. In stead of adding this conversion, There could be a convention in Go and iin the standard library where for every one-method reader, there should be an exported adapter function provided as you suggest. Of course, that would still have the performance cost of wrapping the function.

This brings me to why I still come to support this conversion proposal: it provides a possibility for improving performance by not having to wrap the closure in a struct and then unwrapping it again. In my experience closures are often used in this way, so it would help to avoid this wrapping.

About #25860: interface literals proposed there have a very serious problem with nil safety. One could easily fill in one of the multiple fields with a nil value, causing a panic when using the interface. AFAICS, this conversion here proposed, can be implemented in such a way that it does not have that problem. A conversion from a pointer-to-function to single method interface should be prohibited.

@Merovius
Copy link
Contributor Author

Merovius commented Aug 1, 2021

@D1CED Personally, I don't find the idea of interface-literals very readable. YMMV, of course. If #25860 is adopted, that would likely make this not worth it. If it's rejected, this might still prove valuable.

@Merovius
Copy link
Contributor Author

Merovius commented Aug 1, 2021

@beoran

interface literals proposed there have a very serious problem with nil safety. One could easily fill in one of the multiple fields with a nil value, causing a panic when using the interface. AFAICS, this conversion here proposed, can be implemented in such a way that it does not have that problem. A conversion from a pointer-to-function to single method interface should be prohibited.

I don't think the two proposals differ significantly in this regard. I would still allow converting a nil function to a single-value interface, which would then panic when called. We could make it so that using nil would result in a nil interface. IMO that's a bit of a strange special case though, for little benefit. Note that we are fine with code like this panicing, which is roughly equivalent:

type T struct{}

func (T) Foo() {}

type Fooer interface {
	Foo()
}

func main() {
	var f Fooer = (*T)(nil)
	f.Foo()
}

@beoran
Copy link

beoran commented Aug 1, 2021

Well, for this proposal, at least we could limit the allowed conversion to the case of a closure as you use in your example, or perhaps anything the compiler can statically see that it cannot be nil.

As for the example you show, I consider that unfortunate. I am all in favor of making Go more nil safe whenever possible. So I'd like either proposal to do better than what we have now.

@Merovius
Copy link
Contributor Author

Merovius commented Aug 1, 2021

Well, for this proposal, at least we could limit the allowed conversion to the case of a closure as you use in your example, or perhaps anything the compiler can statically see that it cannot be nil.

I am against doing that. We are doing a cost-benefit analysis and I'm only proposing this, because the cost is low. If we have to start distinguishing between different func values based on whether or not they are a function-literal and/or have to start describing heuristics based on which a compiler may prove that a value is nil, the complexity we add to the spec outweighs the benefit provided.

@beoran
Copy link

beoran commented Aug 1, 2021

Well, yes, perhaps the cost does outweigh the benefit in this case. If I want a static nil checker for go, for this case, I should probably contribute to staticcheck (https://staticcheck.io/docs/checks).

@Merovius
Copy link
Contributor Author

Merovius commented Aug 2, 2021

Based on the discussion in #25860 around nil methods, I've modified the proposal to answer the corresponding question here.

@earthboundkid
Copy link
Contributor

To be clear, I take it the behavior of g in var f func(); g := Fooer(f).Foo is the same as in this?:

type Fooer interface{ Foo() }
type FooFunc func()
func (f FooFunc) Foo() { f() }

func main() {
	var f func()
	g := FooFunc(f).Foo
	fmt.Println(f == nil, g == nil)
	g()
}
true false
panic: runtime error: invalid memory address or nil pointer dereference

@Merovius
Copy link
Contributor Author

Merovius commented Aug 4, 2021

@carlmjohnson That would be my proposal, yes.

@rsc
Copy link
Contributor

rsc commented Aug 18, 2021

The spec says this about type assertions

In this case, T must implement the (interface) type of x

We'd have to change that to "must implement or be convertible to"

@rsc rsc added this to Incoming in Proposals (old) Aug 18, 2021
@rsc rsc changed the title proposal: Go 2: allow conversions of function values to single-method interfaces of the same signature proposal: spec: allow explicit conversion from function to 1-method interface Aug 18, 2021
@rsc
Copy link
Contributor

rsc commented Aug 18, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Aug 18, 2021
@Merovius
Copy link
Contributor Author

@rsc

The spec says this about type assertions

In this case, T must implement the (interface) type of x

We'd have to change that to "must implement or be convertible to"

The way I intended, this isn't needed. The dynamic type of the converted interface wouldn't be func(…) (…), but an unexported (autogenerated) defined type. So you can't type-assert, because you can't refer to that type. Instead, to go from a 1-interface method, to a func, you'd use a method value of that method.

@rsc
Copy link
Contributor

rsc commented Aug 25, 2021

The way I intended, this isn't needed. The dynamic type of the converted interface wouldn't be func(…) (…), but an unexported (autogenerated) defined type. So you can't type-assert, because you can't refer to that type. Instead, to go from a 1-interface method, to a func, you'd use a method value of that method.

If you know what you want to go to, that's fine. But if you are asking "what kind of implementation of io.Reader is this?" you want to be able to answer that question when the answer is "this was an ordinary function". At least I think you do.

@Merovius
Copy link
Contributor Author

IMO there is a choice between a) don't make it possible to use type-assertions/-switches to "unpack" the interface, or b) violate the invariant that only defined types can have methods (possibly breaking code using reflect which assumes that this isn't possible) and add special cases in the spec for type-assertions/-switches on func, as you observe.

I agree that it's a downside not being able to find out what the dynamic type is. It just seems like the lesser evil. In particular as I don't think the usual use-cases for type-assertions/-switches really apply to this func case. ISTM that there are three general reasons you'd want to know the dynamic type:

  1. You need the concrete value to pass to somewhere accepting the concrete type. In this case you can use a method value.
  2. You want to specialize for performance or something. But calling the func shouldn't be any faster than calling the method, ISTM. So a default case calling the method (or using the method expression) should serve you just as well.
  3. You want to use the interface as a sum-ish type. In that case, you can always define your own type and use that as the case.

But if you think supporting it is worth the cost, I won't argue against it. I suggested what seemed the more palatable, less intrusive choice, but I'd take the convenience no matter how it comes :)

@Merovius
Copy link
Contributor Author

Oh and just to be safe: I assume it is a place-holder, but I don't think "must implement or be convertible to" is a good wording, either way. It would allow to type-assert something with underlying type float64 to int, which would be bad. If anything, we should specifically single out the func case.

@Merovius
Copy link
Contributor Author

@natefinch

from this: […] to this: […]

Note that the proposal is to still require a conversion. i.e. it's

mux.Handle("/ping", http.HandleFunc(someHandler))
// vs
mux.Handle("/ping", http.Handler(someHandler))

Which is less of a difference (which is why I don't think http.Handler is the best example - http.HandlerFunc already exists and does most of the heavy lifting).

@Merovius
Copy link
Contributor Author

Merovius commented May 4, 2022

@rsc friendly ping? I don't know who else can/would make this decision?

@ianlancetaylor
Copy link
Contributor

I'll take this off hold.

@ianlancetaylor ianlancetaylor moved this from Hold to Active in Proposals (old) May 4, 2022
@rsc
Copy link
Contributor

rsc commented May 18, 2022

@robpike, @ianlancetaylor, @griesemer, and I all think this is a good change worth trying.
We should experiment with it before deciding whether to accept the proposal.

Does anyone want to prototype it?

@DmitriyMV
Copy link

DmitriyMV commented May 18, 2022

type F func()
type (f F) M() { fmt.Println("F.M") }
type I interface { M() }
var FI F = func() { fmt.Println("FI") }
var V1 I = FI
var V2 = I(FI)
func main() {
    fmt.Printf("%T\n", V1) // prints I
    fmt.Printf("%T\n", V2) // prints I
    V1.M() // prints F.M
    V2.M() // prints F.M
}

After some additional tinkering I think current behavior is correct one, even if this proposal is accepted and implemented. Since F is named type with defined method M() there is no reason to upgrade function value to method M(). Mostly because it's already properly defined.

@rsc
Copy link
Contributor

rsc commented May 25, 2022

The ambiguity with existing valid programs definitely warrants more consideration.

Considering again #25860, it's true that there is not a lot of difference between:

io.Copy(w, io.Reader(myFunc))
io.Copy(w, io.Reader{Read: myFunc})

Maybe the more general form is better after all. We should clearly think more about this.

Either way, this is blocked on someone prototyping either or both of these.

@rsc
Copy link
Contributor

rsc commented Jun 1, 2022

Will put on hold for a prototype.

@rsc rsc moved this from Active to Hold in Proposals (old) Jun 1, 2022
@rsc
Copy link
Contributor

rsc commented Jun 1, 2022

Placed on hold.
— rsc for the proposal review group

@meyermarcel
Copy link
Contributor

I think the proposal is good if you take the writer's perspective, and I myself currently work in a language that allows the proposed construct. Would I like to write the longer 'heavyweight' example (second code excerpt from the description)? No, because a Closure would be significantly more practical to write.
Would I rather read the longer 'heavyweight' example than the shorter example with Closure? Yes, because everyone has written it that way so far and I can trust that it will always be written that way.
If this change were introduced, it would fragment the writing and change the readability because there is one more way to write the code.
I think the reason people come to Go is for readability. In almost all programming language designs, the emphasis is on the expressive power of the writer. Go is an exceptional exception (I must admit I don't know many programming languages).
Could it be better to accept less writing elegance to achieve better readability through consistency?
IMO readability of consistent code is one or maybe the biggest advantage of Go. Maybe we should look more at what disadvantage the writer's advantage brings to the reader. If you believe a well-known person from the IT scene, writing code and reading code are in a ratio of 1:10.

@adonovan
Copy link
Member

I know I'm late to the party, and I don't want to muddy things unnecessarily, but there is an alternative approach that may be more general than the "single-method interface" rule proposed here. @Sajmani many years ago proposed that it be possible to construct interface values by using the composite literal syntax, with one closure value per interface method. The running example would thus translate to:

func f(w io.Writer) {
    var count int64
    cw := io.Writer{
        Write: func (p []byte) (n int, err error) {
           n, err = w.Write(p)
           count += int64(n)
           return n, err
        },
    }
    // ...write things to cw...
    fmt.Println(count, "bytes written")
}

Syntactically, this scheme is not significantly more verbose, and the meaning couldn't be clearer. Compare:

 io.Writer(func (...) {...})
 io.Writer{Write: func (...) {...}}

More importantly, it seems to have two advantages over the current proposal:
(1) it avoids the ambiguity of what should happen if the func value to be converted to an interface is already of a type that implements the interface. (Does a method call on the resulting value call the original method, or the function itself, or is the ambiguity a static error?)
(2) it is not restricted to a single method. It's easy to implement (for example) sort.Interface by providing three closures. The compiler can easily fuse these closures into a single one (analogous to the explicit struct) for efficiency.

@leighmcculloch
Copy link
Contributor

leighmcculloch commented Oct 27, 2023

The idea to be able to construct interfaces from functions was also suggested on the other issue at #21670 (comment), which for a comment it received a significant number of upvotes.

@zephyrtronium
Copy link
Contributor

I believe you're looking for #25860.

@ainar-g
Copy link
Contributor

ainar-g commented Nov 3, 2023

@adonovan, that is very similar to what a lot of test mocks/fakes for interfaces do, and I strongly support that way of constructing interface values, if only to get rid of the mocks 😁.

@gopherbot
Copy link

Change https://go.dev/cl/572835 mentions this issue: all: prototype conversions from functions to single method interface

@cagedmantis
Copy link
Contributor

We've worked on a prototype for the originally suggested direction. Please feel free to use go.dev/cl/572835 to test out the prototype. The composite literal approach mentioned by @adonovan sounds like a promising idea which we may want to consider. I will be working on a prototype for that approach.

@ChrisHines
Copy link
Contributor

As @ainar-g pointed out, the composite literal approach would be quite handy for mocks/fakes. That would remove a good amount of boiler plate from certain classes of test code I've written or reviewed.

In my experience the interfaces involved in these tests often have more than one or even a couple of methods. I think it is typical for different test cases to exercise different subsets of the interface. For that purpose it should be possible to only provide functions for a subset of the interface methods in the composite literal, with the remaining methods causing a panic if called.

@adonovan
Copy link
Member

adonovan commented Mar 19, 2024

For that purpose it should be possible to only provide functions for a subset of the interface methods in the composite literal, with the remaining methods causing a panic if called.

While admittedly convenient, that seems hostile to code evolution. If I add a method to an interface, I want the compiler to point out each place where it can tell statically that a method is missing, not just turn it into a latent crash.

@ChrisHines
Copy link
Contributor

ChrisHines commented Mar 19, 2024

While admittedly convenient, that seems hostile to code evolution. If I add a method to an interface, I want the compiler to point out each place where it can tell statically that a method is missing, not just turn it into a latent crash.

That is a valid point and it may indeed be a reason not to do as I suggest. However, it seems to me adding methods to interfaces introduces other problems that act as a counterbalance to your concern.

If the interface is exported then adding a method is not backwards compatible, so you either have to find all the uses anyway or do a major version bump and then it isn't the same interface anymore. If it is not exported then finding all the uses should be easy.

@ChrisHines
Copy link
Contributor

Also, I think interface embedding already sets the precedent for panicking in a similar situation. Adding a method to an interface already risks the same problem if the interface is embedded in any struct that would then fail to provide an implementation of the new method.

https://go.dev/play/p/FoAENfAYkQZ

package main

import "io"

type reader struct {
	io.Reader
}

func main() {
	var rc reader
	io.Copy(io.Discard, &rc)
}

@griesemer
Copy link
Contributor

@adonovan @ChrisHines Let's move the discussion about whether to permit nil/absent methods in interface literals to #25860 which is about interface literals. Thanks.

@griesemer
Copy link
Contributor

When comparing this proposal and interface literals (#25860), consider also this suggestion which eliminates the (syntactic) size difference for single-method interfaces and which makes these two proposals equivalent in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Hold
Development

No branches or pull requests