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

Spurious deprecation warning from Vararg !<: Type #49553

Closed
timholy opened this issue Apr 28, 2023 · 9 comments · Fixed by #49558
Closed

Spurious deprecation warning from Vararg !<: Type #49553

timholy opened this issue Apr 28, 2023 · 9 comments · Fixed by #49558

Comments

@timholy
Copy link
Member

timholy commented Apr 28, 2023

I don't understand why this function definition seems prohibited (running with --depwarn=error):

julia> struct CenterIndexedArray{T,N,A<:AbstractArray} <: AbstractArray{T,N}
           data::A
           halfsize::NTuple{N,Int}

           function CenterIndexedArray{T,N,A}(data::A) where {T,N,A<:AbstractArray}
               new{T,N,A}(data, _halfsize(data))
           end
       end

julia> CenterIndexedArray(A::AbstractArray{T,N}) where {T,N} = CenterIndexedArray{T,N,typeof(A)}(A)
CenterIndexedArray

julia> CenterIndexedArray{T,N}(::UndefInitializer, sz::Vararg{<:Integer,N}) where {T,N} =
           CenterIndexedArray(Array{T,N}(undef, sz...))
ERROR: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
Stacktrace:
 [1] UnionAll(v::TypeVar, t::Any)
   @ Core ./boot.jl:257
 [2] top-level scope
   @ REPL[3]:1

xref #38136, CC @Keno

@KristofferC
Copy link
Member

Does it work to write Vararg{Integer,N} instead?

@timholy
Copy link
Member Author

timholy commented Apr 28, 2023

Ah, yes it does. Not sure whether this should be problematic, though. Or if it is, we need a clearer error message.

@KristofferC
Copy link
Member

Yeah, the error message is not great; I just remember that this error message means to remove the <:. Maybe just adding a small example would be enough, for example:

e.g. write Vararg{T, N} instead of Vararg{<:T, N}.

@Keno
Copy link
Member

Keno commented Apr 28, 2023

Vararg{<:Integer,N} is syntax for Vararg{T,N} where T<:Integer, which is no longer allowed, you need to write Tuple{Vararg{T,N}} where T<:Integer.

@Keno
Copy link
Member

Keno commented Apr 28, 2023

(In a method signature the Tuple is implicit).

@timholy
Copy link
Member Author

timholy commented Apr 28, 2023

Isn't it:

  • f(::Tuple{Vararg{Any}}) matches f((1,)) but not f(1)
  • f(::Vararg{Any}) matches f(1) or f((1,))

So wrapping in Tuple is not neutral with respect to dispatch.

@Keno
Copy link
Member

Keno commented Apr 28, 2023

The first one's signature is Tuple{typeof(f), Tuple{Vararg{Any}}}.
The second one is Tuple{typeof(f), Vararg{Any}}. The point is that you're not allowed to write
Tuple{typeof(f), Vararg{T} where T} anymore, you have to write Tuple{typeof(f), Vararg{T}} where T.
Of course, the error message is a bit confusing, because the Tuple{} of the signature is hidden behind
a syntax rewrite, but it's hard to give a much better error, because by the time we can detect this, the
difference is no longer there.

@timholy
Copy link
Member Author

timholy commented Apr 28, 2023

I see the point of confusion: I was thinking of just the arg, you were using the whole signature tuple-type.

While we may not be able to give a definitive prescription, there don't seem to be that many options. Would it be acceptable to add a second line,

You may need to write `f(x::Vararg{T})` rather than `f(x::Vararg{<:T})` or `f(x::Vararg{T}) where T` instead of `f(x::Vararg{T} where T)`

@Keno
Copy link
Member

Keno commented Apr 28, 2023

Sure

timholy added a commit that referenced this issue Apr 28, 2023
VEZY added a commit to VirtualPlantLab/PlantSimEngine.jl that referenced this issue Nov 13, 2023
Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead). See: JuliaLang/julia#49553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants