From 6d25fdc41b7444e15395f5e8f9961825f1d0a163 Mon Sep 17 00:00:00 2001
From: Ben Langmuir <blangmuir@apple.com>
Date: Thu, 11 Feb 2016 17:04:42 +0000
Subject: [PATCH] [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

llvm-svn: 260543
---
 clang/lib/Basic/Module.cpp                         |  1 +
 clang/lib/Serialization/ASTReader.cpp              |  4 +++-
 clang/test/Index/Inputs/module.map                 | 14 ++++++++++++++
 .../Index/Inputs/preamble-with-implicit-import-A.h |  1 +
 .../Index/Inputs/preamble-with-implicit-import-B.h |  3 +++
 .../Index/Inputs/preamble-with-implicit-import-C.h |  2 ++
 .../Index/Inputs/preamble-with-implicit-import.h   |  4 ++++
 clang/test/Index/preamble-with-implicit-import.m   |  6 ++++++
 8 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/Index/Inputs/preamble-with-implicit-import-A.h
 create mode 100644 clang/test/Index/Inputs/preamble-with-implicit-import-B.h
 create mode 100644 clang/test/Index/Inputs/preamble-with-implicit-import-C.h
 create mode 100644 clang/test/Index/Inputs/preamble-with-implicit-import.h
 create mode 100644 clang/test/Index/preamble-with-implicit-import.m

diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index baeeb9edbddf..e119c0535eb2 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -492,6 +492,7 @@ LLVM_DUMP_METHOD void Module::dump() const {
 
 void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc,
                                   VisibleCallback Vis, ConflictCallback Cb) {
+  assert(Loc.isValid() && "setVisible expects a valid import location");
   if (isVisible(M))
     return;
 
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 1c62b4e57b0b..b2f59d1fa6c9 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -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();
diff --git a/clang/test/Index/Inputs/module.map b/clang/test/Index/Inputs/module.map
index 4bfc109a8b13..10712accb1c2 100644
--- a/clang/test/Index/Inputs/module.map
+++ b/clang/test/Index/Inputs/module.map
@@ -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 *
+  }
+}
diff --git a/clang/test/Index/Inputs/preamble-with-implicit-import-A.h b/clang/test/Index/Inputs/preamble-with-implicit-import-A.h
new file mode 100644
index 000000000000..c68390159094
--- /dev/null
+++ b/clang/test/Index/Inputs/preamble-with-implicit-import-A.h
@@ -0,0 +1 @@
+// preamble-with-implicit-import-A
diff --git a/clang/test/Index/Inputs/preamble-with-implicit-import-B.h b/clang/test/Index/Inputs/preamble-with-implicit-import-B.h
new file mode 100644
index 000000000000..17c138dfb5aa
--- /dev/null
+++ b/clang/test/Index/Inputs/preamble-with-implicit-import-B.h
@@ -0,0 +1,3 @@
+#pragma once
+#include "preamble-with-implicit-import-C.h" // Circular
+typedef struct { char x; } Typo;
diff --git a/clang/test/Index/Inputs/preamble-with-implicit-import-C.h b/clang/test/Index/Inputs/preamble-with-implicit-import-C.h
new file mode 100644
index 000000000000..a3fc1d4fea06
--- /dev/null
+++ b/clang/test/Index/Inputs/preamble-with-implicit-import-C.h
@@ -0,0 +1,2 @@
+#pragma once
+#include "preamble-with-implicit-import-B.h" // Circular
diff --git a/clang/test/Index/Inputs/preamble-with-implicit-import.h b/clang/test/Index/Inputs/preamble-with-implicit-import.h
new file mode 100644
index 000000000000..1b429678f213
--- /dev/null
+++ b/clang/test/Index/Inputs/preamble-with-implicit-import.h
@@ -0,0 +1,4 @@
+#include "preamble-with-implicit-import-A.h"
+
+// Typo is defined in B, which is not imported.
+void useTypeFromB(Typo *);
diff --git a/clang/test/Index/preamble-with-implicit-import.m b/clang/test/Index/preamble-with-implicit-import.m
new file mode 100644
index 000000000000..e3d0e8b1a62b
--- /dev/null
+++ b/clang/test/Index/preamble-with-implicit-import.m
@@ -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"