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

cmd/compile: interface type not recognized when defined on the consumer side #37748

Closed
juliandroid opened this issue Mar 8, 2020 · 10 comments
Closed

Comments

@juliandroid
Copy link

@juliandroid juliandroid commented Mar 8, 2020

What version of Go are you using (go version)?

go version go1.14 linux/amd64

What did you do?

https://play.golang.org/p/NacQPnyXdh-

What did you expect to see?

interface with a method, created locally in the consumer side, to satisfy the compiler.

-- rect/rectangle.go --
package rect
// with implemented method:
func (r RectType) Area() int

-- calc/calculator.go --
package calc
// with interface referring to method implemented in rect.
type AreaReporter interface {
	Area() int
}
// with implemented method:
func (r CalcType) AreaSum(list []AreaReporter) int 

-- service/calcservice.go --
package service
type AreaReporter interface {
	Area() int
}

// with interface referring to method implemented in calc.
type AreaSumProvider interface {
	AreaSum(list []AreaReporter) int
}

I expect AreaReporter interface defined in package service and part of AreaSum() method prototype to be compatible with AreaReporter interface defined in package calc. I.e. passing []AreaReporter of calc package to the service package to be accepted.

What did you see instead?

Compilation error:

cannot use []calc.AreaReporter literal (type []calc.AreaReporter) as type []service.AreaReporter in argument to service.New
...
have AreaSum([]calc.AreaReporter) int
want AreaSum([]service.AreaReporter) int

@juliandroid
Copy link
Author

@juliandroid juliandroid commented Mar 8, 2020

Current recommendation in Go FAQ is to define interfaces on the consumer side - that works only when you have simple, built-in types. The problem appears when you need to satisfy interface with method that has non-built-in type in the signature.

For example:
AreaSum(list []AreaReporter) int

A workaround is to define the type in another package (or on the implementer side) and then import the package in the consumer side, but then that defeat the purpose and introduce dependencies between those packages.

I would expect either both AreaReporter types to be treated as equivalent or documentation and FAQ to be updated.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 8, 2020

You are trying to convert a slice of one defined type to a slice of a different defined type. The language doesn't support that. See this FAQ entry: https://golang.org/doc/faq#convert_slice_with_same_underlying_type.

I'm going to close this issue because this is not a bug with cmd/compile. cmd/compile is correctly implementing the language as defined and intended. If we were going to change this, which I think is unlikely, the way to start is with a language change proposal.

@juliandroid
Copy link
Author

@juliandroid juliandroid commented Mar 8, 2020

@ianlancetaylor
The problem is not related to the slice. I've removed all the slices, but the result is no different:
https://play.golang.org/p/HzkFOhQ51qy

./prog.go:17:36: cannot use c (type calc.CalcType) as type service.AreaSumProvider in argument to service.New:
calc.CalcType does not implement service.AreaSumProvider (wrong type for AreaSum method)
have AreaSum(calc.AreaReporter) int
want AreaSum(service.AreaReporter) int

Both interfaces calc.AreaReporter and service.AreaReporter are locally defined in their packages and have the same single method prototype: Area() int

When I originally encountered that problem it wasn't because of slice argument, but today while I was wondering what silly example to create in order the code to be easy readable I end up cooking up the above.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 9, 2020

You're right, slices are just an example of the problem. But it's the same problem. A defined type in one package is not the same as a defined type in another package, even if they have the same underlying type. As the error message says, you have AreaSum(calc.AreaReporter) int and AreaSum(service.AreaReporter) int. The types calc.AreaReporter and service.AreaReporter are not the same type, even if they have the same underlying type.

Compare to https://play.golang.org/p/RDf1Zc0UJcJ, where I replaced the defined types with their underlying definition. That version compiles.

This is the way the language is designed.

@juliandroid
Copy link
Author

@juliandroid juliandroid commented Mar 9, 2020

