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

lowering: Disallow splatting in non-final default value #50563

Merged
merged 4 commits into from
Jul 18, 2023

Conversation

Keno
Copy link
Member

@Keno Keno commented Jul 15, 2023

Pop quiz: Do you know what the following will do?

julia> function g1(a=(1,2)..., b...=3)
    b
end

julia> g1()

julia> function g2(a=(1,2)..., b=3, c=4)
    (b, c)
end

julia> g2()

julia> function g3(a=(1,2)..., b=3, c...=4)
    (b, c)
end

julia> g3()

julia> g3(1)

I don't either and I don't think it's particularly well defined. Splatting a default argument makes sense on the last argument, which can be a vararg, and it is desirable to be able to specify the default for the whole varargs tuple at once (although arguably that should just be the non-... behavior, but that'd be too breaking a change). Ref #50518. However, for other arguments, there isn't really a sensible semantic meaning. This PR disallows this in lowering. This is technically a minor change, but I doubt anybody is using this. Splatting in default values wasn't really ever supposed to work anyway, it just happened to fall out of our lowering.

Pop quiz: Do you know what the following will do?
```
julia> function g1(a=(1,2)..., b...=3)
    b
end

julia> g1()

julia> function g2(a=(1,2)..., b=3, c=4)
    (b, c)
end

julia> g2()

julia> function g3(a=(1,2)..., b=3, c...=4)
    (b, c)
end

julia> g3()

julia> g3(1)
```

I don't either and I don't think it's particularly well defined.
Splatting a default argument makes sense on the last argument,
which can be a vararg, and it is desirable to be able to specify
the default for the whole varargs tuple at once (although arguably
that should just be the non-`...` behavior, but that'd be too
breaking a change). Ref #50518. However, for other arguments,
there isn't really a sensible semantic meaning. This PR disallows
this in lowering. This is technically a minor change, but I doubt
anybody is using this. Splatting in default values wasn't really
ever supposed to work anyway, it just happened to fall out of
our lowering.
@Keno Keno force-pushed the kf/disallownonfinaldefaultsplat branch from e21aece to b5bb8f3 Compare July 15, 2023 17:54
@ericphanson
Copy link
Contributor

Is the variant from the issue still allowed?

function f(xs...=["a", "b", "c"]...; debug=false)
           return xs[1]
end

Maybe worth a test?

@Keno
Copy link
Member Author

Keno commented Jul 15, 2023

Yes, it's allowed and was added as a test in #50559.

src/julia-syntax.scm Outdated Show resolved Hide resolved
src/julia-syntax.scm Outdated Show resolved Hide resolved
@Keno Keno force-pushed the kf/disallownonfinaldefaultsplat branch from 75c18f9 to 69d7385 Compare July 17, 2023 15:24
@maleadt
Copy link
Member

maleadt commented Jul 18, 2023

This is technically a minor change, but I doubt anybody is using this.

Let's see if Nanosoldier finds any packages:

@nanosoldier runtests()

@nanosoldier
Copy link
Collaborator

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@Keno Keno merged commit b3f766c into master Jul 18, 2023
1 check passed
@Keno Keno deleted the kf/disallownonfinaldefaultsplat branch July 18, 2023 21:40
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 this pull request may close these issues.

5 participants