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: global interning of equivalent interfaces in the program #58112

Closed
Jorropo opened this issue Jan 27, 2023 · 5 comments
Closed
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented Jan 27, 2023

Problem:

I recently refactored some code (moving a bunch of multi repos modules into a mono repo one), which gave me this solution:

// --- old module
type SomeInterface interface {
    DoStuff()
}

// --- new module (in the monorepo)
type SomeInterface interface {
    DoStuff()
}

// --- a third module (I actually have close to a hundred different modules like this)
type SomeProducerInterface interface {
    Get() old.SomeInterface
}

// --- a consumer module
func Use(SomeProducerInterface) { /* ... */ }

// --- an implementation, that I refactored to use the new API (this is also in the monorepo)
type impl struct{}
func (impl) Get() new.SomeInterface {
    return nil
}

consumer.Use(impl{}) // does not compile:
/*
 ./prog.go:28:13: cannot use impl{} (value of type impl) as type third.SomeProducerInterface in variable declaration:
	impl does not implement third.SomeProducerInterface (wrong type for Get method)
		have Get() new.SomeInterface
		want Get() old.SomeInterface
*/

// other example:
type SomeProducerInterface interface { // new equivalent definition of SomeProducerInterface but in the monorepo
    Get() old.SomeInterface
}
var _ third.SomeProducerInterface = SomeProducerInterface(nil) // does not compile

Current workarounds:

  1. Remove all usage of the old interface definition within the program.
    Not Possible for me (prevents me from doing incremental refactors).
  2. Create stub types and update the old module. (tedious but work, my current solution)
    My main complain with this is that my new monorepo librairy does not consume all of the stubs, so sometime consumers see unexpected types errors when they upgrade my monorepo and they have to manually go get all packages that have been stubbed to use the stubs too.

Solution:

As a solution I propose that for all sets of "compatible types", the compiler picks one of them and use it in all usage of the other compatible types in the set within the program.

Two types are said to be compatible:

  • if they are interfaces, they both need to have the same methods and for all methods arguments and return types need to be compatible in order to be considered compatible, for cyclic case of this definitions it is considered compatible.
  • for everything else, the types have to be the same (current rules).

TODO: Should the behaviour of which type of the compatible set is picked be defined in the spec ?
This would ensure consistency across compiler.

Non breaking changes:

Programs like shown above will now compile.

Breaking changes:

Interfaces types gotten through reflect may now point to other (compatible) interfaces unexpectedly.
So things like this would give a different answer:

package main

import (
	"fmt"
	"reflect"
)

type A interface {
	DoStuff()
}

type B interface {
	DoStuff()
}

type impl struct{}

func (impl) Get() A {
	return nil
}

func main() {
	t := reflect.TypeOf(impl{}).Method(0).Type.Out(0)
	// Right now t is certain to be A
	// But after the change it may be interned to B
	fmt.Println(t.Name())
}
@gopherbot gopherbot added this to the Proposal milestone Jan 27, 2023
@earthboundkid
Copy link
Contributor

The canonical solution to this problem is

package old

import "newpkg"

type SomeInterface = newpkg.SomeInterface

A different solution is to use generics.

func DoThing[I DoStuff()](fn func() I) {
   // ...
}

@seankhliao
Copy link
Member

Please fill out https://github.com/golang/proposal/blob/master/go2-language-changes.md when proposing language changes

@seankhliao seankhliao added LanguageChange Suggested changes to the Go language v2 An incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jan 27, 2023
@Jorropo
Copy link
Member Author

Jorropo commented Jan 27, 2023

@carlmjohnson the first solution is already what I am doing already, it requires manual work:

Create stub types and update the old module. (tedious but work, my current solution)

The second solution requires updating consumers (again work) and make code quality suffer.

Interning interfaces I think also make sense from a language design.
In go everything teaches you that interfaces are transparent, types implicitly convert to narrower or matching interfaces.
However this case I've described is the only case (*I know about) where interfaces are not transparent.

@earthboundkid
Copy link
Contributor

I couldn't find the issue with a quick "site:github.com/golang/go/issues" search, but there is an ancient issue from the early days of Go discussing this problem. If it could be solved easily, it would have been solved by now.

@seankhliao
Copy link
Member

Duplicate of #7512

@seankhliao seankhliao marked this as a duplicate of #7512 Jan 27, 2023
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2023
Jorropo added a commit to Jorropo/lotus that referenced this issue May 25, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 1, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 1, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 1, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 1, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 2, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 2, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 2, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 2, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 2, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
aschmahmann pushed a commit to Jorropo/lotus that referenced this issue Jun 6, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
hannahhoward pushed a commit to Jorropo/lotus that referenced this issue Jun 19, 2023
This migrates everything except the `go-car` librairy: ipfs/boxo#218 (comment)

I didn't migrated everything in the previous release because all the boxo code wasn't compatible with the go-ipld-prime one due to a an in flight (/ aftermath) revert of github.com/ipfs/go-block-format. go-block-format has been unmigrated since slight bellow absolutely everything depends on it that would have required everything to be moved on boxo or everything to optin into using boxo which were all deal breakers for different groups.

This worked fine because lotus's codebase could live hapely on the first multirepo setup however boost is now trying to use boxo's code with lotus's (still on multirepo) setup: https://filecoinproject.slack.com/archives/C03AQ3QAUG1/p1685022344779649

The alternative would be for boost to write shim types which just forward calls and return with the different interface definitions.

Btw why is that an issue in the first place is because unlike what go's duck typing model suggest interfaces are not transparent golang/go#58112, interfaces are strongly typed but they have implicit narrowing. The issue is if you return an interface from an interface Go does not have a function definition to insert the implicit conversion thus instead the type checker complains you are not returning the right type.

Stubbing types were reverted ipfs/boxo#218 (comment)

Last time I only migrated `go-bitswap` to `boxo/bitswap` because of the security issues and because we never had the interface return an interface problem (we had concrete wrappers where the implicit conversion took place).
@golang golang locked and limited conversation to collaborators Jan 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants