Skip to content

proposal: spec: permit non-interface types that support == to satisfy the comparable constraint #52474

Closed

Description

@ianlancetaylor

Background

In Go 1.18 we introduced a type constraint comparable. The type constraint is satisfied by any type that supports == and for which == will not panic. For example, it is satisfied by int and string and by composite types like struct { f int } or [10]string. On the other hand, it is not satisfied by types like any or interface { String() } or struct { f any } or [10]interface{ String() }.

This decision has led to considerable discussion; for example, #50646, #51257, #51338.

When considering whether a type argument satisfies the comparable constraint, there are two cases to consider.

For an interface type, the rule in the spec is simple: an interface type T implements an interface I if "the type set of T is a subset of the type set of I." By this definition the type any does not implement comparable: there are many elements in the type set of any that are not in the type set of comparable. More generally, no basic interface implements comparable. Some general interfaces, such as interface { ~int }, implement comparable; it is not possible today to use this kind of general interface as an ordinary type, but a type parameter constrained by such a general interface will implement comparable.

For a non-interface type, the rule is different. A non-interface type T implements an interface I if it "is an element of the type set of I." For a language-defined type like comparable, the language defines that type set. The current definition says that comparable "denotes the set of all non-interface types that are comparable."

However, the current implementation is slightly different. In the current implementation, a composite type that includes an element of interface type does not implement comparable, although such a composite type is "comparable" according to the definition in the spec. The implementation was written that way based on the belief that comparable should only be implemented by types that will not panic when not used in a comparison. This is a valuable property, but it leads to some complications.

For example (this is based on a comment by @Merovius), consider a package "annotated" that implements an annotated value type:

type Value struct {
	Annotation string
	Val        any
}
// Various methods on Value.

Now consider a package "p" that uses that type, and suppose that package p ensures that it only stores values with comparable types in the Val field. It's fine for package p to use the type map[annotated.Value]bool. That works because the type annotated.Value is comparable according to the language definition. However, annotated.Value does not implement comparable, because the type of the element Val is an interface type that does not implement comparable. That means that code like this does not work:

type Set[Elem comparable] map[Elem]bool
var ValSet Set[annotated.Value]

Since annotated.Value does not implement comparable, the compiler will reject this code.

There is no good workaround for package p in this scenario. There is no way for p to say that it wants a version of the type annotated.Value that restricts Val to comparable types. It wouldn't be appropriate to change the annotated package, since that package can also be used by other packages that don't want to restrict Val to comparable types.

Proposal

Therefore, we propose that we change the implementation. Arguably the implementation is not quite following the spec here, so there may be no need to change the spec.

The new rule is:

As before:

  • an interface type implements comparable if the type set of the interface type is a subset of the type set of comparable
  • a non-interface type implements comparable if it is a member of the type set of comparable

For example, by this definition, annotated.Value implements comparable, and the problem outlined above goes away.

Note on interface types

This change means that there is little reason to seek the comparable version of a non-interface type T, as such types are now always comparable or never comparable. However, people may still want to get the comparable version of an interface type. For example, code might want the fmt.Stringer type but only permitting comparable types. If we adopt #51338, then people can get this type by writing interface { comparable; fmt.Stringer }. However, that is not part of this proposal.

Timing

Because of the confusion in this area, and the apparent discrepancy between the spec and the implementation, it may be worth implementing this in the 1.19 release, even though that is soon.

