Merge pull request #62629 from DougGregor/new-parser-diagnostics

Experimentally emit diagnostics from the new Swift parser

Author: swift-ci

include/swift/AST/DiagnosticsParse.def

@@ -1964,12 +1964,6 @@ ERROR(attr_requires_concurrency, none,
 ERROR(associatedtype_cannot_be_variadic,none,
       "associated types cannot be variadic", ())
 
-//------------------------------------------------------------------------------
-// MARK: Consistency diagnostics
-//------------------------------------------------------------------------------
-ERROR(new_parser_failure, none,
-      "new parser has failed consistency checking; please see errors above", ())
-
 //------------------------------------------------------------------------------
 // MARK: syntax parsing diagnostics
 //------------------------------------------------------------------------------
@@ -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",
+       ())
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/Basic/Features.def

@@ -138,9 +138,10 @@ EXPERIMENTAL_FEATURE(ParserRoundTrip, false)
 /// Swift parser.
 EXPERIMENTAL_FEATURE(ParserValidation, false)
 
-/// Whether to fold sequence expressions in the syntax tree produced by the
-/// Swift Swift parser.
-EXPERIMENTAL_FEATURE(ParserSequenceFolding, false)
+/// Whether to emit diagnostics from the new parser first, and only emit
+/// diagnostics from the existing parser when there are none from the new
+/// parser.
+EXPERIMENTAL_FEATURE(ParserDiagnostics, false)
 
 /// Enables implicit some while also enabling existential `any`
 EXPERIMENTAL_FEATURE(ImplicitSome, false)

lib/AST/ASTPrinter.cpp

@@ -2974,7 +2974,7 @@ static bool usesFeatureParserValidation(Decl *decl) {
   return false;
 }
 
-static bool usesFeatureParserSequenceFolding(Decl *decl) {
+static bool usesFeatureParserDiagnostics(Decl *decl) {
   return false;
 }
 

lib/ASTGen/CMakeLists.txt

@@ -76,7 +76,6 @@ if (SWIFT_SWIFT_PARSER)
     SwiftOperators
     SwiftSyntaxBuilder
     _SwiftSyntaxMacros
-    SwiftCompilerSupport
   )
 
   # Compute the list of SwiftSyntax targets

lib/ASTGen/Sources/ASTGen/Diagnostics.swift

@@ -91,32 +91,34 @@ func emitDiagnostic(
   diagnostic: Diagnostic,
   messageSuffix: String? = nil
 ) {
-  // Collect all of the Fix-It changes based on their Fix-It ID.
-  var fixItChangesByID: [MessageID : [FixIt.Change]] = [:]
-  for fixIt in diagnostic.fixIts {
-    fixItChangesByID[fixIt.message.fixItID, default: []]
-      .append(contentsOf: fixIt.changes.changes)
-  }
-
   // Emit the main diagnostic
   emitDiagnosticParts(
     diagEnginePtr: diagEnginePtr,
     sourceFileBuffer: sourceFileBuffer,
     message: diagnostic.diagMessage.message + (messageSuffix ?? ""),
     severity: diagnostic.diagMessage.severity,
     position: diagnostic.position,
-    highlights: diagnostic.highlights,
-    fixItChanges: fixItChangesByID[diagnostic.diagnosticID] ?? []
+    highlights: diagnostic.highlights
   )
 
+  // Emit Fix-Its.
+  for fixIt in diagnostic.fixIts {
+    emitDiagnosticParts(
+        diagEnginePtr: diagEnginePtr,
+        sourceFileBuffer: sourceFileBuffer,
+        message: fixIt.message.message,
+        severity: .note, position: diagnostic.position,
+        fixItChanges: fixIt.changes.changes
+    )
+  }
+
   // Emit any notes as follow-ons.
   for note in diagnostic.notes {
     emitDiagnosticParts(
       diagEnginePtr: diagEnginePtr,
       sourceFileBuffer: sourceFileBuffer,
       message: note.message,
-      severity: .note, position: note.position,
-      fixItChanges: fixItChangesByID[note.noteMessage.fixItID] ?? []
+      severity: .note, position: note.position
     )
   }
 }

