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

Add note to Vararg/UnionAll warning about making it an error #56662

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mcabbott
Copy link
Contributor

This warning message

WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
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)`.

(last extended in #49558) seems clear enough if you wrote the code. But if it's coming from 10 packages deep, there's no information to track down the origin.

Turns out you can do this with --depwarn=error. But the message doesn't tell you that, and doesn't call itself a deprecation warning at all.

src/jltypes.c Outdated Show resolved Hide resolved
@nsajko nsajko added the error messages Better, more actionable error messages label Nov 24, 2024
@nsajko
Copy link
Contributor

nsajko commented Nov 24, 2024

I wonder if it'd be possible to add this message to all deprecation warning automatically, instead of hardcoding it here?

@oscardssmith
Copy link
Member

I believe this does happen for most deprecation messages, but this one comes from C so needs to do things itself.

@mcabbott
Copy link
Contributor Author

Right. Until I looked at the code, I had no idea this warning shared anything at all with Base.depwarn -- it makes no claim to be a deprecation.

I don't think the warnings from Base.depwarn tell you how to make them into errors, by default. Maybe they should but not this PR:

julia> Base.depwarn("msg", :fun; force=true)
┌ Warning: msg
│   caller = ip:0x0
└ @ Core :-1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Better, more actionable error messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants