-
Notifications
You must be signed in to change notification settings - Fork 261
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
Improve rules for nameonly/positional/nameless and required/optional … #3864
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.
LGTM
// * required: a caller has to supply an argument | ||
// * optional: the parameter has a default value that is used if a caller omits passing a specific argument | ||
// | ||
// The syntax for giving a positional-only (i.e., nameless) parameter does not a default-value expression, so |
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.
does not a
=> does not allow a
// | ||
// At a call site, positional arguments are not allowed to follow named arguments. Therefore, if "x" is | ||
// a nameonly parameter, then there is no way to supply the parameters after "x" by position. Thus, any | ||
// parameter that follows "x" must either by passed by name or have a default value. That is, if a later |
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.
be passed
// "formal" is preceded by a nameonly parameter, but itself is neither nameonly nor has a default value | ||
reporter.Error(MessageSource.Resolver, formal.tok, | ||
$"this parameter is effectively nameonly (because of the earlier nameonly parameter '{nameOfMostRecentNameonlyParameter}'); " + | ||
"declare it as nameonly or give it an default-value expression"); |
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.
a default-value expression
@@ -275,15 +275,15 @@ module NameOnlyParameters { | |||
least predicate LP(a: int, nameonly b: int, c: int := 100) | |||
static greatest predicate GP(a: int, nameonly b: int := 75, c: int := 100) | |||
} | |||
iterator Iter(nameonly u: int, x: int) | |||
iterator Iter(nameonly u: int, x: int) // error (x2, because of the desugaring of iterators): x is effectively nameonly, but not declared as such |
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.
Maybe we should come up with a testing file format where you can specify the expected error messages inline : )
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 do like being able to read the expected outcome in the test file itself. Therefore, I always put such "error" comments in the tests. But these comments aren't the exact error messages, don't capture column information, and in some cases are even placed only on a nearby line. It's plausible that we could turn these into a formally recognized testing file format.
Such a nice quality of life improvement, thanks ! |
75b30b1
Fixes #3859
This PR allows more parameter declarations (for example, as suggested in #3859, a
nameonly
parameter with a default value can now be follow by othernameonly
parameters), clarifies some error messages, and gives new errors for useless situations (for example, a parameter's default value can not be used if it is followed by a positional-only parameter).The new rules
Formal parameters have three ways to indicate how they are to be passed in:
nameonly
: the only way to give a specific argument value is to name the parameterA parameter is either required or optional:
The syntax for giving a positional-only (i.e., nameless) parameter does not a default-value expression, so a positional-only parameter is always required.
At a call site, positional arguments are not allowed to follow named arguments. Therefore, if
x
is a nameonly parameter, then there is no way to supply the parameters afterx
by position. Thus, any parameter that followsx
must either by passed by name or have a default value. That is, if a later parameter does not have a default value, it is effectively nameonly. We impose the rule thatFor a positional-only parameter
x
, every parameter precedingx
is effectively required. We impose the rule thatBy submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.