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: type switch case statements with multiple interface types should produce an intersection of methods #65031

Closed
1 of 4 tasks
lu4p opened this issue Jan 9, 2024 · 10 comments
Labels
LanguageChange Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Milestone

Comments

@lu4p
Copy link

lu4p commented Jan 9, 2024

Go Programming Experience

Experienced

Other Languages Experience

Go, Rust, TS/JS, C/C++, Python

Related Idea

  • Has this idea, or one like it, been proposed before?
  • Does this affect error handling?
  • Is this about generics?
  • Is this change backward compatible? Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit

Has this idea, or one like it, been proposed before?

No.

Does this affect error handling?

No.

Is this about generics?

No.

Proposal

// typ is of type types.Type
switch newTyp := typ.(type) {
case *types.Named, *types.Interface:

}

Currently newTyp in the case statement has the types.Type type:

type Type interface {
	Underlying() Type
	String() string
}

Proposed change:
Instead it should have the type of an implicitly generated interface, which consists of the intersection of all methods both types have in common.

In current go this would be:

switch newTyp := typ.(type) {
case *types.Named, *types.Interface:
	{
		newTyp := newTyp.(interface {
			Method(i int) *types.Func
			NumMethods() int
			Underlying() types.Type
			String() string
		})
	}
}

I know that this helps people who analyze go code, because it allows for more concise type switches, which are very common when analyzing code. There are probably other use cases, like parsers, anytime when there is a type switch to narrow down the type and then calling a method on that type.

It would also be nice if this works for the intersection of struct fields, but I think additional work on generics is required first, in order to have a similarly low cost "syntactic sugar" implementation.

Language Spec Changes

This new behavior would need to be documented under https://go.dev/ref/spec#Type_switches

Informal Change

In case statements with multiple types, the switched type is of an interface which contains the intersection of both types methods.

Is this change backward compatible?

Strictly speaking no, because the output of reflect.TypeOf may change, however I don't think this will break programs. reflect.TypeOf could pretend that the implicitly generated interface doesn't exist and tell the caller that typ is of type types.Type, then it would be backward compatible.

Before the change (this is real code from here):

switch typ := typ.(type) {
case *types.Named:
	for i := 0; i < typ.NumMethods(); i++ {
		if m := typ.Method(i); token.IsExported(m.Name()) && isSupportedSig(m) {
			methods = append(methods, m)
			if len(methods) > limitFunctionCount {
				break
			}
		}
	}
case *types.Interface:
	for i := 0; i < typ.NumMethods(); i++ {
		if m := typ.Method(i); token.IsExported(m.Name()) && isSupportedSig(m) {
			methods = append(methods, m)
			if len(methods) > limitFunctionCount {
				break
			}
		}
	}
}

After the change:

switch typ := typ.(type) {
case *types.Named, *types.Interface:
	for i := 0; i < typ.NumMethods(); i++ {
		if m := typ.Method(i); token.IsExported(m.Name()) && isSupportedSig(m) {
			methods = append(methods, m)
			if len(methods) > limitFunctionCount {
				break
			}
		}
	}
}

Orthogonality: How does this change interact or overlap with existing features?

This should make go code easier to read and write.

Would this change make Go easier or harder to learn, and why?

Easier, I think the current behavior is non intuitive.

Cost Description

This requires someone to implement the transformation in the compiler. Also gopls needs to be updated to know that this is something the compiler does. And someone needs to update the language spec.

Changes to Go ToolChain

gopls, compile

Performance Costs

Some small compile time cost, no run time cost.

Prototype

This code:

switch typ := typ.(type) {
case *types.Named, *types.Interface:
	for i := 0; i < typ.NumMethods(); i++ {
		if m := typ.Method(i); token.IsExported(m.Name()) && isSupportedSig(m) {
			methods = append(methods, m)
			if len(methods) > limitFunctionCount {
				break
			}
		}
	}
}

Could be converted to:

switch typ := typ.(type) {
case *types.Named, *types.Interface:
	{
		typ := typ.(interface {
			Method(i int) *types.Func
			NumMethods() int
			Underlying() types.Type
			String() string
		})
               for i := 0; i < typ.NumMethods(); i++ {
		    if m := typ.Method(i); token.IsExported(m.Name()) && isSupportedSig(m) {
			    methods = append(methods, m)
			    if len(methods) > limitFunctionCount {
		                break
			    }
		    }
	       }
	}
}
@lu4p lu4p added LanguageChange Proposal v2 A language change or incompatible library change labels Jan 9, 2024
@gopherbot gopherbot added this to the Proposal milestone Jan 9, 2024
@rittneje
Copy link

rittneje commented Jan 9, 2024

Strictly speaking no, because the output of reflect.TypeOf may change, however I don't think this will break programs. reflect.TypeOf could pretend that the implicitly generated interface doesn't exist and tell the caller that typ is of type types.Type, then it would be backward compatible.

