-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
Comments
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. |
@zephyrtronium You are correct, forgot to mention that, fixed. |
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)? |
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 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. |
I don't think the two proposals differ significantly in this regard. I would still allow converting a type T struct{}
func (T) Foo() {}
type Fooer interface {
Foo()
}
func main() {
var f Fooer = (*T)(nil)
f.Foo()
} |
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. |
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 |
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). |
Based on the discussion in #25860 around |
To be clear, I take it the behavior of 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()
}
|
@carlmjohnson That would be my proposal, yes. |
The spec says this about type assertions
We'd have to change that to "must implement or be convertible to" |
This proposal has been added to the active column of the proposals project |
The way I intended, this isn't needed. The dynamic type of the converted interface wouldn't be |
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. |
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 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
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 :) |
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 |
I'll take this off hold. |
@robpike, @ianlancetaylor, @griesemer, and I all think this is a good change worth trying. Does anyone want to prototype it? |
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 |
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:
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. |
Will put on hold for a prototype. |
Placed on hold. |
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. |
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:
Syntactically, this scheme is not significantly more verbose, and the meaning couldn't be clearer. Compare:
More importantly, it seems to have two advantages over the current proposal: |
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. |
I believe you're looking for #25860. |
@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 😁. |
Change https://go.dev/cl/572835 mentions this issue: |
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. |
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. |
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. |
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
|
@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. |
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. |
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
to use a closure to implement an interface. #21670 mentions
http.HandlerFunc
, but for me the cases that comes up most often areio.Reader
andio.Writer
, which I want to wrap with some trivial behavior. For example, if I want anio.Writer
, which counts the numbers of bytes written, I can writeHowever, 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
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):
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
io.Reader
andio.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 anio.Reader
or anio.Writer
.reflect
(except to allow the conversion itself) or the use of type-assertions is needed.io
package defined an unexportedtype readerFunc func(p []byte) (int, error)
implementingio.Reader
and afunc ReaderFunc(f func(p []byte) (int, error)) Reader { return readerFunc(f) }
.func([]byte) (int, error)
that carries the method, butruntime.io_reader
.func
are not possible, as the type is unexported. That is a plus, because again, thefunc
type can't carry methods so it would be strange to use it in a type-assertion.var r io.Reader
to afunc([]byte) (int, error)
, by simply evaluatingr.Read
.runtime
package, because it seems like if two different packages do afunc
->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 aspkgpath.func1
in stack-traces.var f func(); g := Fooer(f).Foo
,g
should not benil
, even thoughf
is. Method expressions are nevernil
, 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.
The text was updated successfully, but these errors were encountered: