Skip to content

proposal: spec: allow explicit conversion from function to 1-method interface #47487

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

Open
Merovius opened this issue Jul 31, 2021 · 146 comments
Open
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal
Milestone

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 An incompatible library change LanguageChange Suggested changes to the Go language 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 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

@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.

@fzipp
Copy link
Contributor

fzipp commented Aug 25, 2021

What would reflect.TypeOf(cw).String() return?

@Merovius
Copy link
Contributor Author

@fzipp If the underlying type is an autogenerated defined type, that autogenerated name. e.g. runtime.io_reader, using some mangling scheme. Or runtime.autogenerated_58a99a38 or something, if we can't come up with a mangling scheme.

Of course, if the underlying type ends up the func type, it would print func([]byte) (int, error).

@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
Contributor

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.

@andig
Copy link
Contributor

andig commented Feb 13, 2025

This proposal and especially #47487 (comment) are unbelievably helpful. Some time ago I wrote about this in https://groups.google.com/g/golang-nuts/c/Oo9Nwm4AZRI/m/uQ6xFWgNCAAJ.

Would it make sense to move this into the regular proposal review meeting?

@ianlancetaylor ianlancetaylor removed the LanguageChangeReview Discussed by language change review committee label Mar 5, 2025
@ianlancetaylor
Copy link
Contributor

There is an implementation of this proposal in https://go.dev/cl/572835. Taking off hold.

@griesemer
Copy link
Contributor

It would be good to see concrete examples where this feature is clearly better/more readable etc. than the corresponding manual creation of a struct with associate method.

@andig
Copy link
Contributor

andig commented May 7, 2025

@griesemer see linked google discussions code above which has generated code of this form: googlegroups.com. These are all one-method interface that each take an embedding structure to be generated. Only purpose of that struct is to hold the single method that actually makes up that interface. See https://github.com/evcc-io/evcc/blob/master/meter/e3dc_decorators.go for example output generated. I've found that the generated code, used in various places, adds about 10% to binary size. I'd hope that getting rid of the embedding struct might contribute to smaller binary side, too.

@mvdan
Copy link
Member

mvdan commented May 7, 2025

A recent example of mine:

type oneByteReader struct {
	r io.Reader
}

func (r *oneByteReader) Read(p []byte) (int, error) {
	if len(p) == 0 {
		return 0, nil
	}
	return r.r.Read(p[:1])
}

