Improve recovery of dollar identifiers #998
Conversation
ahoppen
force-pushed
the
ahoppen/dollar-recovery
branch
from
9abddc8 to
a0b5580
Compare
|
@swift-ci Please test |
| let unexpectedBeforeLabel: RawUnexpectedNodesSyntax? | ||
| let label: RawTokenSyntax? | ||
| let colon: RawTokenSyntax? | ||
| if let labelAndColon = self.consume(if: { $0.canBeArgumentLabel() }, followedBy: { $0.tokenKind == .colon }) { |
There was a problem hiding this comment.
Seems like it would be cleaner to just do
if currentToken.canBeArgumentLabel(allowDollarIdentifier: true) && peek().tokenKind == .colon {
(unexpectedBeforeLabel, label) = parseArgumentLabel()
colon = consumeAnyToken()
}
Or maybe not 🤷.
| @@ -459,8 +465,9 @@ extension Parser { | |||
| keepGoing = trailingComma != nil | |||
| elements.append(RawTupleTypeElementSyntax( | |||
| inOut: nil, | |||
| RawUnexpectedNodesSyntax(misplacedSpecifiers, arena: self.arena), | |||
| RawUnexpectedNodesSyntax(misplacedSpecifiers.map(RawSyntax.init) + (unexpectedBeforeFirst?.elements ?? []), arena: self.arena), | |||
There was a problem hiding this comment.
There's a bunch of places we're doing things like this now. Couldn't we just return [RawSyntax] any calls that return unexpected + make all the unexpected parameters [RawSyntax] instead? That would both avoid creating unexpected nodes that aren't removed + avoid looking into them to grab elements.
There was a problem hiding this comment.
I don’t think I like the idea of changing the parameters of all raw nodes from RawUnexpectedNodesSyntax to RawSyntax (incidentally that would also prevent us from using all the convenience initializer on RawUnexpectedNodesSyntax we currently have (e.g. the one taking [RawSyntax?]). I put together #1005. I’m not entirely sure whether I like the change yet because it does add as bit of complexity on the declaration side, but the call side is rather nice. WDYT?
There was a problem hiding this comment.
I've commented in that PR. Personally I'd find having all [RawSyntax] more convenient than the [RawSyntax?] convenience initializer. Though I haven't actually checked how many places that's being used, so maybe that wouldn't be the case. Anyway, that PR is better than the status quo so I'd be fine with merging that in.
I believe that the remaining diagnostics need to be diagnosed in the lexer.