Avoid a couple of assertions when preprocessing with modules

1. We were hitting the NextIsPrevious assertion because we were trying
to merge decl chains that were independent of each other because we had
no Sema object to allow them to find existing decls. This is fixed by
delaying loading the "preloaded" decls until Sema is available.

2. We were trying to get identifier info from an annotation token, which
asserts.  The fix is to special-case the module annotations in the
preprocessed output printer.

Fixed in a single commit because when you hit 1 you almost invariably
hit 2 as well.

llvm-svn: 217550

8 files changed, 61 insertions, 22 deletions

diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 12d27f105f2..1b3d46a6e47 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2076,9 +2076,9 @@ public:
   /// \brief Retrieve the AST context that this AST reader supplements.
   ASTContext &getContext() { return Context; }
 
-  // \brief Contains declarations that were loaded before we have
+  // \brief Contains the IDs for declarations that were requested before we have
   // access to a Sema object.
-  SmallVector<NamedDecl *, 16> PreloadedDecls;
+  SmallVector<uint64_t, 16> PreloadedDeclIDs;
 
   /// \brief Retrieve the semantic analysis object used to analyze the
   /// translation unit in which the precompiled header is being
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index 17d7279e3ff..7c1d9a56883 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -332,7 +332,10 @@ void PrintPPOutputPPCallbacks::InclusionDirective(SourceLocation HashLoc,
     MoveToLine(HashLoc);
     OS << "@import " << Imported->getFullModuleName() << ";"
        << " /* clang -E: implicit import for \"" << File->getName() << "\" */";
+    // Since we want a newline after the @import, but not a #<line>, start a new
+    // line immediately.
     EmittedTokensOnThisLine = true;
+    startNewLineIfNeeded();
   }
 }
 