lib/ASTGen/Sources/ASTGen/SourceFile.swift

@@ -1,5 +1,6 @@
 import SwiftParser
 import SwiftSyntax
+import SwiftParserDiagnostics
 
 /// Describes a source file that has been "exported" to the C++ part of the
 /// compiler, with enough information to interface with the C++ layer.
@@ -59,3 +60,49 @@ public func roundTripCheck(
     return sf.syntax.syntaxTextBytes.elementsEqual(sf.buffer) ? 0 : 1
   }
 }
+
+extension Syntax {
+  /// Whether this syntax node is or is enclosed within a #if.
+  fileprivate var isInIfConfig: Bool {
+    if self.is(IfConfigDeclSyntax.self) {
+      return true
+    }
+
+    return parent?.isInIfConfig ?? false
+  }
+}
+
+/// Emit diagnostics within the given source file.
+@_cdecl("swift_ASTGen_emitParserDiagnostics")
+public func emitParserDiagnostics(
+  diagEnginePtr: UnsafeMutablePointer<UInt8>,
+  sourceFilePtr: UnsafeMutablePointer<UInt8>
+) -> CInt {
+  return sourceFilePtr.withMemoryRebound(
+    to: ExportedSourceFile.self, capacity: 1
+  ) { sourceFile in
+    var anyDiags = false
+
+    let diags = ParseDiagnosticsGenerator.diagnostics(
+      for: sourceFile.pointee.syntax
+    )
+    for diag in diags {
+      // Skip over diagnostics within #if, because we don't know whether
+      // we are in an active region or not.
+      // FIXME: This heuristic could be improved.
+      if diag.node.isInIfConfig {
+        continue
+      }
+
+      emitDiagnostic(
+        diagEnginePtr: diagEnginePtr,
+        sourceFileBuffer: UnsafeMutableBufferPointer(
+          mutating: sourceFile.pointee.buffer),
+        diagnostic: diag
+      )
+      anyDiags = true
+    }
+
+    return anyDiags ? 1 : 0
+  }
+}

lib/Parse/CMakeLists.txt

@@ -33,7 +33,7 @@ if (SWIFT_SWIFT_PARSER)
   #
   #   target_link_libraries(swiftParse
   #     PRIVATE
-  #     SwiftSyntax::SwiftCompilerSupport
+  #     SwiftSyntax::SwiftParser
   #     ...
   #   )
   target_link_libraries(swiftParse
@@ -46,7 +46,6 @@ if (SWIFT_SWIFT_PARSER)
     SwiftSyntax::SwiftOperators
     SwiftSyntax::SwiftSyntaxBuilder
     SwiftSyntax::_SwiftSyntaxMacros
-    SwiftSyntax::SwiftCompilerSupport
     $<TARGET_OBJECTS:swiftASTGen>
   )
 
@@ -59,15 +58,9 @@ if (SWIFT_SWIFT_PARSER)
     SwiftSyntax::SwiftOperators
     SwiftSyntax::SwiftSyntaxBuilder
     SwiftSyntax::_SwiftSyntaxMacros
-    SwiftSyntax::SwiftCompilerSupport
     swiftASTGen
   )
 
