defining a method of a generic comparison with one argument unconstrained in a package is a bug

[nsajko]

Defining a == method with one of the arguments untyped should never be done in a registered package. This is done here, and in some other places:

MultivariatePolynomials.jl/src/comparison.jl

Lines 118 to 119 in 674b38f

	Base.:(==)(α, q::RationalPoly) = α * q.den == q.num
	Base.:(==)(q::RationalPoly, α) = α == q

The reason that's a bug is that that kind of method signature will be ambiguous with regards to methods defined in other packages. Observe, fresh REPL session:

julia> struct A end  # in one package

julia> struct B end  # in another package

julia> function Base.:(==)(::A, ::Any) end

julia> function Base.:(==)(::Any, ::A) end

julia> function Base.:(==)(::B, ::Any) end

julia> function Base.:(==)(::Any, ::B) end

julia> using Test

julia> detect_ambiguities(Main)
12-element Vector{Tuple{Method, Method}}:
 (==(::A, ::Any) @ Main REPL[3]:1, ==(::Any, ::Missing) @ Base missing.jl:76)
 (==(::Any, ::A) @ Main REPL[4]:1, ==(::Missing, ::Any) @ Base missing.jl:75)
 (==(::A, ::Any) @ Main REPL[3]:1, ==(w, v::WeakRef) @ Base gcutils.jl:36)
 (==(::Any, ::A) @ Main REPL[4]:1, ==(w::WeakRef, v) @ Base gcutils.jl:35)
 (==(::A, ::Any) @ Main REPL[3]:1, ==(::Any, ::B) @ Main REPL[6]:1)
 (==(::A, ::Any) @ Main REPL[3]:1, ==(::Any, ::A) @ Main REPL[4]:1)
 (==(::B, ::Any) @ Main REPL[5]:1, ==(::Any, ::B) @ Main REPL[6]:1)
 (==(::Any, ::B) @ Main REPL[6]:1, ==(::Missing, ::Any) @ Base missing.jl:75)
 (==(::B, ::Any) @ Main REPL[5]:1, ==(w, v::WeakRef) @ Base gcutils.jl:36)
 (==(::B, ::Any) @ Main REPL[5]:1, ==(::Any, ::Missing) @ Base missing.jl:76)
 (==(::Any, ::A) @ Main REPL[4]:1, ==(::B, ::Any) @ Main REPL[5]:1)
 (==(::Any, ::B) @ Main REPL[6]:1, ==(w::WeakRef, v) @ Base gcutils.jl:35)

Or, more minimally and concretely:

julia> struct A end  # in one package

julia> function Base.:(==)(::A, ::Any) end

julia> struct B end  # in another package

julia> function Base.:(==)(::Any, ::B) end

julia> A() == B()
ERROR: MethodError: ==(::A, ::B) is ambiguous.

Candidates:
  ==(::A, ::Any)
    @ Main REPL[2]:1
  ==(::Any, ::B)
    @ Main REPL[4]:1

Possible fix, define
  ==(::A, ::B)

Stacktrace:
 [1] top-level scope
   @ REPL[5]:1

I saw this on issue #290 first, but decided to open a new issue, since what I'm pointing out seems worse than invalidation of precompiled code and is not related to invalidation.

[blegat]

The strategy I use for most operators (not only ==) is to define op(::Any, ::A), op(::A, ::Any) and op(::A, ::A) then you don't have any ambiguity. If several types are defined in the package you use A being the union of all of them. Then you redirect the methods that received Any to a method op_constant to avoid any issues with ambiguities. So in principle 3 methods are enough.

[nsajko]

define op(::Any, ::A), op(::A, ::Any) and op(::A, ::A) then you don't have any ambiguity

This is not true. The extra method obviously has no effect on the examples I give above.

[nsajko]

To be specific:

julia> struct A end  # in one package

julia> function Base.:(==)(::A, ::A) end

julia> function Base.:(==)(::Any, ::A) end

julia> function Base.:(==)(::A, ::Any) end

julia> struct B end  # in another package

julia> function Base.:(==)(::B, ::B) end

julia> function Base.:(==)(::Any, ::B) end

julia> function Base.:(==)(::B, ::Any) end

julia> A() == B()
ERROR: MethodError: ==(::A, ::B) is ambiguous.

Candidates:
  ==(::A, ::Any)
    @ Main REPL[4]:1
  ==(::Any, ::B)
    @ Main REPL[7]:1

Possible fix, define
  ==(::A, ::B)

Stacktrace:
 [1] top-level scope
   @ REPL[9]:1

[blegat]

Sorry I didn't read in detail, yes indeed, it won't work of two packages do it. Would the suggestion in #290 (comment) address the concern ?

[nsajko]

FYI: created a doc PR for JuliaLang/julia regarding a generalization of the issue here, to warn against it in the Manual:

• manual: methods: new section on avoiding ambiguity: "playing nicely" JuliaLang/julia#58005

Perhaps take a look at it if there's time, it could be good to review if something is not clear or otherwise.

Would the suggestion in #290 (comment) address the concern ?

I believe so, yeah.