r260543 - [Modules] Don't infinite recurse on implicit import of circular modules in preamble

[Ben Langmuir via cfe-commits]

Author: benlangmuir
Date: Thu Feb 11 11:04:42 2016
New Revision: 260543

URL: http://llvm.org/viewvc/llvm-project?rev=260543&view=rev
Log:
[Modules] Don't infinite recurse  on implicit import of circular modules in preamble

Update the Preprocessor's VisibleModuleSet when typo-correction creates
an implicit module import so that we won't accidentally write an invalid
SourceLocation into the preamble AST.  This would later lead to infinite
recursion when loading the preamble AST because we use the value in
ImportLocs to prevent visiting a module twice.

rdar://problem/24440990

Added:
    cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h
    cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h
    cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h
    cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h
    cfe/trunk/test/Index/preamble-with-implicit-import.m
Modified:
    cfe/trunk/lib/Basic/Module.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/test/Index/Inputs/module.map

Modified: cfe/trunk/lib/Basic/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=260543&r1=260542&r2=260543&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Module.cpp (original)
+++ cfe/trunk/lib/Basic/Module.cpp Thu Feb 11 11:04:42 2016
@@ -492,6 +492,7 @@ LLVM_DUMP_METHOD void Module::dump() con
 
 void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc,
                                   VisibleCallback Vis, ConflictCallback Cb) {
+  assert(Loc.isValid() && "setVisible expects a valid import location");
   if (isVisible(M))
     return;
 

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=260543&r1=260542&r2=260543&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Feb 11 11:04:42 2016
@@ -4062,7 +4062,9 @@ void ASTReader::InitializeContext() {
     if (Module *Imported = getSubmodule(Import.ID)) {
       makeModuleVisible(Imported, Module::AllVisible,
                         /*ImportLoc=*/Import.ImportLoc);
-      PP.makeModuleVisible(Imported, Import.ImportLoc);
+      if (Import.ImportLoc.isValid())
+        PP.makeModuleVisible(Imported, Import.ImportLoc);
+      // FIXME: should we tell Sema to make the module visible too?
     }
   }
   ImportedModules.clear();

Modified: cfe/trunk/test/Index/Inputs/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/module.map?rev=260543&r1=260542&r2=260543&view=diff
==============================================================================
--- cfe/trunk/test/Index/Inputs/module.map (original)
+++ cfe/trunk/test/Index/Inputs/module.map Thu Feb 11 11:04:42 2016
@@ -6,3 +6,17 @@ module ModuleNeedsVFS {
 framework module * { }
 
 module ModuleUndef { header "module-undef.h" }
+
+module PreambleWithImplicitImport {
+  module A {
+    header "preamble-with-implicit-import-A.h"
+  }
+  module B {
+    header "preamble-with-implicit-import-B.h"
+    export *
+  }
+  module C {
+    header "preamble-with-implicit-import-C.h"
+    export *
+  }
+}

Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h?rev=260543&view=auto
==============================================================================
--- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h (added)
+++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h Thu Feb 11 11:04:42 2016
@@ -0,0 +1 @@
+// preamble-with-implicit-import-A

Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h?rev=260543&view=auto
==============================================================================
--- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h (added)
+++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h Thu Feb 11 11:04:42 2016
@@ -0,0 +1,3 @@
+#pragma once
+#include "preamble-with-implicit-import-C.h" // Circular
+typedef struct { char x; } Typo;

Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h?rev=260543&view=auto
==============================================================================
--- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h (added)
+++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h Thu Feb 11 11:04:42 2016
@@ -0,0 +1,2 @@
+#pragma once
+#include "preamble-with-implicit-import-B.h" // Circular

Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h?rev=260543&view=auto
==============================================================================
--- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h (added)
+++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h Thu Feb 11 11:04:42 2016
@@ -0,0 +1,4 @@
+#include "preamble-with-implicit-import-A.h"
+
+// Typo is defined in B, which is not imported.
+void useTypeFromB(Typo *);

Added: cfe/trunk/test/Index/preamble-with-implicit-import.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-with-implicit-import.m?rev=260543&view=auto
==============================================================================
--- cfe/trunk/test/Index/preamble-with-implicit-import.m (added)
+++ cfe/trunk/test/Index/preamble-with-implicit-import.m Thu Feb 11 11:04:42 2016
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 2 none %s -I %S/Inputs -fmodules -fmodules-cache-path=%t -fspell-checking 2>&1 | FileCheck %s
+// CHECK: error: declaration of 'Typo' must be imported
+// CHECK: error: declaration of 'Typo' must be imported
+
+#include "preamble-with-implicit-import.h"