func f(...) {
	scanner := bufio.NewScanner(&oneByteReader{ctx.Stdin})

I would like to rewrite this as the following, which is shorter, and lets me keep all the logic inline too:

func f(...) {
	scanner := bufio.NewScanner(io.Reader(func(p []byte) (int, error) {
		if len(p) == 0 {
			return 0, nil
		}
		return ctx.Stdin.Read(p[:1])
	}))

Another recent example:

type errWriter Command

func (w *errWriter) Write(b []byte) (int, error) {
	c := (*Command)(w)
	c.hasErr = true
	return c.Command.OutOrStderr().Write(b)
}

func (c *Command) Stderr() io.Writer {
	return (*errWriter)(c)
}

could be rewritten as simply:

func (c *Command) Stderr() io.Writer {
	return io.Writer(func(b []byte) (int, error) {
		c.hasErr = true
		return c.Command.OutOrStderr().Write(b)
	})
}

@mvdan
Copy link
Member

mvdan commented May 7, 2025

I'll throw in a third example which is a thin http.RoundTripper wrapper to add User-Agent headers:

func f(...) {
	return &userAgentTransport{
		next:      next,
		userAgent: UserAgent(clientType),
	}
}

type userAgentTransport struct {
	next      http.RoundTripper
	userAgent string
}

func (t *userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	req1 := *req
	req1.Header = maps.Clone(req.Header)
	req1.Header.Set("User-Agent", t.userAgent)
	return t.next.RoundTrip(&req1)
}

could be rewritten as:

func f(...) {
	return http.RoundTripper(func(req *http.Request) (*http.Response, error) {
		req1 := *req
		req1.Header = maps.Clone(req.Header)
		req1.Header.Set("User-Agent", UserAgent(clientType))
		return next.RoundTrip(&req1)
	}))
}

@griesemer griesemer added the LanguageChangeReview Discussed by language change review committee label May 7, 2025
@hherman1
Copy link

hherman1 commented May 8, 2025

It also seems really nice for testing. I’m going to be much more inclined to write fakes when I can easily create them. So there’s a category of code that I think is simply not being written in the first place because it’s just a bit too annoying to write.

@Merovius
Copy link
Contributor Author

Merovius commented May 8, 2025

Going through the stdlib, with no claim to completeness:

  • There are 8 function types to implement io.Reader/io.Writer/io.Closer, all in tests (1, 2, 3, 4, 5, 6, 7, 8). Interesting in particular is that net/http has two identical types with different names (funcWriter and writerFunc).
  • There are 4 function types to implement http.RoundTripper, all in tests and in two pairs of duplicates: net/http has roundTripFunc/testRoundTripper and net/http/httputil has RoundTripperFunc/roundTripperFunc.
  • There are 2 unexported function types to implement types.Importer: 1, 2. Neither of those are in tests.
  • There is 1 unexported function type to implement cryptobyte.MarshalingValue, not in a test.
  • There are 3 exported function types to implement exported interfaces, presumably because this was considered a significance convenience for callers: net/http.HandlerFunc/Handler, cmd/go/internal/work.ActorFunc/Actor and golang.org/x/mod/sumdb/tlog.HashReaderFunc/HashReader.

All of these usages would be trivially obsoleted by this proposal.

There is also a few cases where type someFunc func() implements an interface by calling the receiver for its side-effects in a test and then returns an error: net/http.funcRoundTripper (not to be confused with roundTripFunc or testRoundTripper in the same package, which are mentioned above) and net/http.eofReaderFunc. While these are not directly mirroring this proposal, they would presumably not exist under this proposal either, because adding a return 0, io.EOF to a function literal isn't super onerous. But there is an argument here.

There is also another interesting usage, which definitely isn't solved by this proposal: testing.matchStringOnly is a function type that implements testing.testDeps, an interface with lots of methods, by delegating one of the methods (MatchString) to its receiver, while returning zero values or static errors from all the other ones.

There is also golang.org/x/text/transform.removeF, which implements a two-method interface (similarly to matchStringOnly), but the body of the one calling the receiver has a lot of extra logic. Similarly for go/ast.inspector. These also couldn't use this proposal.

I believe these are all the cases I found (again, no promise) that used function types with methods in the stdlib. I think the first list (in particular the duplication) shows some demand.

Notably, this only takes into account cases where the underlying type is already a function type, so we can conclude with certainty that these cases only exist because a lack of this proposal. Presumably, there are other cases where this could be useful, but they are harder to enumerate. For example io.byteAndEOFReader is an implementation of io.Reader that could conveniently expressed as a function-literal. Though this particular type is used multiple times, so benefits from having a dedicated type, I think it's a good example of the kind of thing this proposal would make more convenient.

[edit] I do think this significantly undercounts the use cases of this proposal, because by nature it only includes such cases, where the authors already felt that the convenience of using closures outweighs the added boilerplate of defining a type. If my go source analysis foo was stronger, I'd look for any types that 1. have exactly one method, 2. are only created once and 3. are then assigned to a 1-method interface. All such types could use this proposal. Of course, they would then have to be filtered for whether or not to use a closure, from a readability perspective.

@findleyr
Copy link
Member

findleyr commented May 13, 2025

I think the wording of the proposal here needs to address the problem of unexported names.

For example, suppose a library uses the closed form of the variadic options pattern:

type Option interface {
  set(s *Server)
}

func NewServer(opts ...Option) *Server

In which the library provides a finite set of Options to configure the Server.
As worded, it sounds like this proposal would allow the conversion lib.Option(func(s *lib.Server) { ... }), letting users define their own options, subverting the intent of the library author.

Therefore, I think the proposal should be that such conversions are only allowed if the method name in question is accessible from the site of the conversion (similar to the rule for unkeyed composite literals).

@urandom
Copy link

urandom commented May 14, 2025

As worded, it sounds like this proposal would allow the conversion lib.Option(func(s *lib.Server) { ... }), letting users define their own options, subverting the intent of the library author.

I don't think this has any real relevance. Your example supposedly allows the "Option"s to modify something in the server. Either these modifications happen on exported fields in the Server struct, which means that you don't really need options, or they modify unexported fields, which means that there is no way for someone else to provide different set of options.

@findleyr
Copy link
Member

@urandom yes, I was just trying to give an example of a one-method interface that is intentionally unexported to control the library's API surface. The example is a little contrived, but the point remains: I think the proposal should disallow implementing methods that aren't accessible from the site of the conversion. Are you disagreeing with that conclusion?

@urandom
Copy link

urandom commented May 15, 2025

@findleyr

I do not disagree. Though I'd like to see some real-world examples that clearly illustrate the issue, since the one that's given doesn't really work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal
Projects
Status: Incoming
Development

No branches or pull requests