-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
RFC: required keyword arguments #25830
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Looks good to me. Probably should get @JeffBezanson's review.
base/boot.jl
Outdated
@@ -253,6 +254,9 @@ end | |||
struct ArgumentError <: Exception | |||
msg::AbstractString | |||
end | |||
struct UnassignedKeyword <: Exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this end with Error
or Exception
for consistency with most (all) other errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can do that. UnassignedKeywordError
is a bit long. Maybe UndefKeywordError
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KeywordError
? Or KeywordException
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also have UndefVarError
and UndefRefError
, so UndefKeywordError
seems consistent. KeywordError
seems too vague, since there are other ways you could get errors from keyword args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But UndefKeywordError
feels a bit like something that would be thrown for cases like
foo(;a = 1) = a
foo(b=2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider that an unexpected keyword argument error. "undef" is pretty consistently used to mean something that is used as a value without having been given one.
Looks like you'll need --- a/test/syntax.jl
+++ b/test/syntax.jl
@@ -500,10 +500,6 @@ let m_error, error_out, filename = Base.source_path()
error_out = sprint(showerror, m_error)
@test startswith(error_out, "ArgumentError: invalid type for argument number 1 in method definition for method_c6 at $filename:")
- m_error = try @eval method_c6(A; B) = 3; catch e; e; end
- error_out = sprint(showerror, m_error)
- @test error_out == "syntax: keyword argument \"B\" needs a default value"
-
# issue #20614
m_error = try @eval foo(types::NTuple{N}, values::Vararg{Any,N}, c) where {N} = nothing; catch e; e; end
error_out = sprint(showerror, m_error) since that's now a valid method definition. |
Removed the obsolete syntax test; hopefully tests pass now. |
Yay, green CI. Will merge shortly (in a day or so) since it seems everyone approves. |
Wow, CI is green across the board, nice work! |
@ararslan, yes, it is clearly the lack of required keyword arguments that has been the source of all of our CI unreliability. 😉 |
…lue. This syntax has been allowed for quite some time: JuliaLang/julia#25830
This PR implements required keyword arguments (closes #5111):
is sugar for