diff --git a/clang/lib/Lex/TokenConcatenation.cpp b/clang/lib/Lex/TokenConcatenation.cpp
index 0a66bba91fc..866cbb142c8 100644
--- a/clang/lib/Lex/TokenConcatenation.cpp
+++ b/clang/lib/Lex/TokenConcatenation.cpp
@@ -163,8 +163,8 @@ bool TokenConcatenation::AvoidConcat(const Token &PrevPrevTok,
     return false;
 
   tok::TokenKind PrevKind = PrevTok.getKind();
-  if (PrevTok.getIdentifierInfo())  // Language keyword or named operator.
-    PrevKind = tok::identifier;
+  if (!PrevTok.isAnnotation() && PrevTok.getIdentifierInfo())
+    PrevKind = tok::identifier; // Language keyword or named operator.
 
   // Look up information on when we should avoid concatenation with prevtok.
   unsigned ConcatInfo = TokenInfo[PrevKind];
@@ -178,6 +178,14 @@ bool TokenConcatenation::AvoidConcat(const Token &PrevPrevTok,
       return true;
     ConcatInfo &= ~aci_avoid_equal;
   }
+  if (Tok.isAnnotation()) {
+    // Modules annotation can show up when generated automatically for includes.
+    assert((Tok.is(tok::annot_module_include) ||
+            Tok.is(tok::annot_module_begin) ||
+            Tok.is(tok::annot_module_end)) &&
+           "unexpected annotation in AvoidConcat");
+    ConcatInfo = 0;
+  }
 
   if (ConcatInfo == 0) return false;
 
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 8f1d88c6d58..93c01cb1e12 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -6893,11 +6893,11 @@ void ASTReader::InitializeSema(Sema &S) {
 
   // Makes sure any declarations that were deserialized "too early"
   // still get added to the identifier's declaration chains.
-  for (unsigned I = 0, N = PreloadedDecls.size(); I != N; ++I) {
-    pushExternalDeclIntoScope(PreloadedDecls[I],
-                              PreloadedDecls[I]->getDeclName());
+  for (uint64_t ID : PreloadedDeclIDs) {
+    NamedDecl *D = cast<NamedDecl>(GetDecl(ID));
+    pushExternalDeclIntoScope(D, D->getDeclName());
   }
-  PreloadedDecls.clear();
+  PreloadedDeclIDs.clear();
 
   // FIXME: What happens if these are changed by a module import?
   if (!FPPragmaOptions.empty()) {
@@ -7349,24 +7349,26 @@ ASTReader::SetGloballyVisibleDecls(IdentifierInfo *II,
   }
 
   for (unsigned I = 0, N = DeclIDs.size(); I != N; ++I) {
-    NamedDecl *D = cast<NamedDecl>(GetDecl(DeclIDs[I]));
-    if (SemaObj) {
-      // If we're simply supposed to record the declarations, do so now.
-      if (Decls) {
-        Decls->push_back(D);
-        continue;
-      }
-
-      // Introduce this declaration into the translation-unit scope
-      // and add it to the declaration chain for this identifier, so
-      // that (unqualified) name lookup will find it.
-      pushExternalDeclIntoScope(D, II);
-    } else {
+    if (!SemaObj) {
       // Queue this declaration so that it will be added to the
       // translation unit scope and identifier's declaration chain
       // once a Sema object is known.
-      PreloadedDecls.push_back(D);
+      PreloadedDeclIDs.push_back(DeclIDs[I]);
+      continue;
     }
+
+    NamedDecl *D = cast<NamedDecl>(GetDecl(DeclIDs[I]));
+
+    // If we're simply supposed to record the declarations, do so now.
+    if (Decls) {
+      Decls->push_back(D);
+      continue;
+    }
+
+    // Introduce this declaration into the translation-unit scope
+    // and add it to the declaration chain for this identifier, so
+    // that (unqualified) name lookup will find it.
+    pushExternalDeclIntoScope(D, II);
   }
 }
 
diff --git a/clang/test/Modules/Inputs/diamond_left.h b/clang/test/Modules/Inputs/diamond_left.h
index fce2e48882f..6494551e4bf 100644
--- a/clang/test/Modules/Inputs/diamond_left.h
+++ b/clang/test/Modules/Inputs/diamond_left.h
@@ -1,3 +1,5 @@
+int top_left_before(void *);
+
 @import diamond_top;
 
 float left(float *);
diff --git a/clang/test/Modules/Inputs/diamond_top.h b/clang/test/Modules/Inputs/diamond_top.h
index 34998cd4324..30da14f6449 100644
--- a/clang/test/Modules/Inputs/diamond_top.h
+++ b/clang/test/Modules/Inputs/diamond_top.h
@@ -2,3 +2,4 @@ int top(int *);
 
 int top_left(char *c);
 
+int top_left_before(void *);
diff --git a/clang/test/Modules/Inputs/preprocess-prefix.h b/clang/test/Modules/Inputs/preprocess-prefix.h
new file mode 100644
index 00000000000..04d01758320
--- /dev/null
+++ b/clang/test/Modules/Inputs/preprocess-prefix.h
@@ -0,0 +1,2 @@
+int left_and_right(int *);
+#import "diamond_left.h"
diff --git a/clang/test/Modules/preprocess.m b/clang/test/Modules/preprocess.m
new file mode 100644
index 00000000000..a1c024d6ffb
--- /dev/null
+++ b/clang/test/Modules/preprocess.m
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs -include %S/Inputs/preprocess-prefix.h -E %s | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs -x objective-c-header -emit-pch %S/Inputs/preprocess-prefix.h -o %t.pch
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs -include-pch %t.pch -E %s | FileCheck -strict-whitespace %s
+#import "diamond_right.h"
+#import "diamond_right.h" // to check that imports get their own line
+void test() {
+  top_left_before();
+  left_and_right();
+}
+
+
+// CHECK: int left_and_right(int *);{{$}}
+// CHECK-NEXT: @import diamond_left; /* clang -E: implicit import for "{{.*}}diamond_left.h" */{{$}}
+
+// CHECK: @import diamond_right; /* clang -E: implicit import for "/Users/blangmuir/src/clang/test/Modules/Inputs/diamond_right.h" */{{$}}
+// CHECK: @import diamond_right; /* clang -E: implicit import for "/Users/blangmuir/src/clang/test/Modules/Inputs/diamond_right.h" */{{$}}
+// CHECK-NEXT: void test() {{{$}}
+// CHECK-NEXT:    top_left_before();{{$}}
+// CHECK-NEXT:    left_and_right();{{$}}
+// CHECK-NEXT: }{{$}}