r217550 - Avoid a couple of assertions when preprocessing with modules

Ben Langmuir blangmuir at apple.com
Wed Sep 10 14:29:42 PDT 2014


Author: benlangmuir
Date: Wed Sep 10 16:29:41 2014
New Revision: 217550

URL: http://llvm.org/viewvc/llvm-project?rev=217550&view=rev
Log:
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.

Added:
    cfe/trunk/test/Modules/Inputs/preprocess-prefix.h
    cfe/trunk/test/Modules/preprocess.m
Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp
    cfe/trunk/lib/Lex/TokenConcatenation.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/test/Modules/Inputs/diamond_left.h
    cfe/trunk/test/Modules/Inputs/diamond_top.h

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=217550&r1=217549&r2=217550&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Sep 10 16:29:41 2014
@@ -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

Modified: cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp?rev=217550&r1=217549&r2=217550&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp (original)
+++ cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp Wed Sep 10 16:29:41 2014
@@ -332,7 +332,10 @@ void PrintPPOutputPPCallbacks::Inclusion
     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();
   }
 }
 

Modified: cfe/trunk/lib/Lex/TokenConcatenation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/TokenConcatenation.cpp?rev=217550&r1=217549&r2=217550&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/TokenConcatenation.cpp (original)
+++ cfe/trunk/lib/Lex/TokenConcatenation.cpp Wed Sep 10 16:29:41 2014
@@ -163,8 +163,8 @@ bool TokenConcatenation::AvoidConcat(con
     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(con
       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;
 

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=217550&r1=217549&r2=217550&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Sep 10 16:29:41 2014
@@ -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(Ident
   }
 
   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);
   }
 }
 

Modified: cfe/trunk/test/Modules/Inputs/diamond_left.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diamond_left.h?rev=217550&r1=217549&r2=217550&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/diamond_left.h (original)
+++ cfe/trunk/test/Modules/Inputs/diamond_left.h Wed Sep 10 16:29:41 2014
@@ -1,3 +1,5 @@
+int top_left_before(void *);
+
 @import diamond_top;
 
 float left(float *);

Modified: cfe/trunk/test/Modules/Inputs/diamond_top.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diamond_top.h?rev=217550&r1=217549&r2=217550&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/diamond_top.h (original)
+++ cfe/trunk/test/Modules/Inputs/diamond_top.h Wed Sep 10 16:29:41 2014
@@ -2,3 +2,4 @@ int top(int *);
 
 int top_left(char *c);
 
+int top_left_before(void *);

Added: cfe/trunk/test/Modules/Inputs/preprocess-prefix.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/preprocess-prefix.h?rev=217550&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/preprocess-prefix.h (added)
+++ cfe/trunk/test/Modules/Inputs/preprocess-prefix.h Wed Sep 10 16:29:41 2014
@@ -0,0 +1,2 @@
+int left_and_right(int *);
+#import "diamond_left.h"

Added: cfe/trunk/test/Modules/preprocess.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/preprocess.m?rev=217550&view=auto
==============================================================================
--- cfe/trunk/test/Modules/preprocess.m (added)
+++ cfe/trunk/test/Modules/preprocess.m Wed Sep 10 16:29:41 2014
@@ -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: }{{$}}





More information about the cfe-commits mailing list