-  target_include_directories(swiftParse
-    PRIVATE
-    ${CMAKE_CURRENT_SOURCE_DIR}/../../../swift-syntax/Sources/SwiftCompilerSupport
-  )
-
   target_compile_definitions(swiftParse
     PRIVATE
     SWIFT_SWIFT_PARSER

lib/Parse/ParseDecl.cpp

@@ -182,6 +182,11 @@ extern "C" void swift_ASTGen_destroySourceFile(void *sourceFile);
 /// round-trip succeeded, non-zero otherwise.
 extern "C" int swift_ASTGen_roundTripCheck(void *sourceFile);
 
+/// Emit parser diagnostics for given source file.. Returns non-zero if any
+/// diagnostics were emitted.
+extern "C" int swift_ASTGen_emitParserDiagnostics(
+    void *diagEngine, void *sourceFile
+);
 
 // Build AST nodes for the top-level entities in the syntax.
 extern "C" void swift_ASTGen_buildTopLevelASTNodes(void *sourceFile,
@@ -200,9 +205,12 @@ extern "C" void swift_ASTGen_buildTopLevelASTNodes(void *sourceFile,
 /// \endverbatim
 void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
 #if SWIFT_SWIFT_PARSER
+  Optional<DiagnosticTransaction> existingParsingTransaction;
   if ((Context.LangOpts.hasFeature(Feature::Macros) ||
        Context.LangOpts.hasFeature(Feature::BuiltinMacros) ||
        Context.LangOpts.hasFeature(Feature::ParserRoundTrip) ||
+       Context.LangOpts.hasFeature(Feature::ParserDiagnostics) ||
+       Context.LangOpts.hasFeature(Feature::ParserValidation) ||
        Context.LangOpts.hasFeature(Feature::ParserASTGen)) &&
       !SourceMgr.hasIDEInspectionTargetBuffer() &&
       SF.Kind != SourceFileKind::SIL) {
@@ -219,6 +227,18 @@ void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
       swift_ASTGen_destroySourceFile(exportedSourceFile);
     });
 
+    // If we're supposed to emit diagnostics from the parser, do so now.
+    if ((Context.LangOpts.hasFeature(Feature::ParserDiagnostics) ||
+         Context.LangOpts.hasFeature(Feature::ParserASTGen)) &&
+        swift_ASTGen_emitParserDiagnostics(
+            &Context.Diags, SF.exportedSourceFile) &&
+        Context.Diags.hadAnyError() &&
+        !Context.LangOpts.hasFeature(Feature::ParserASTGen)) {
+      // Errors were emitted, and we're still using the C++ parser, so
+      // disable diagnostics from the C++ parser.
+      existingParsingTransaction.emplace(Context.Diags);
+    }
+
     // If we want to do ASTGen, do so now.
     if (Context.LangOpts.hasFeature(Feature::ParserASTGen)) {
       swift_ASTGen_buildTopLevelASTNodes(
@@ -281,16 +301,31 @@ void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
   }
 
 #if SWIFT_SWIFT_PARSER
-  // Perform round-trip checking.
-  if (Context.LangOpts.hasFeature(Feature::ParserRoundTrip) &&
+  if (existingParsingTransaction)
+    existingParsingTransaction->abort();
+
+  // Perform round-trip and/or validation checking.
+  if ((Context.LangOpts.hasFeature(Feature::ParserRoundTrip) ||
+       Context.LangOpts.hasFeature(Feature::ParserValidation)) &&
       SF.exportedSourceFile &&
-      !L->lexingCutOffOffset() &&
-      swift_ASTGen_roundTripCheck(SF.exportedSourceFile)) {
-    SourceLoc loc;
-    if (auto bufferID = SF.getBufferID()) {
-      loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
+      !L->lexingCutOffOffset()) {
+    if (Context.LangOpts.hasFeature(Feature::ParserRoundTrip) &&
+        swift_ASTGen_roundTripCheck(SF.exportedSourceFile)) {
+      SourceLoc loc;
+      if (auto bufferID = SF.getBufferID()) {
+        loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
+      }
+      diagnose(loc, diag::parser_round_trip_error);
+    } else if (Context.LangOpts.hasFeature(Feature::ParserValidation) &&
+               !Context.Diags.hadAnyError() &&
+               swift_ASTGen_emitParserDiagnostics(
+                   &Context.Diags, SF.exportedSourceFile)) {
+      SourceLoc loc;
+      if (auto bufferID = SF.getBufferID()) {
+        loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
+      }
+      diagnose(loc, diag::parser_new_parser_errors);
     }
-    diagnose(loc, diag::parser_round_trip_error);
   }
 #endif
 }

lib/Parse/ParseRequests.cpp

@@ -23,10 +23,6 @@
 #include "swift/Parse/Parser.h"
 #include "swift/Subsystems.h"
 
-#if SWIFT_SWIFT_PARSER
-#include "SwiftCompilerSupport.h"
-#endif
-
 using namespace swift;
 
 namespace swift {
@@ -175,44 +171,6 @@ SourceFileParsingResult ParseSourceFileRequest::evaluate(Evaluator &evaluator,
   if (auto tokens = parser.takeTokenReceiver()->finalize())
     tokensRef = ctx.AllocateCopy(*tokens);
 
-#if SWIFT_SWIFT_PARSER
-  if (ctx.LangOpts.hasFeature(Feature::ParserValidation) &&
-      ctx.SourceMgr.getIDEInspectionTargetBufferID() != bufferID &&
-      SF->Kind != SourceFileKind::SIL) {
-    auto bufferRange = ctx.SourceMgr.getRangeForBuffer(*bufferID);
-    unsigned int flags = 0;
-
-    if (!ctx.Diags.hadAnyError() &&
-        ctx.LangOpts.hasFeature(Feature::ParserValidation))
-      flags |= SCC_ParseDiagnostics;
-
-    if (ctx.LangOpts.hasFeature(Feature::ParserSequenceFolding) &&
-        !parser.L->lexingCutOffOffset())
-      flags |= SCC_FoldSequences;
-
-    if (flags) {
-      SourceLoc startLoc =
-          parser.SourceMgr.getLocForBufferStart(parser.L->getBufferID());
-      struct ParserContext {
-        SourceLoc startLoc;
-        DiagnosticEngine *engine;
-      } context{startLoc, &parser.Diags};
-      int roundTripResult = swift_parser_consistencyCheck(
-          bufferRange.str().data(), bufferRange.getByteLength(),
-          SF->getFilename().str().c_str(), flags, static_cast<void *>(&context),
-          [](ptrdiff_t off, const char *text, void *ctx) {
-            auto *context = static_cast<ParserContext *>(ctx);
-            SourceLoc loc = context->startLoc.getAdvancedLoc(off);
-            context->engine->diagnose(loc, diag::foreign_diagnostic,
-                                      StringRef(text));
-          });
-
-      if (roundTripResult)
-        ctx.Diags.diagnose(SourceLoc(), diag::new_parser_failure);
-    }
-  }
-#endif
-
   return SourceFileParsingResult{ctx.AllocateCopy(items), tokensRef,
                                  parser.CurrentTokenHash};
 }

test/Macros/macro_expand.swift

@@ -57,8 +57,10 @@ struct OnlyAdds {
 func testAddBlocker(a: Int, b: Int, c: Int, oa: OnlyAdds) {
   _ = #addBlocker(a * b * c)
 #if TEST_DIAGNOSTICS
-  _ = #addBlocker(a + b * c) // expected-error{{blocked an add; did you mean to subtract? (from macro 'addBlocker')}}{{21-22=-}}
-  _ = #addBlocker(oa + oa) // expected-error{{blocked an add; did you mean to subtract? (from macro 'addBlocker')}}{{22-23=-}}
-      // expected-note@-1{{in expansion of macro 'addBlocker' here}}
+  _ = #addBlocker(a + b * c) // expected-error{{blocked an add; did you mean to subtract? (from macro 'addBlocker')}}
+  // expected-note@-1{{use '-'}}{{21-22=-}}
+  _ = #addBlocker(oa + oa) // expected-error{{blocked an add; did you mean to subtract? (from macro 'addBlocker')}}
+  // expected-note@-1{{in expansion of macro 'addBlocker' here}}
+  // expected-note@-2{{use '-'}}{{22-23=-}}
 #endif
 }

test/Parse/new_parser_diagnostics.swift

@@ -0,0 +1,9 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature ParserDiagnostics
+
+// FIXME: Swift parser is not enabled on Linux CI yet.
+// REQUIRES: OS=macosx
+// REQUIRES: asserts
+
+_ = [(Int) -> async throws Int]()
+// expected-error@-1{{'async throws' may only occur before '->'}}
+// expected-note@-2{{move 'async throws' in front of '->'}}{{15-21=}} {{21-28=}} {{20-21= }} {{12-12=async }} {{12-12=throws }}