I'm not sure that reflect.TypeOf itself is a problem, since that is going to give you the underlying concrete type, not the variable type. However, I do see two potential issues:

  1. pointers: https://go.dev/play/p/19MX5hQyTm5
  2. generics: https://go.dev/play/p/quLstpawYiX

@lu4p
Copy link
Author

lu4p commented Jan 9, 2024

@rittneje valid concerns, for this to be backward compatible returns of values which "just" implement the interface must be valid

Simplified example:
https://go.dev/play/p/kJuflSP1QbC

@apparentlymart
Copy link

apparentlymart commented Jan 10, 2024

This is an interesting idea, and I do like how it echoes the way that constrained type parameters on generic functions end up supporting the intersection of methods that all of the allowed types support.

I don't think there's any existing precedent in Go for the compiler silently generating a unnameable type, which means there are likely to be unexpected consequences of adding the first case of that. However, it does seem to me that doing this for interface types in particular is plausible because the language already allows implicit conversions between compatible interface types anyway, and so there's a reasonable set of things you could do with a value of one of these unnameable types that would still be useful.

Making the following legal also seems like an implication of this proposal:

type Example interface {
    Method(i int) *types.Func
    NumMethods() int
    Underlying() types.Type
    String() string
}

func UsesExample(ex Example) {
    // ...
}

func Elsewhere(typ reflect.Type) {
    switch newTyp := typ.(type) {
    case *types.Named, *types.Interface:
        // The automatically-created anonymous interface type
        // is compatible with the named type Example, so
        // no explicit conversion is required here.
        UsesExample(newTyp)
    }
}

This is admittedly contrived because it would've been equivalent to write case Example: and thus make newTyp have type Example directly rather than the anonymous interface type. But I think it follows naturally from what the proposal describes, illustrating that this is a relatively orthogonal change that doesn't drastically change the existing "flavor" of the language.


The fact that the "hidden" anonymous interface type would be visible through reflect is a little concerning for backward-compatibility, but I would guess that doing reflection in a type switch arm like that is pretty rare and that anyone who is doing it knows they are doing it. If it were something that got enabled only when the module's go.mod specifies a new enough version in its go directive then maintainers of modules that would be affected by this change can delay opting in to the new language version without preventing other modules from adopting this behavior. (In principle the type switch and the reflection might be in different collaborating modules, which would make this opt-in a little messier.)


I am also a little concerned about how this automatically-generated anonymous type would appear in error messages when there's a type mismatch.

It seems like we'd need to spell it out in full as interface { Method(i int) *types.Func, NumMethods() int, Underlying() types.Type, String() string } in order for it to be clear what's going on. Just referring to it as reflect.Type would be misleading, because it's a subtype of reflect.Type with some additional methods. 🤔

@adonovan
Copy link
Member

adonovan commented Jan 10, 2024

One consequence of this change is that the call to z.F would be to an interface method call that has no declaration:

type X struct{}
func (X) F() {}
type Y struct{}
func (Y) F() {}

switch z := z.(type) {
case X, Y:
	z.F()

This would affect the go/types.Defs API, and many tools that, like it, assume every identifier refers to a single declaration.
Is "the" declaration now the set {X.F, Y.F}? If it is, then it breaks another invariant that an interface method call refers to an interface method declaration, not to a concrete method declaration. Both of these would be complex breaking changes for tools like gopls.

(That said, the solution to #51259 and the analogous feature for methods, both of which seem like important generics features, will pose the same problem, and other language changes sometimes do break go/types API incompatibility.)

@seankhliao seankhliao changed the title proposal: Go 2: type switches should be smarter proposal: Go 2: type switch case statements with multiple interface types should produce an intersection of methods Jan 10, 2024
@timothy-king
Copy link
Contributor

I kinda feel like the underlying request is for interface{*types.Named | *types.Interface} to be representable [either as a dynamic value or have special static support in type switches] and have its method set be the intersection of the listed elements in the union. The syntax would then just be case interface{*types.Named | *types.Interface}:. Maybe I am misrepresenting the request?

@lu4p
Copy link
Author

lu4p commented Jan 11, 2024

@timothy-king I'm not set on the specific syntax, your syntax would also work.

@findleyr
Copy link
Contributor

findleyr commented Feb 7, 2024

This looks like a subset of https://go.dev/issue/57644.

@adonovan
Copy link
Member

adonovan commented Feb 7, 2024

I know the domain of go/types well, and while I agree that it is sometimes a nuisance to have to type out a throwaway interface type for a type switch, it is not very common, and it is exceedingly rare to need the interface to have more than one method. So I don't think there's a compelling need for a language change here.

@findleyr
Copy link
Contributor

findleyr commented Feb 7, 2024

Given there is a workaround, and a more general proposal in #57644, this is a likely decline. Leaving open for four weeks for final comments.
— rfindley for the language proposal review group

@findleyr
Copy link
Contributor

findleyr commented Mar 6, 2024

No change in consensus, so declined.
— rfindley for the language proposal review group

@findleyr findleyr closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

7 participants