Experimentally emit diagnostics from the new Swift parser #62629
Conversation
DougGregor
requested review from
ahoppen,
bnbarham,
CodaFi,
rintaro and
zoecarver
as code owners
|
@swift-ci please smoke test |
| // Determine the set of note IDs. | ||
| let knownNoteIDs: Set<MessageID> = .init( | ||
| diagnostic.notes.map { $0.noteMessage.fixItID } | ||
| ) |
There was a problem hiding this comment.
The Swift parser models diagnostics a little differently. Each diagnostic has both notes + fix-it's. A note has no fix-it attached (I think fixItID is actually misnamed here, it's just the note ID). A fix-it has both a message (or "note" if you like :P) and a set of changes.
We can talk about whether that's how we want to model that or not later, but IMO the way these should be emitted right now is that each of the fix-it's just become a note with an attached fix-it.
There was a problem hiding this comment.
This doesn't match the model used by the compiler, and is somewhat lossy. But I'll embrace the model here (see my latest commit) and we can iterate on the swift-syntax diagnostic model more.
| @@ -118,5 +128,10 @@ func emitDiagnostic( | |||
| severity: .note, position: note.position, | |||
| fixItChanges: fixItChangesByID[note.noteMessage.fixItID] ?? [] | |||
There was a problem hiding this comment.
Can drop this - notes never have fix-its.
| @@ -109,6 +117,8 @@ func emitDiagnostic( | |||
| fixItChanges: fixItChangesByID[diagnostic.diagnosticID] ?? [] | |||
There was a problem hiding this comment.
Can drop this - a diagnostic never has a fix-it directly attached (in the current parser model)
| @@ -109,6 +117,8 @@ func emitDiagnostic( | |||
| fixItChanges: fixItChangesByID[diagnostic.diagnosticID] ?? [] | |||
| ) | |||
|
|
|||
| fixItChangesByID.removeValue(forKey: diagnostic.diagnosticID) | |||
|
|
|||
There was a problem hiding this comment.
Then you can just add:
for fixIt in diagnostic.fixIts {
emitDiagnosticParts(
diagEnginePtr: diagEnginePtr,
sourceFileBuffer: sourceFileBuffer,
message: fixIt.message,
severity: .note, position: note.position,
fixItChanges: fixIt.changes.changes)
}
Replace the use of the "consistency check" vended by swift-syntax with an ASTGen-implemented operation that emits diagnostics from the new parser via the normal diagnostic engine. This eliminates our last dependency on SwiftCompilerSupport, so stop linking it.
Introduce the experimental feature `ParserDiagnostics`, which emits diagnostics from the new Swift parser *first* for a source file. If that produces any errors, we suppress any diagnostics emitted from the C++ parser.
Any Fix-Its not associated with a note are automatically associated with the main diagnostic.
The swift-syntax diagnostic system always treats Fix-Its as separate notes, which are never attached to the primary diagnostic. Embrace this module in the mapping over to the existing C++ diagnostic engine.
DougGregor
force-pushed
the
new-parser-diagnostics
branch
from
69f0873 to
4ef35c5
Compare
|
@swift-ci please smoke test and merge |
|
@swift-ci please smoke test |
| @@ -2021,6 +2015,9 @@ ERROR(macro_expansion_decl_expected_macro_identifier,none, | |||
|
|
|||
| ERROR(parser_round_trip_error,none, | |||
| "source file did not round-trip through the Swift Swift parser", ()) | |||
| ERROR(parser_new_parser_errors,none, | |||
| "Swift Swift parser generated errors for code that C++ parser accepted", | |||
There was a problem hiding this comment.
Similar to my comment on another PR, I think Swift Swift parser is confusing. What about SwiftParser or SwiftParser implemented in Swift?
Introduce the experimental feature
ParserDiagnostics, which emitsdiagnostics from the new Swift parser first for a source file. If
that produces any errors, we suppress any diagnostics emitted from the
C++ parser.
While here, move diagnostic validation checking into ASTGen, so we
can drop our dependency on SwiftCompilerSupport... which will go away.