[SwiftParser] Improve attribute parsing and recovering #3136
Conversation
rintaro
requested review from
ahoppen,
bnbarham and
hamishknight
as code owners
| if self.at(.atSign), case .attribute(let attribute) = self.parseAttribute() { | ||
| if self.at(.atSign) { | ||
| return RawUsingDeclSyntax( | ||
| unexpectedBeforeUsingKeyword, | ||
| usingKeyword: usingKeyword, | ||
| specifier: .attribute(attribute), | ||
| specifier: .attribute(self.parseAttribute()), |
There was a problem hiding this comment.
This .attribute(let attribute) pattern matching was scary 😅 Since we were checking at(.atSign), there was no way it returned non-RawAttributeSyntax, but it looked like we were discarding the parsed nodes unless it's RawAttributeSyntax.
| DiagnosticSpec(message: "extraneous code 'private(' at top level") | ||
| ] | ||
| DiagnosticSpec(message: "expected 'set)' to end modifier", fixIts: ["insert 'set)'"]), | ||
| DiagnosticSpec(message: "expected declaration after 'private' modifier", fixIts: ["insert declaration"]), |
There was a problem hiding this comment.
Now decl modifier without decl introducer is also considered MissingDeclSyntax.
rintaro
force-pushed
the
parser-attr-recover
branch
from
c0c7a8c to
1281394
Compare
Attributes followed by non-attribute `#if` were not `atStartOfDeclaration()` nor `parseStatementItem()`, so the rest of the code including the attribute were parsed as "unexpected code". * Improve attibute list checking in `atStartOfDeclaration()` and parse attributes without decl introducer as `MissingDeclSyntax` * Make sure `#if` contains only attributes before parsing them as an attribute list element * Remove `#if` parsing from `parseAttribute()`, and introduce `parseAttributeListElement()` for attribute list element parsing including `#if`. This prevents a crash when `#if` is passed to `AttributeSyntax.parse(from:)` swiftlang#3134
DeclarationKeyword should use VariableDeclSyntax.BindingSpecifierOptions instead of ValueBindingPatternSyntax one.
rintaro
force-pushed
the
parser-attr-recover
branch
from
1281394 to
2e92078
Compare
|
@swift-ci Please test |
'#if' and '#endif' can be in the same node
rintaro
force-pushed
the
parser-attr-recover
branch
from
2e92078 to
556a2f3
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
| var loopProgress = LoopProgressCondition() | ||
| repeat { | ||
| let attribute = self.parseAttribute() | ||
| while self.hasProgressed(&loopProgress) && shouldContinue() { |
There was a problem hiding this comment.
Nitpick, hasProgressed is always the last check. Probably not so relevant here but there are other parts of the parser where a loop iteration doesn’t consume anything and the check is expected to return false. If we have the hasProgressed check first, we’ll hit an assertion failure where it’s not necessary. So, I think it’s mostly nice to stay consistent about the ordering. Maybe also worth adding this as a comment to hasProgressed.
Attributes followed by non-attribute
#ifwere notatStartOfDeclaration()norparseStatementItem(), so the rest of the code including the attribute were parsed as "unexpected code".atStartOfDeclaration()and parse attributes without decl introducer asMissingDeclSyntax#ifcontains only attributes before parsing them as an attribute list element#ifparsing fromparseAttribute(), and introduceparseAttributeListElement()for attribute list element parsing including#if. This prevents a crash when#ifis passed toAttributeSyntax.parse(from:)#3134