Activity

  1. added this to the Proposal milestone on Apr 21, 2022
  2. DeedleFake commented on Apr 21, 2022

    @DeedleFake

    So, to clarify, this would allow the following?

    func Equal[T comparable](t1, t2 T) bool {
      return t1 == t2
    }
    
    type Example struct {
      Val any
    }
    
    func main() {
      Equal(Example{""}, Example{""}) // true
      Equal(Example{[]int(nil)}, Example{[]int(nil)}) // panic
    }

    However, Equal(any(0), any(0)) would still not be allowed, correct? And in the panicking case where it is allowed, it would just be up to the user to make sure that nothing that isn't actually comparable is used with a comparable constraint?

    Without allowing any to be used directly, it would allow a workaround for the issue of CustomMap[K comparable, V any] not working as CustomMap[any, string], but it would be kind of messy because it would require that it be wrapped in a struct type first. That seems like it'll be confusing to new users.

  3. zephyrtronium commented on Apr 21, 2022

    @zephyrtronium
    Contributor

    Practically, this seems no different to me from just removing the special case that interface types are comparable types that are not in the type set of comparable. If I have a generic wrapper for sync.Map and I want the type behavior that sync.Map currently provides, I write var m Map[[1]any]any instead of var m Map[any]any, along with extra syntax around uses.

  4. atdiar commented on Apr 21, 2022

    @atdiar

    The issue is that an interface can reside at an arbitrary deep level within a struct. (struct of struct of struct...)

    Hence, it may not be obvious that a comparison may panic.

    I'm not too convinced and would rather keep the current, stricter, behavior.
    It seems to me that this example exposes a rather rare pathological case where the comparison intent is being guaranteed by convention and not code.

    On the other hand, I see the other issue: it forces the creators of packages to find out in advance whether an interface field should require comparable or not.
    That definitely deserves more thinking.
    Would be great to get such examples from the corpus of Go code that exists at large.
    I looked but I don't have such examples myself.

  5. jimmyfrasche commented on Apr 21, 2022

    @jimmyfrasche
    Member

    I think ultimately the least confusing thing to allow interface values to match the comparable constraint. That doesn't stop you from allowing values of the comparable interface later. It is admittedly somewhat confusing but this is carpet stuck under furniture that's too heavy to move so it's going to stick up somewhere.

  6. atdiar commented on Apr 21, 2022

    @atdiar

    I think ultimately the least confusing thing to allow interface values to match the comparable constraint. That doesn't stop you from allowing values of the comparable interface later. It is admittedly somewhat confusing but this is carpet stuck under furniture that's too heavy to move so it's going to stick up somewhere.

    Unfortunately, I don't think that it would mix well with allowing constraint interfaces (such as comparable) as regular interfaces.
    That would make any interface embedding comparable only usable as a constraint. A "color" that could be more annoying to deal with.

  7. jimmyfrasche commented on Apr 21, 2022

    @jimmyfrasche
    Member

    You can't put interface values in other interfaces so you could still allow values of the comparable interface.

  8. atdiar commented on Apr 21, 2022

    @atdiar

    You can't put interface values in other interfaces so you could still allow values of the comparable interface.

    Edit: (I had misunderstood.your response)

    The issue lies with asserting an interface value to comparable or any interface embedding comparable. That would cause an incongruity.
    Which would also occur on variable assignment.

  9. ianlancetaylor commented on Apr 21, 2022

    @ianlancetaylor
    ContributorAuthor

    @DeedleFake I agree with what you wrote above.

    @DeedleFake @zephyrtronium Yes, it is easy to use a struct or array to permit using an arbitrary interface type to satisfy a comparable constraint. I think that's OK. As noted, I think that #51338 will provide a cleaner way to do the same thing while avoiding the potential panic.

    @jimmyfrasche We could do that. In fact we could do that even after adopting this proposal if we decide to. I'm not personally sure that it's the least confusing approach; see the lengthy discussion in #50646 when we started down that path before changing our minds. I think it's become clear that any approach in this area is confusing.

  10. Merovius commented on Apr 21, 2022

    @Merovius
    Contributor

    @atdiar

    Unfortunately, I don't think that it would mix well with allowing constraint interfaces

    I think it is clear that we should either allow comparable as a normal interface type or make interface-types implement comparable, but not both. They are complementary, equally simple solutions to the issues.

  11. jimmyfrasche commented on Apr 21, 2022

    @jimmyfrasche
    Member

    @Merovius that's not at all clear to me. Doing both seems like a perfectly fine thing to do me—and the best thing to do overall. It's possible I'm missing something, though.

  12. atdiar commented on Apr 21, 2022

    @atdiar

    @atdiar

    Unfortunately, I don't think that it would mix well with allowing constraint interfaces

    I think it is clear that we should either allow comparable as a normal interface type or make interface-types implement comparable, but not both. They are complementary, equally simple solutions to the issues.

    Well, it also depends on whether there are plans to make all constraint interfaces usable as regular interface types (which was in consideration, I recall).
    The present decisions could preclude some future plans to do just that.

  13. Merovius commented on Apr 21, 2022

    @Merovius
    Contributor

    @jimmyfrasche If all interface types implement comparable and comparable is a type, then

    var a any = func() {}
    var b comparable = a

    should be allowed. In that case, I don't see the point of making comparable a type, as it does not seem to provide better guarantees than any.

    To me, making comparable a type is a specific solution to guarantee that func Eq[T comparable](a, b T) bool { return a == b } never panics. To me, that goal isn't achieved if you can then instantiate Eq[any].

    i.e. to me, they are both different solutions to the same problem, attacking it from opposite angles.

  14. jimmyfrasche commented on Apr 22, 2022

    @jimmyfrasche
    Member

    You would have to write

    var a any = func() {}
    var b comparable = a.(comparable)

    replace comparable with any other nonempty interface and your example fails at compile time and my rewrite fails at runtime so I would assume that the same would hold true for comparable.

  15. 52 remaining items

  16. Merovius commented on Apr 23, 2022

    @Merovius
    Contributor

    @abligh

    This is despite the fact that comparison of interface values can never panic (as far as I know).

    It can

    could we at least have some form of constraint mapkey which includes only those types that are used as map keys - I think this is a superset of comparable

    It's not a superset. The types usable as map keys are exactly the same as the types which can be compared using ==. And storing an interface value in a map panics exactly if comparing that value panics.

  17. abligh commented on Apr 23, 2022

    @abligh

    @Merovius

    could we at least have some form of constraint mapkey which includes only those types that are used as map keys - I think this is a superset of comparable

    It's not a superset. The types usable as map keys are exactly the same as the types which can be compared using ==. And storing an interface value in a map panics exactly if comparing that value panics.

    Sorry, I put comparable in backticks meaning the keyword not the language definition, accidentally illustrating the confusion :-). I meant "could we at least have some form of constraint mapkey which includes only those types that are used as map keys - I think this is a superset of comparable" - i.e. the set of things that can be used as keys of maps is a superset of the things which implement comparable either under this proposal or Go 1.18. I know the set of things that can be used as keys of maps is exactly equal to the things that are comparable (per the language spec) (i.e. can be compared using ==), though as you point out some comparisons may panic at runtime.

  18. Merovius commented on Apr 23, 2022

    @Merovius
    Contributor

    @abligh My point was kind of that, if we think the behavior of mapkey is better, then comparable should just behave that way. There is no difference between "useful as a map key" and "useful for comparisons", both would panic or not panic in the same situations and allow or disallow the same operations to happen. I don't think we should have two things in the language to mean the same thing, where the only difference is that one behaves worse than the other (or, let's say "some people think one of them is worse, others think the other is worse, but neither is actually better or worse").

    If we think mapkey is the better behavior, we should adopt #52509.

  19. jimmyfrasche commented on Apr 23, 2022

    @jimmyfrasche
    Member

    Meta. I think regardless of the outcome of these various proposals what comparable means needs to match what the spec defines as comparable, even if that means coming up with a new term for types that permit == but don't match this hypothetical new definition of comparable.

  20. atdiar commented on Apr 24, 2022

    @atdiar

    Created a sort of counter-proposal #52531
    I think it might be a way to address the issue about comparability in its entirety.
    (it's also my first proposal and issue ever 🙂)

  21. changkun commented on Apr 25, 2022

    @changkun
    Member

    Will the following code panic if this proposal is accepted?

    package main
    
    type s struct {
    	f any
    }
    
    func eq[T comparable](x, y T) bool { return x == y }
    
    func main() {
    	_ = eq(s{func() {}}, s{func() {}})
    }

    If yes, how does it differ from the following case?

    package main
    
    func eq[T comparable](x, y T) bool { return x == y }
    
    func main() {
    	_ = eq(any(func() {}), any(func() {}))
    }
  22. Merovius commented on Apr 25, 2022

    @Merovius
    Contributor

    @changkun

    Will the following code panic if this proposal is accepted?

    Yes.

    If yes, how does it differ from the following case?

    That case is not valid, currently or under this proposal. If you mean "it doesn't seem very different, so if we should allow one, why not allow the other?", I agree.

  23. changkun commented on Apr 25, 2022

    @changkun
    Member

    Yes.

    That's great. So we can't prevent panic anyway. I think this might clarify #52474 (comment) and left for the decision.

  24. Merovius commented on Apr 25, 2022

    @Merovius
    Contributor

    FWIW I think there is an argument to be made that preventing panics is important (not in favor of this proposal, because it doesn't, but e.g. for #51338), even if we didn't do it so far, which goes like this:

    1. We maybe want, at some point, want to use type element interfaces to add variant types to Go
    2. If we do that, we probably want to be able to do it for comparable as well, because it would be strange to have an exception to this
    3. If comparable becomes a full type, it should probably be distinguished by preventing panics on comparison statically, because that would be the only thing differentiating it from any, in terms of real benefit in usage.
    4. For consistency, comparable as a constraint should then be fulfilled by the same set of types which are assignable to comparable.
    5. Therefore, we should maintain the meaning of comparable as a constraint to "types on which equality is statically prevented from panicing"

    This chain would then argue against this proposal as well as #52509, as they don't maintain that meaning. So, if we want to use type element interfaces as variant types at some point, rejecting this proposal and #52509 and accepting #51338 will ultimately lead to the most consistent and simple language.

    Personally, I don't like that this argument depends on wanting to do something which we haven't yet decided to do. I don't know how likely it is that we will and at what point that would happen. And we might want to solve the "instantiating comparable constrained generic types with interface types" before that. But it is a fair argument.

    (I also agree with @jimmyfrasche here, that we should then make sure comparable reflects what the spec calls comparable)

  25. bcmills commented on Apr 25, 2022

    @bcmills
    Contributor

    I went back to the discussion in #51338 to evaluate how this proposal compares.

    As far as I can tell, it does have one advantage — namely, it allows type-assertions to comparable to succeed for all values that are actually comparable. (Compare #51338 (comment).)

    However, that comes with an equivalent disadvantage: a successful type-assertion to comparable would not actually ensure that the value can be compared without panicking. So it would allow run-time weakening of static checks, but would not succeed in providing an equivalent dynamic check that programs could use to avoid the panic.

  26. ianlancetaylor commented on Apr 26, 2022

    @ianlancetaylor
    ContributorAuthor

    Thanks for the good discussion. I'm persuaded that this proposal makes the language harder to understand without providing a commensurate improvement. I'm withdrawing it.

  27. ianlancetaylor commented on Apr 29, 2022

    @ianlancetaylor
    ContributorAuthor

    See #52614 for another take on this problem.

  28. locked and limited conversation to collaborators on Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions