Issues with the new x^y Val lowering

[malmaud]

I worry that the new lowering from 3222749 is going to break people's code in a way that will seem very mysterious to them. Possibly the loss of referential transparency it introduces outweighs the benefits. Consider:

julia> type T
       x
       end

julia> ^(t::T, b) = t.x + b
^ (generic function with 1 method)

julia> T(2)^3
ERROR: MethodError: no method matching +(::Int64, ::Type{Val{3}})
Closest candidates are:
  +(::Any, ::Any, ::Any, ::Any...) at operators.jl:420
  +(::T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}, ::T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}) where T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8} at int.jl:32
  +(::Real, ::Complex{Bool}) at complex.jl:235
  ...
Stacktrace:
 [1] ^(::T, ::Type{T} where T) at ./REPL[2]:1

This will be quite confusing to users who aren't aware of this particular optimization.

[StefanKarpinski]

cc @stevengj

[stevengj]

Oh, right, I originally had a fallback for ^(x, ::Val{p}), but then @timholy was concerned that this would cause too many method ambiguities.

If you just define ^(t::T, b::Number) above then it will work via the fallback ^(x,p) = internal_pow(x,p).

[stevengj]

There is, of course, an inherent tradeoff here. If you want x^literal to be a different function than x^variable so that it is available for specialized methods (e.g integer^-literal, unitful^literal, or inlining), then by the same token this fact must be occasionally visible to users defining new methods.

[stevengj]

The question is, how often will people actually encounter this? (How often do people define new ^ methods where the exponent is not a number?) And how confused will they be in practice? The main purpose of including this in 0.6 is to find out.

Is there a way we could define a better fallback method or a better error message?

[stevengj]

(This is not a bug. This is the designed behavior.)

[mbauman]

I think this may be a case where we want method ambiguities. Note that the ^ dispatch tree has had an ambiguity-inducing definition with ^(x, ::Integer) since… forever. In this case, you don't want to accidentally get a Val in a place where you don't expect it — the ambiguity error will be relevant and somewhat self-documenting.

But there's also the fact that the fallbacks/ambiguity will only work if ^ === Base.:^. In the case Jon reported, he's not extending Base.:^.

[stevengj]

I tend to agree than an ambiguity error might be better here, but see also the discussion in #20530.

The basic point is that the goal of x^literal -> x^Val{literal} is to expose literal exponents for dispatch by user-defined ^ methods. We cannot simultaneously expose it for dispatch and make it invisible to all user-defined ^ methods.

(In @malmaud's case, to the extent that this models a real case and not just a contrived example, my expectation is that in any realistic version of this you'd be extending Base.^.)

[ajkeller34]

Humor me for a moment, and suppose we don't lower x^literal -> x^Val{literal} but rather as x^literal -> Base.internal_pow(x, Val{literal}). What should happen?

Well, if x<:HWNumber, then you get the desired behavior: for integer literals, you call one of the internal_pow methods, with special behavior for 0,1,2,3 and a fallback of x^p. If !(x<:HWNumber), then you just call the fallback x^p. This means that provided the user is doing import Base:^, the following happens:

julia> import Base: ^

julia> type T
       x
       end

julia> ^(t::T, b) = t.x + b
^ (generic function with 53 methods)

julia> T(2)^3
ERROR: MethodError: ^(::T, ::Int64) is ambiguous. Candidates:
  ^(x, p::Integer) in Base at intfuncs.jl:196
  ^(t::T, b) in Main at REPL[3]:1
Stacktrace:
 [1] internal_pow(::T, ::Type{Val{3}}) at ./intfuncs.jl:207

julia> ^(t::T, b::Integer) = t.x + b
^ (generic function with 54 methods)

julia> T(2)^3
5

The key point is that we get expected behavior, but we still leave a path for the specialist: importing Base.internal_pow and specializing for new types.

To get this behavior, patch here as:

`(call (|.| Base (quote internal_pow)) ,(caddr e) (call (core apply_type) (top Val) ,(cadddr e)))))

Now, I recognize that this still doesn't give the desired behavior when the user doesn't import ^. Here's what happens in that case, given the parser patch is implemented:

julia> ^(t::T, b) = t.x + b
^ (generic function with 1 method)

julia> T(2)^3
ERROR: MethodError: no method matching *(::T, ::T)
Closest candidates are:
  *(::Any, ::Any, ::Any, ::Any...) at operators.jl:420
Stacktrace:
 [1] power_by_squaring(::T, ::Int64) at ./intfuncs.jl:182
 [2] internal_pow(::T, ::Type{Val{3}}) at ./intfuncs.jl:207

The problem is that internal_pow doesn't know which ^ was used originally: the one the user defined, or the one in Base. My parser hacking skills are limited, but I propose someone who knows what they are doing try the following: lower as x^literal -> Base.internal_pow(x, Val{literal}, ^) and rewrite this method of internal_pow to use the function that was passed in. In this way, perhaps everything may just work?

For what it's worth, tests seem to pass with my first proposed parser change, except for the new PR 20530 tests because now you'd need to specialize internal_pow instead of ^ for the Val types (see this line).

edit: note that in the original example, you get the same method error whether you import ^ or not.