[Parse] Minor code improvement in function-type parsing #5721
Conversation
rintaro
force-pushed
the
parse-functy
branch
from
741288b to
c3aebfe
Compare
| (Tok.is(tok::kw_throw) && peekToken().is(tok::arrow))) { | ||
| if (Tok.is(tok::kw_throw)) { | ||
| if (Tok.isAny(tok::kw_throws, tok::kw_rethrows, tok::kw_throw) && | ||
| peekToken().is(tok::arrow)) { |
There was a problem hiding this comment.
This is definitely a behavior change for the recovery in a partial line:
let fn: () throwsCan you add tests for this case, and see if it's a net benefit?
There was a problem hiding this comment.
I'm not sure. What you mention is Don't consume 'throws', if next token is not '->'?
Swift 3.0.1 and this PR both emits:
test.swift:1:11: error: consecutive statements on a line must be separated by ';'
let fn: () throws
^
;
test.swift:1:12: error: expected expression
let fn: () throws
^
Actually, this behavior is from the original code:
https://github.com/apple/swift/blob/71364d0d/lib/Parse/ParseType.cpp#L222-L224
rintaro
force-pushed
the
parse-functy
branch
from
c3aebfe to
d356cae
Compare
|
@jrose-apple Anyway, I agree about bad recovery here. Added the test case with |
| "only function declarations may be marked 'rethrows'", ()) | ||
| ERROR(rethrowing_function_type,none, | ||
| "only function declarations may be marked 'rethrows'; " | ||
| "did you mean 'throws'?", ()) |
There was a problem hiding this comment.
This doesn't make sense to me. The fact that rethrows isn't allowed in a function type (which I didn't realize until seeing this PR) seems like an unnecessary and surprising limitation. I would guess users will expect to be able to write rethrows function types, so suggesting did you mean 'throws'? will just be confusing.
There was a problem hiding this comment.
Hmm. Maybe, rejecting rethrows is just a bug.
Function types that can throw an error must be marked with the
throwskeyword, and function types that can rethrow an error must be marked with therethrowskeyword.GRAMMAR OF A FUNCTION TYPE
function-type → attributesopt function-type-argument-clause
throwsopt->type
function-type → attributesopt function-type-argument-clause rethrows->type
There was a problem hiding this comment.
Summoning @rjmccall. A rethrows function type is useful, but it also makes the language more complicated, and I thought we decided not to do it.
rintaro
force-pushed
the
parse-functy
branch
from
d356cae to
81cc86c
Compare
* Code clean up. * Eliminate backtracking.
rintaro
force-pushed
the
parse-functy
branch
from
81cc86c to
2a2da1f
Compare
|
@swift-ci Please smoke test |
|
@rintaro Can you please rebase? |
|
For the record, this was waiting on comments from @rjmccall about |
|
@jrose-apple I pinged John about this but haven't got a response yet. Since this patch is not changing the behavior of anything, can we go ahead and merge it? |
|
Fair point, sure. |
|
That was definitely not a sneaky attempt to add rethrows function types to the language. That would be a major addition. |
|
Sorry for delayed response, I was on vacation. So, should I file a radar about "GRAMMAR OF A FUNCTION TYPE" documentation issue? |
|
Seems like a good idea to me. It's not a critical issue, since the grammar generally allows more productions than the language, but it seems easy enough for the Apple DevPubs folks to just delete that line. |
In addition:
rethrowsin function type.