[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 198654.
nik edited the summary of this revision.
nik added a comment.

Rebased for current trunk.

If I miss something obvious, please tell me. Otherwise I'm waiting.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Basic/SourceManager.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===================================================================
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1871,6 +1871,18 @@
     return {ImportAction::None};
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+      SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+    Diag(FilenameTok.getLocation(),
+         diag::err_pp_including_mainfile_for_preamble);
+    return {ImportAction::None};
+  }
+
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
Index: lib/Basic/SourceManager.cpp
===================================================================
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1591,7 +1591,7 @@
         // as the main file.
         const FileEntry *MainFile = MainContentCache->OrigEntry;
         SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-        if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) {
+        if (MainFile && *SourceFileName == llvm::sys::path::filename(MainFile->getName())) {
           SourceFileUID = getActualFileUID(SourceFile);
           if (SourceFileUID) {
             if (Optional<llvm::sys::fs::UniqueID> MainFileUID =
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -426,6 +426,8 @@
   "did not find header '%0' in framework '%1' (loaded from '%2')">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
+def err_pp_including_mainfile_for_preamble : Error<
+  "main file cannot be included recursively when building a preamble">;
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or <FILENAME>">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53866.198654.patch
Type: text/x-patch
Size: 3034 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190508/5c4d1fa9/attachment.bin>