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

Slurped kwargs with a type are an error in Julia 1.7 #43625

Closed
cstjean opened this issue Jan 1, 2022 · 9 comments
Closed

Slurped kwargs with a type are an error in Julia 1.7 #43625

cstjean opened this issue Jan 1, 2022 · 9 comments

Comments

@cstjean
Copy link
Contributor

cstjean commented Jan 1, 2022

On Julia 1.7.1, slurped kwargs can no longer have a type declaration:

julia> f(args::Any...) = 10
f (generic function with 2 methods)

julia> f(;kwargs::Any...) = 10
ERROR: syntax: "kwargs::Any" is not a valid function argument name around REPL[24]:1
Stacktrace:
 [1] top-level scope
   @ REPL[24]:1

This is a headache for us at FluxML/MacroTools.jl#177. Was that intentional? I wonder if this wasn't an unintended side-effect of #40977

@JeffBezanson
Copy link
Sponsor Member

This was definitely a bug before; for example

julia> f(;kwargs::Any...) = 10;

julia> methods(f)
# 1 method for generic function "f":
[1] f(; var"(:: kwargs Any)"...) in Main at REPL[1]:1

The declaration expression got stringified and used as the name of the argument! So this really ought to be an error.

@simeonschaub
Copy link
Member

Could we not just support this properly though?

@cstjean
Copy link
Contributor Author

cstjean commented Jan 3, 2022

This was definitely a bug before; for example

Looks like it was only a method printing bug, because it worked fine nevertheless...

julia> f(;kwargs::Any...) = kwargs;

julia> f(; x=1)
pairs(::NamedTuple) with 1 entry:
  :x => 1

julia> methods(f)
# 1 method for generic function "f":
[1] f(; (:: kwargs Any)...) in Main at REPL[5]:1

On Julia 1.5

@JeffBezanson
Copy link
Sponsor Member

For reference, caused by #39593. But it looks like this was a side effect and not an explicitly intended change there.

Could we not just support this properly though?

I think that would be a bit odd and not very useful... how do you imagine it working?

It would be easy enough to put in a shim to ignore the type declaration. It's still a bug though; we used to let you put any type whatsoever there and it had no effect. Is it really that difficult to fix the macro?

@cstjean
Copy link
Contributor Author

cstjean commented Jan 4, 2022

Is it really that difficult to fix the macro?

We have to either make a breaking MacroTools release, or minimally add a (is_slurp && type==:Any) ? ... : ... special-case. It's not very satisfying, but I can see your point. Closing as wont-fix...

@cstjean cstjean closed this as completed Jan 4, 2022
@willow-ahrens
Copy link
Contributor

We've fixed the macro, so this is just for the record.

If we're looking for a behavior for f(; kwargs::T...), consider:
f(arg::T) dispatches when arg is T
f(args::T...) dispatches when all of args are T
f(;kwarg::T=val) errors if kwarg is not T
f(;kwargs::T...) should error if all of kwargs are not T, right?

I feel this is a consistent behavior that would simplify expression manipulation.

@cstjean
Copy link
Contributor Author

cstjean commented Jan 4, 2022

Well, this is really a discussion for julialang, right? Personally, I can't think of a situation where f(;kwargs::Int...) would really be something I want to express, but to the extent that it's allowed, I agree that your proposal makes the most sense...

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 7, 2022

It is slightly uncertain right now if the ::T would apply to the kwargs variable, or the values of the kwargs, or the values (pairs) you get when iterating the kwargs. Formerly it meant the former meaning, which deviated from what it means for varargs. I agree we should likely bring something back for it, but the first step of that was to remove the old, broken version.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 7, 2022

And that meaning is further complicated by invariance, since it is currently ambiguous (for Tuples) which subtyping definition is being used. So (to agree with the behavior of Tuple), it is potentially the case that ::T... should convert the type of the container of the values to hold objects of type T.

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

No branches or pull requests

5 participants