@ianlancetaylor thanks for taking time on this! I believe it is a bit more serious issue to just close and ignore though.

I understand that both types are not the same (with reason just a simpler parser, I guess) and the limitations that may cause.

In my case I'm addressing the recommended practice in Go to define the interfaces on the consumer side (see the "DO NOT DO IT!!!" comment), which to me that looks wrong, knowing that you cannot actually do it properly, don't you agree?

If I have an interface that have, for example, two methods Area() and Area2(), then the proposed solution will look like this:

type AreaSumProvider interface {
	AreaSum(ro interface { Area() int; Area2() int }) int
}
// and method definition like this:
func (r CalcType) AreaSum(ro interface { Area() int; Area2() int }) int {}

I've checked in the Go SDK and I couldn't find a single instance where such solution was used (to inline the interface inside the prototype). What I've seen was a lot of places where interface{} was used instead, and giving up the type safety.

To me that means, for every non-trivial case where you have an interface with method(s) that have non-built-in type arguments in the method prototypes it is not feasible to have interfaces defined on the consumer side, but it is better to define them on the implementation side? Do you agree with me based on that example?

You said it is unlikely this to be changed even if I go with making a proposal of changing the spec? Would you like to elaborate?

In current situation if I understand correctly, you as a developer need to unfold all interface definitions until you end up with built-in types. My expectation is that the compiler should do that for me.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 9, 2020

Sorry, I didn't mean to seriously suggest that people should write out interface types inline. I was using that as a demonstration of how the language works.

I would say that most people facing this kind of situation simply define the relevant interface in one place. In your example, introduce a package that defines the AreaReporter interface, and have the other packages import that one. There's no obvious reason for your program to define AreaReporter the same way in two different packages.

I think that changing the spec here would be unlikely to be accepted for the reasons outlined in the FAQ entry I mentioned earlier (https://golang.org/doc/faq#convert_slice_with_same_underlying_type). This is a subtle area in Go's type system, and any changes would have to be carefully thought out to see that they not introduce new problems.

@juliandroid
Copy link
Author

@juliandroid juliandroid commented Mar 9, 2020

Thanks @ianlancetaylor

That is what I'm currently doing in my projects and sharing the types between consumers and implementation, but that solution definitely is not really duck typing in action, it is more close to java explicit interface definition.

What I'm worried is that you can find a lot blog posts based on duck typing, "accept interfaces return structs" paradigm and how and where you are defining the interfaces, for example https://github.com/golang/go/wiki/CodeReviewComments#interfaces or tweets like this one https://twitter.com/davecheney/status/942593128355192832?lang=en by @davecheney

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 9, 2020

The issue does not arise when using values of interface types. For those, structural typing continues to apply (it's not quite duck typing). The issue arises when using aggregate types that incorporate defined interface types (in your examples, slice types or interface types). I think you are basically suggesting the language should relax the requirements on aggregate types incorporating defined types in some way. As I say, that is a subtle area in the language.

@juliandroid
Copy link
Author

@juliandroid juliandroid commented Mar 9, 2020

Actually that is valid for all composite interface types, i.e. interface that have a method using another interface. Typically that happens when you have two different packages that refers to the same interface type.

That is a problem, because where I use that type is in the input arguments of a method, i.e. to follow the principle "accept interfaces return structs" I have to decouple both(or more) packages via interface arguments.

Once you are in that trap, to refer the same type in two different packages and you need to call methods using those interfaces you get compilation errors. Both interfaces unfold to the same methods and they are verifiable at compile time, but to workaround that compilation error, as you suggested, you need to extract the types to different package.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 9, 2020

Yes, what you call composite interface types are an example of what I mean by an aggregate type that refers to a defined interface type.

"Accept interfaces return structs" is a guideline, not a rule that must be followed.

It's possible that you have an XY problem that you could consider taking to golang-nuts or some other forum.

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
2 participants
You can’t perform that action at this time.