Merge pull request #66029 from hamishknight/you-shall-not-parse

Author: hamishknight

include/swift/AST/ParseRequests.h

@@ -116,6 +116,24 @@ class ParseSourceFileRequest
   readDependencySource(const evaluator::DependencyRecorder &) const;
 };
 
+/// Parse the ExportedSourceFile for a given SourceFile.
+class ExportedSourceFileRequest
+    : public SimpleRequest<ExportedSourceFileRequest,
+                           void *(const SourceFile *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  void *evaluate(Evaluator &evaluator, const SourceFile *SF) const;
+
+public:
+  // Cached.
+  bool isCached() const { return true; }
+};
+
 /// Parse the top-level items of a SourceFile.
 class ParseTopLevelDeclsRequest
     : public SimpleRequest<

include/swift/AST/ParseTypeIDZone.def

@@ -28,3 +28,6 @@ SWIFT_REQUEST(Parse, ParseSourceFileRequest,
 SWIFT_REQUEST(Parse, ParseTopLevelDeclsRequest,
               ArrayRef<Decl *>(SourceFile *), Cached,
               NoLocationInfo)
+SWIFT_REQUEST(Parse, ExportedSourceFileRequest,
+              void *(const SourceFile *), Cached,
+              NoLocationInfo)

include/swift/AST/SourceFile.h

@@ -91,9 +91,11 @@ class SourceFile final : public FileUnit {
     /// files, as they get parsed multiple times.
     SuppressWarnings = 1 << 4,
 
-    /// Whether to disable the Swift Parser ASTGen
-    /// e.g. in dependency scanning, where an AST is not needed.
-    DisableSwiftParserASTGen = 1 << 5,
+    /// Ensure that the SwiftSyntax tree round trips correctly.
+    RoundTrip = 1 << 5,
+
+    /// Validate the new SwiftSyntax parser diagnostics.
+    ValidateNewParserDiagnostics = 1 << 6,
   };
   using ParsingOptions = OptionSet<ParsingFlags>;
 
@@ -262,6 +264,10 @@ class SourceFile final : public FileUnit {
   /// code for it. Note this method returns \c false in WMO.
   bool isPrimary() const { return IsPrimary; }
 
+  /// Retrieve the \c ExportedSourceFile instance produced by ASTGen, which
+  /// includes the SourceFileSyntax node corresponding to this source file.
+  void *getExportedSourceFile() const;
+
   /// The list of local type declarations in the source file.
   llvm::SetVector<TypeDecl *> LocalTypeDecls;
 
@@ -334,10 +340,6 @@ class SourceFile final : public FileUnit {
   /// this source file.
   llvm::SmallVector<Located<StringRef>, 0> VirtualFilePaths;
 
-  /// The \c ExportedSourceFile instance produced by ASTGen, which includes
-  /// the SourceFileSyntax node corresponding to this source file.
-  void *exportedSourceFile = nullptr;
-
   /// Returns information about the file paths used for diagnostics and magic
   /// identifiers in this source file, including virtual filenames introduced by
   /// \c #sourceLocation(file:) declarations.

lib/AST/Module.cpp

@@ -3687,6 +3687,10 @@ SourceFile::getDefaultParsingOptions(const LangOptions &langOpts) {
     opts |= ParsingFlags::DisablePoundIfEvaluation;
   if (langOpts.CollectParsedToken)
     opts |= ParsingFlags::CollectParsedTokens;
+  if (langOpts.hasFeature(Feature::ParserRoundTrip))
+    opts |= ParsingFlags::RoundTrip;
+  if (langOpts.hasFeature(Feature::ParserValidation))
+    opts |= ParsingFlags::ValidateNewParserDiagnostics;
   return opts;
 }
 
@@ -3762,6 +3766,11 @@ ArrayRef<Decl *> SourceFile::getHoistedDecls() const {
   return Hoisted;
 }
 
+void *SourceFile::getExportedSourceFile() const {
+  auto &eval = getASTContext().evaluator;
+  return evaluateOrDefault(eval, ExportedSourceFileRequest{this}, nullptr);
+}
+
 void SourceFile::addDeclWithRuntimeDiscoverableAttrs(ValueDecl *decl) {
   assert(!decl->getRuntimeDiscoverableAttrs().empty());
   DeclsWithRuntimeDiscoverableAttrs.insert(decl);

lib/Basic/Statistic.cpp

@@ -419,11 +419,14 @@ UnifiedStatsReporter::noteCurrentProcessExitStatus(int status) {
 
 void
 UnifiedStatsReporter::publishAlwaysOnStatsToLLVM() {
+  // NOTE: We do `Stat = 0` below to force LLVM to register the statistic,
+  // ensuring we print counters, even if 0.
   if (FrontendCounters) {
     auto &C = getFrontendCounters();
 #define FRONTEND_STATISTIC(TY, NAME)                            \
     do {                                                        \
       static Statistic Stat = {#TY, #NAME, #NAME};              \
+      Stat = 0;                                                 \
       Stat += (C).NAME;                                         \
     } while (0);
 #include "swift/Basic/Statistics.def"
@@ -434,6 +437,7 @@ UnifiedStatsReporter::publishAlwaysOnStatsToLLVM() {
 #define DRIVER_STATISTIC(NAME)                                       \
     do {                                                             \
       static Statistic Stat = {"Driver", #NAME, #NAME};              \
+      Stat = 0;                                                      \
       Stat += (C).NAME;                                              \
     } while (0);
 #include "swift/Basic/Statistics.def"

lib/Frontend/Frontend.cpp

@@ -1513,6 +1513,9 @@ void CompilerInstance::finishTypeChecking() {
 
 SourceFile::ParsingOptions
 CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const {
+  using ActionType = FrontendOptions::ActionType;
+  using ParsingFlags = SourceFile::ParsingFlags;
+
   const auto &frontendOpts = Invocation.getFrontendOptions();
   const auto action = frontendOpts.RequestedAction;
 
@@ -1521,42 +1524,45 @@ CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const {
     // Generally in a parse-only invocation, we want to disable #if evaluation.
     // However, there are a couple of modes where we need to know which clauses
     // are active.
-    if (action != FrontendOptions::ActionType::EmitImportedModules &&
-        action != FrontendOptions::ActionType::ScanDependencies) {
-      opts |= SourceFile::ParsingFlags::DisablePoundIfEvaluation;
+    if (action != ActionType::EmitImportedModules &&
+        action != ActionType::ScanDependencies) {
+      opts |= ParsingFlags::DisablePoundIfEvaluation;
     }
 
     // If we need to dump the parse tree, disable delayed bodies as we want to
     // show everything.
-    if (action == FrontendOptions::ActionType::DumpParse)
-      opts |= SourceFile::ParsingFlags::DisableDelayedBodies;
+    if (action == ActionType::DumpParse)
+      opts |= ParsingFlags::DisableDelayedBodies;
   }
 
-  auto typeOpts = getASTContext().TypeCheckerOpts;
-  if (forPrimary || isWholeModuleCompilation()) {
+  const auto &typeOpts = getASTContext().TypeCheckerOpts;
+  const auto isEffectivelyPrimary = forPrimary || isWholeModuleCompilation();
+  if (isEffectivelyPrimary) {
     // Disable delayed body parsing for primaries and in WMO, unless
     // forcefully skipping function bodies
     if (typeOpts.SkipFunctionBodies == FunctionBodySkipping::None)
-      opts |= SourceFile::ParsingFlags::DisableDelayedBodies;
+      opts |= ParsingFlags::DisableDelayedBodies;
   } else {
     // Suppress parse warnings for non-primaries, as they'll get parsed multiple
     // times.
-    opts |= SourceFile::ParsingFlags::SuppressWarnings;
+    opts |= ParsingFlags::SuppressWarnings;
   }
 
-  // Dependency scanning does not require an AST, so disable Swift Parser
-  // ASTGen parsing completely.
-  if (frontendOpts.RequestedAction ==
-      FrontendOptions::ActionType::ScanDependencies)
-    opts |= SourceFile::ParsingFlags::DisableSwiftParserASTGen;
+  // Turn off round-trip checking for secondary files, and for dependency
+  // scanning and IDE inspection.
+  if (!isEffectivelyPrimary || SourceMgr.hasIDEInspectionTargetBuffer() ||
+      frontendOpts.RequestedAction == ActionType::ScanDependencies) {
+    opts -= ParsingFlags::RoundTrip;
+    opts -= ParsingFlags::ValidateNewParserDiagnostics;
+  }
 
   // Enable interface hash computation for primaries or emit-module-separately,
   // but not in WMO, as it's only currently needed for incremental mode.
   if (forPrimary ||
       typeOpts.SkipFunctionBodies ==
           FunctionBodySkipping::NonInlinableWithoutTypes ||
       frontendOpts.ReuseFrontendForMultipleCompilations) {
-    opts |= SourceFile::ParsingFlags::EnableInterfaceHash;
+    opts |= ParsingFlags::EnableInterfaceHash;
   }
   return opts;
 }

lib/Parse/ParseDecl.cpp

@@ -209,9 +209,7 @@ extern "C" void swift_ASTGen_buildTopLevelASTNodes(void *sourceFile,
 void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
 #if SWIFT_SWIFT_PARSER
   Optional<DiagnosticTransaction> existingParsingTransaction;
-  if (!SF.getParsingOptions()
-      .contains(SourceFile::ParsingFlags::DisableSwiftParserASTGen))
-    parseSourceFileViaASTGen(items, existingParsingTransaction);
+  parseSourceFileViaASTGen(items, existingParsingTransaction);
 #endif
 
   // Prime the lexer.
@@ -261,92 +259,135 @@ void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
   }
 
 #if SWIFT_SWIFT_PARSER
-  if (!SF.getParsingOptions().contains(
-          SourceFile::ParsingFlags::DisableSwiftParserASTGen)) {
-    if (existingParsingTransaction)
-      existingParsingTransaction->abort();
-
-    // Perform round-trip and/or validation checking.
-    if ((Context.LangOpts.hasFeature(Feature::ParserRoundTrip) ||
-         Context.LangOpts.hasFeature(Feature::ParserValidation)) &&
-        SF.exportedSourceFile &&
-        !SourceMgr.hasIDEInspectionTargetBuffer()) {
-      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,
-                     /*emitOnlyErrors=*/true,
-                     /*downgradePlaceholderErrorsToWarnings=*/
-                     Context.LangOpts.Playground ||
-                         Context.LangOpts.WarnOnEditorPlaceholder)) {
-        // We might have emitted warnings in the C++ parser but no errors, in
-        // which case we still have `hadAnyError() == false`. To avoid emitting
-        // the same warnings from SwiftParser, only emit errors from SwiftParser
-        SourceLoc loc;
-        if (auto bufferID = SF.getBufferID()) {
+  if (existingParsingTransaction)
+    existingParsingTransaction->abort();
+
+  using ParsingFlags = SourceFile::ParsingFlags;
+  const auto parsingOpts = SF.getParsingOptions();
+
+  // If we don't need to validate anything, we're done.
+  if (!parsingOpts.contains(ParsingFlags::RoundTrip) &&
+      !parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics)) {
+    return;
+  }
+
+  auto *exportedSourceFile = SF.getExportedSourceFile();
+  if (!exportedSourceFile)
+    return;
+
+  // Perform round-trip and/or validation checking.
+  if (parsingOpts.contains(ParsingFlags::RoundTrip) &&
+      swift_ASTGen_roundTripCheck(exportedSourceFile)) {
+    SourceLoc loc;
+    if (auto bufferID = SF.getBufferID()) {
+      loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
+    }
+    diagnose(loc, diag::parser_round_trip_error);
+    return;
+  }
+  if (parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics) &&
+      !Context.Diags.hadAnyError()) {
+    auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics(
+        &Context.Diags, exportedSourceFile,
+        /*emitOnlyErrors=*/true,
+        /*downgradePlaceholderErrorsToWarnings=*/
+        Context.LangOpts.Playground ||
+            Context.LangOpts.WarnOnEditorPlaceholder);
+    if (hadSyntaxError) {
+      // We might have emitted warnings in the C++ parser but no errors, in
+      // which case we still have `hadAnyError() == false`. To avoid
+      // emitting the same warnings from SwiftParser, only emit errors from
+      // SwiftParser
+      SourceLoc loc;
+      if (auto bufferID = SF.getBufferID()) {
           loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
-        }
-        diagnose(loc, diag::parser_new_parser_errors);
       }
+      diagnose(loc, diag::parser_new_parser_errors);
     }
   }
 #endif
 }
 
+void *ExportedSourceFileRequest::evaluate(Evaluator &evaluator,
+                                          const SourceFile *SF) const {
+#if SWIFT_SWIFT_PARSER
+  // The SwiftSyntax parser doesn't (yet?) handle SIL.
+  if (SF->Kind == SourceFileKind::SIL)
+    return nullptr;
+
+  auto &ctx = SF->getASTContext();
+  auto &SM = ctx.SourceMgr;
+
+  auto bufferID = SF->getBufferID();
+  if (!bufferID)
+    return nullptr;
+
+  StringRef contents = SM.extractText(SM.getRangeForBuffer(*bufferID));
+
+  // Parse the source file.
+  auto exportedSourceFile = swift_ASTGen_parseSourceFile(
+      contents.begin(), contents.size(),
+      SF->getParentModule()->getName().str().str().c_str(),
+      SF->getFilename().str().c_str());
+
+  ctx.addCleanup([exportedSourceFile] {
+    swift_ASTGen_destroySourceFile(exportedSourceFile);
+  });
+  return exportedSourceFile;
+#else
+  return nullptr;
+#endif
+}
+
 void
 Parser::parseSourceFileViaASTGen(SmallVectorImpl<ASTNode> &items,
                                  Optional<DiagnosticTransaction> &transaction,
                                  bool suppressDiagnostics) {
 #if SWIFT_SWIFT_PARSER
-  Optional<DiagnosticTransaction> existingParsingTransaction;
-  if (SF.Kind != SourceFileKind::SIL) {
-    StringRef contents =
-        SourceMgr.extractText(SourceMgr.getRangeForBuffer(L->getBufferID()));
-
-    // Parse the source file.
-    auto exportedSourceFile = swift_ASTGen_parseSourceFile(
-        contents.begin(), contents.size(),
-        SF.getParentModule()->getName().str().str().c_str(),
-        SF.getFilename().str().c_str());
-    SF.exportedSourceFile = exportedSourceFile;
-    Context.addCleanup([exportedSourceFile] {
-      swift_ASTGen_destroySourceFile(exportedSourceFile);
-    });
+  using ParsingFlags = SourceFile::ParsingFlags;
+  const auto parsingOpts = SF.getParsingOptions();
+  const auto &langOpts = Context.LangOpts;
+
+  // We only need to do parsing if we either have ASTGen enabled, or want the
+  // new parser diagnostics.
+  auto needToParse = [&]() {
+    if (langOpts.hasFeature(Feature::ParserASTGen))
+      return true;
+    if (!suppressDiagnostics &&
+        langOpts.hasFeature(Feature::ParserDiagnostics)) {
+      return true;
+    }
+    return false;
+  }();
+  if (!needToParse)
+    return;
 
-    // If we're supposed to emit diagnostics from the parser, do so now.
-    if ((Context.LangOpts.hasFeature(Feature::ParserDiagnostics) ||
-         Context.LangOpts.hasFeature(Feature::ParserASTGen)) &&
-        !suppressDiagnostics &&
-        swift_ASTGen_emitParserDiagnostics(
-            &Context.Diags, SF.exportedSourceFile, /*emitOnlyErrors=*/false,
-            /*downgradePlaceholderErrorsToWarnings=*/
-                Context.LangOpts.Playground ||
-                Context.LangOpts.WarnOnEditorPlaceholder) &&
-        Context.Diags.hadAnyError() &&
-        !Context.LangOpts.hasFeature(Feature::ParserASTGen)) {
+  auto *exportedSourceFile = SF.getExportedSourceFile();
+  if (!exportedSourceFile)
+    return;
+
+  // If we're supposed to emit diagnostics from the parser, do so now.
+  if (!suppressDiagnostics) {
+    auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics(
+        &Context.Diags, exportedSourceFile, /*emitOnlyErrors=*/false,
+        /*downgradePlaceholderErrorsToWarnings=*/langOpts.Playground ||
+            langOpts.WarnOnEditorPlaceholder);
+    if (hadSyntaxError && Context.Diags.hadAnyError() &&
+        !langOpts.hasFeature(Feature::ParserASTGen)) {
       // Errors were emitted, and we're still using the C++ parser, so
       // disable diagnostics from the C++ parser.
       transaction.emplace(Context.Diags);
     }
+  }
 
-    // If we want to do ASTGen, do so now.
-    if (Context.LangOpts.hasFeature(Feature::ParserASTGen)) {
-      swift_ASTGen_buildTopLevelASTNodes(
-          exportedSourceFile, CurDeclContext, &Context, &items, appendToVector);
-
-      // Spin the C++ parser to the end; we won't be using it.
-      while (!Tok.is(tok::eof)) {
-        consumeToken();
-      }
+  // If we want to do ASTGen, do so now.
+  if (langOpts.hasFeature(Feature::ParserASTGen)) {
+    swift_ASTGen_buildTopLevelASTNodes(exportedSourceFile, CurDeclContext,
+                                       &Context, &items, appendToVector);
 
-      return;
+    // Spin the C++ parser to the end; we won't be using it.
+    while (!Tok.is(tok::eof)) {
+      consumeToken();
     }
   }
 #endif