Merge pull request swiftlang#30482 from apple/syntax-incremental-parse-diagnostic

akyrtzi · web-flow · commit 08f70284464e · 2020-03-18T20:17:27.000-07:00
[Parse] Fix issue with incremental re-parsing for SwiftSyntax emitting bogus parsing error
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -296,6 +296,11 @@ namespace swift {
     /// incrementals parsing.
     bool BuildSyntaxTree = false;
 
+    /// Whether parsing is occurring for creation of syntax tree only, and no typechecking will occur after
+    /// parsing e.g. when parsing for SwiftSyntax. This is intended to affect parsing, e.g. disable
+    /// unnecessary name lookups that are not useful for pure syntactic parsing.
+    bool ParseForSyntaxTreeOnly = false;
+
     /// Whether to verify the parsed syntax tree and emit related diagnostics.
     bool VerifySyntaxTree = false;
 
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
@@ -2264,7 +2264,10 @@ Expr *Parser::parseExprIdentifier() {
   }
   
   ValueDecl *D = nullptr;
-  if (!InPoundIfEnvironment) {
+  // When doing incremental re-parsing for SwiftSyntax this check may emit bogus
+  // diagnostic. Also really the syntactic parser should not be doing name
+  // lookups, so disable this check when parsing for SwiftSyntax.
+  if (!InPoundIfEnvironment && !Context.LangOpts.ParseForSyntaxTreeOnly) {
     D = lookupInScope(name);
     // FIXME: We want this to work: "var x = { x() }", but for now it's better
     // to disallow it than to crash.
diff --git a/test/incrParse/incremental-diagnostic.swift b/test/incrParse/incremental-diagnostic.swift
@@ -0,0 +1,7 @@
+// RUN: %empty-directory(%t)
+// RUN: %validate-incrparse %s --test-case REPLACE
+
+let myFont: Font = {
+    let myFont = Font.body
+    return myFont.weight(.<<REPLACE<bold|||light>>>)
+}()
diff --git a/tools/libSwiftSyntaxParser/libSwiftSyntaxParser.cpp b/tools/libSwiftSyntaxParser/libSwiftSyntaxParser.cpp
@@ -277,6 +277,7 @@ swiftparse_client_node_t SynParser::parse(const char *source) {
   TypeCheckerOptions tyckOpts;
   LangOptions langOpts;
   langOpts.BuildSyntaxTree = true;
+  langOpts.ParseForSyntaxTreeOnly = true;
   langOpts.CollectParsedToken = false;
   // Disable name lookups during parsing.
   // Not ready yet:
diff --git a/tools/swift-syntax-test/swift-syntax-test.cpp b/tools/swift-syntax-test/swift-syntax-test.cpp
@@ -175,6 +175,10 @@ static llvm::cl::opt<std::string>
 OutputFilename("output-filename",
                llvm::cl::desc("Path to the output file"));
 
+static llvm::cl::opt<std::string>
+DiagsOutputFilename("diags-output-filename",
+  llvm::cl::desc("Path to the output file for parser diagnostics text"));
+
 static llvm::cl::opt<bool>
 PrintVisualReuseInfo("print-visual-reuse-info",
                      llvm::cl::desc("Print a coloured output of which parts of "
@@ -556,12 +560,17 @@ bool verifyReusedRegions(ByteBasedSourceRangeSet ExpectedReparseRegions,
   return NoUnexpectedParse;
 }
 
+struct ParseInfo {
+  SourceFile *SF;
+  SyntaxParsingCache *SyntaxCache;
+  std::string Diags;
+};
+
 /// Parse the given input file (incrementally if an old syntax tree was
 /// provided) and call the action specific callback with the new syntax tree
 int parseFile(
     const char *MainExecutablePath, const StringRef InputFileName,
-    llvm::function_ref<int(SourceFile *, SyntaxParsingCache *SyntaxCache)>
-        ActionSpecificCallback) {
+    llvm::function_ref<int(ParseInfo)> ActionSpecificCallback) {
   // The cache needs to be a heap allocated pointer since we construct it inside
   // an if block but need to keep it alive until the end of the function.
   SyntaxParsingCache *SyntaxCache = nullptr;
@@ -597,6 +606,7 @@ int parseFile(
   // Set up the compiler invocation
   CompilerInvocation Invocation;
   Invocation.getLangOptions().BuildSyntaxTree = true;
+  Invocation.getLangOptions().ParseForSyntaxTreeOnly = true;
   Invocation.getLangOptions().VerifySyntaxTree = options::VerifySyntaxTree;
   Invocation.getLangOptions().RequestEvaluatorGraphVizPath = options::GraphVisPath;
   Invocation.getFrontendOptions().InputsAndOutputs.addInputFile(InputFileName);
@@ -608,7 +618,9 @@ int parseFile(
   Invocation.setMainFileSyntaxParsingCache(SyntaxCache);
   Invocation.setModuleName("Test");
 
-  PrintingDiagnosticConsumer DiagConsumer;
+  std::string DiagsString;
+  llvm::raw_string_ostream DiagOS(DiagsString);
+  PrintingDiagnosticConsumer DiagConsumer(DiagOS);
   CompilerInstance Instance;
   Instance.addDiagnosticConsumer(&DiagConsumer);
   if (Instance.setup(Invocation)) {
@@ -658,7 +670,8 @@ int parseFile(
     }
   }
 
-  int ActionSpecificExitCode = ActionSpecificCallback(SF, SyntaxCache);
+  int ActionSpecificExitCode =
+    ActionSpecificCallback({SF, SyntaxCache, DiagOS.str()});
   if (ActionSpecificExitCode != EXIT_SUCCESS) {
     return ActionSpecificExitCode;
   } else {
@@ -713,16 +726,18 @@ int doDumpRawTokenSyntax(const StringRef InputFile) {
 int doFullParseRoundTrip(const char *MainExecutablePath,
                          const StringRef InputFile) {
   return parseFile(MainExecutablePath, InputFile,
-    [](SourceFile *SF, SyntaxParsingCache *SyntaxCache) -> int {
-    SF->getSyntaxRoot().print(llvm::outs(), {});
+    [](ParseInfo info) -> int {
+    info.SF->getSyntaxRoot().print(llvm::outs(), {});
     return EXIT_SUCCESS;
   });
 }
 
 int doSerializeRawTree(const char *MainExecutablePath,
                        const StringRef InputFile) {
   return parseFile(MainExecutablePath, InputFile,
-    [](SourceFile *SF, SyntaxParsingCache *SyntaxCache) -> int {
+    [](ParseInfo info) -> int {
+    auto SF = info.SF;
+    auto SyntaxCache = info.SyntaxCache;
     auto Root = SF->getSyntaxRoot().getRaw();
     std::unordered_set<unsigned> ReusedNodeIds;
     if (options::IncrementalSerialization && SyntaxCache) {
@@ -780,6 +795,23 @@ int doSerializeRawTree(const char *MainExecutablePath,
         SerializeTree(llvm::outs(), Root, SyntaxCache);
       }
     }
+
+    if (!options::DiagsOutputFilename.empty()) {
+      std::error_code errorCode;
+      llvm::raw_fd_ostream os(options::DiagsOutputFilename, errorCode,
+                              llvm::sys::fs::F_None);
+      if (errorCode) {
+        llvm::errs() << "error opening file '" << options::DiagsOutputFilename
+          << "': " << errorCode.message() << '\n';
+        return EXIT_FAILURE;
+      }
+      if (!info.Diags.empty())
+        os << info.Diags << '\n';
+    } else {
+      if (!info.Diags.empty())
+        llvm::errs() << info.Diags << '\n';
+    }
+
     return EXIT_SUCCESS;
   });
 }
@@ -800,27 +832,28 @@ int doDeserializeRawTree(const char *MainExecutablePath,
 
 int doParseOnly(const char *MainExecutablePath, const StringRef InputFile) {
   return parseFile(MainExecutablePath, InputFile,
-    [](SourceFile *SF, SyntaxParsingCache *SyntaxCache) {
-    return SF ? EXIT_SUCCESS : EXIT_FAILURE;
+    [](ParseInfo info) {
+    return info.SF ? EXIT_SUCCESS : EXIT_FAILURE;
   });
 }
 
 int dumpParserGen(const char *MainExecutablePath, const StringRef InputFile) {
   return parseFile(MainExecutablePath, InputFile,
-    [](SourceFile *SF, SyntaxParsingCache *SyntaxCache) {
+    [](ParseInfo info) {
     SyntaxPrintOptions Opts;
     Opts.PrintSyntaxKind = options::PrintNodeKind;
     Opts.Visual = options::Visual;
     Opts.PrintTrivialNodeKind = options::PrintTrivialNodeKind;
-    SF->getSyntaxRoot().print(llvm::outs(), Opts);
+    info.SF->getSyntaxRoot().print(llvm::outs(), Opts);
     return EXIT_SUCCESS;
   });
 }
 
 int dumpEOFSourceLoc(const char *MainExecutablePath,
                      const StringRef InputFile) {
   return parseFile(MainExecutablePath, InputFile,
-    [](SourceFile *SF, SyntaxParsingCache *SyntaxCache) -> int {
+    [](ParseInfo info) -> int {
+    auto SF = info.SF;
     auto BufferId = *SF->getBufferID();
     auto Root = SF->getSyntaxRoot();
     auto AbPos = Root.getEOFToken().getAbsolutePosition();
@@ -841,8 +874,8 @@ int dumpEOFSourceLoc(const char *MainExecutablePath,
 }
 }// end of anonymous namespace
 
-int invokeCommand(const char *MainExecutablePath,
-                  const StringRef InputSourceFilename) {
+static int invokeCommand(const char *MainExecutablePath,
+                         const StringRef InputSourceFilename) {
   int ExitCode = EXIT_SUCCESS;
   
   switch (options::Action) {
diff --git a/utils/incrparse/incr_transfer_round_trip.py b/utils/incrparse/incr_transfer_round_trip.py
@@ -161,6 +161,7 @@ def main():
                                      serialization_format=serialization_format,
                                      omit_node_ids=False,
                                      output_file=pre_edit_tree_file,
+                                     diags_output_file=None,
                                      temp_dir=temp_dir,
                                      swift_syntax_test=swift_syntax_test,
                                      print_visual_reuse_info=False)
@@ -172,6 +173,7 @@ def main():
                                      serialization_format=serialization_format,
                                      omit_node_ids=False,
                                      output_file=incremental_tree_file,
+                                     diags_output_file=None,
                                      temp_dir=temp_dir,
                                      swift_syntax_test=swift_syntax_test,
                                      print_visual_reuse_info=False)
diff --git a/utils/incrparse/incr_transfer_tree.py b/utils/incrparse/incr_transfer_tree.py
@@ -72,6 +72,7 @@ def main():
                                      serialization_format='json',
                                      omit_node_ids=False,
                                      output_file=incremental_serialized_file,
+                                     diags_output_file=None,
                                      temp_dir=temp_dir + '/temp',
                                      swift_syntax_test=swift_syntax_test,
                                      print_visual_reuse_info=False)
diff --git a/utils/incrparse/test_util.py b/utils/incrparse/test_util.py
@@ -133,8 +133,9 @@ def prepareForIncrParse(test_file, test_case, pre_edit_file, post_edit_file,
 
 def serializeIncrParseMarkupFile(test_file, test_case, mode,
                                  serialization_mode, serialization_format,
-                                 omit_node_ids, output_file, temp_dir,
-                                 swift_syntax_test, print_visual_reuse_info):
+                                 omit_node_ids, output_file, diags_output_file,
+                                 temp_dir, swift_syntax_test,
+                                 print_visual_reuse_info):
     test_file_name = os.path.basename(test_file)
     pre_edit_file = temp_dir + '/' + test_file_name + '.' + test_case + \
         '.pre.swift'
@@ -171,6 +172,9 @@ def serializeIncrParseMarkupFile(test_file, test_case, mode,
             '-output-filename', output_file
         ]
 
+        if diags_output_file:
+            command.extend(['-diags-output-filename', diags_output_file])
+
         if omit_node_ids:
             command.extend(['-omit-node-ids'])
 
@@ -320,6 +324,7 @@ def main():
                                      serialization_format=serialization_format,
                                      omit_node_ids=omit_node_ids,
                                      output_file=output_file,
+                                     diags_output_file=None,
                                      temp_dir=temp_dir,
                                      swift_syntax_test=swift_syntax_test,
                                      print_visual_reuse_info=visual_reuse_info)
diff --git a/utils/incrparse/validate_parse.py b/utils/incrparse/validate_parse.py
@@ -67,6 +67,11 @@ def main():
     post_edit_serialized_file = temp_dir + '/' + test_file_name + '.' \
         + test_case + '.post.json'
 
+    incremental_diags_file = temp_dir + '/' + test_file_name + '.' \
+        + test_case + '.diagsViaIncr.txt'
+    post_edit_diags_file = temp_dir + '/' + test_file_name + '.' \
+        + test_case + '.post.diags.txt'
+
     # Generate the syntax tree once incrementally and once from scratch
     try:
         serializeIncrParseMarkupFile(test_file=test_file,
@@ -76,6 +81,7 @@ def main():
                                      serialization_format='json',
                                      omit_node_ids=True,
                                      output_file=incremental_serialized_file,
+                                     diags_output_file=incremental_diags_file,
                                      temp_dir=temp_dir + '/temp',
                                      swift_syntax_test=swift_syntax_test,
                                      print_visual_reuse_info=visual_reuse_info)
@@ -91,6 +97,7 @@ def main():
                                      serialization_format='json',
                                      omit_node_ids=True,
                                      output_file=post_edit_serialized_file,
+                                     diags_output_file=post_edit_diags_file,
                                      temp_dir=temp_dir + '/temp',
                                      swift_syntax_test=swift_syntax_test,
                                      print_visual_reuse_info=visual_reuse_info)
@@ -104,7 +111,7 @@ def main():
     lines = difflib.unified_diff(open(incremental_serialized_file).readlines(),
                                  open(post_edit_serialized_file).readlines(),
                                  fromfile=incremental_serialized_file,
-                                 tofile=incremental_serialized_file)
+                                 tofile=post_edit_serialized_file)
     diff = '\n'.join(line for line in lines)
     if diff:
         print('Test case "%s" of %s FAILed' % (test_case, test_file),
@@ -114,6 +121,31 @@ def main():
         print(diff, file=sys.stderr)
         sys.exit(1)
 
+    # Verify that if the incremental parse resulted in parser diagnostics, those
+    # diagnostics were also emitted during the full parse.
+    # We can't just diff the outputs because the full parse includes diagnostics
+    # from the whole file, while the incremental parse includes only a subset.
+    # Each diagnostic is searched in the full parse diagnostics array but the
+    # search for each diagnostic continues from where the previous search
+    # stopped.
+    incremental_diags = open(incremental_diags_file).readlines()
+    post_edit_diags = open(post_edit_diags_file).readlines()
+    full_idx = 0
+    for diag in incremental_diags:
+        while full_idx < len(post_edit_diags):
+            if post_edit_diags[full_idx] == diag:
+                break
+            full_idx += 1
+        if full_idx == len(post_edit_diags):
+            print('Test case "%s" of %s FAILed' % (test_case, test_file),
+                  file=sys.stderr)
+            print('Parser diagnostic of incremental parsing was not emitted '
+                  'during from-scratch parsing of post-edit file:',
+                  file=sys.stderr)
+            print(diag, file=sys.stderr)
+            sys.exit(1)
+        full_idx += 1  # continue searching from the next diagnostic line.
+
 
 if __name__ == '__main__':
     main()
