thakis created this revision.
thakis added a reviewer: rnk.
thakis added subscribers: cfe-commits, rsmith.

PCH files store the macro history for a  given macro, and the whole history 
list for one identifier is given to the Preprocessor at once via 
Preprocessor::setLoadedMacroDirective(). This contained an assert that no macro 
history exists yet for that identifier. That's usually true, but it's not true 
for builtin macros, which are created in Preprocessor() before flags and pchs 
are processed. Luckily, ASTWriter stops writing macro history lists at builtins 
(see shouldIgnoreMacro() in ASTWriter.cpp), so the head of the history list was 
missing for builtin macros. So make the assert weaker, and splice the history 
list to the existing single define for builtins.


https://reviews.llvm.org/D27545

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPMacroExpansion.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/builtin-macro.c

Index: test/PCH/builtin-macro.c
===================================================================
--- test/PCH/builtin-macro.c
+++ test/PCH/builtin-macro.c
@@ -0,0 +1,33 @@
+
+// Test this without pch.
+// RUN: %clang_cc1 -D__DATE__= -D__TIMESTAMP__= -include %s -Wno-builtin-macro-redefined -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -D__DATE__= -D__TIMESTAMP__= -Wno-builtin-macro-redefined -emit-pch -o %t %s
+// RUN: %clang_cc1 -D__DATE__= -D__TIMESTAMP__= -Wno-builtin-macro-redefined -include-pch %t -fsyntax-only -verify %s 
+
+#if !defined(HEADER)
+#define HEADER
+
+#define __TIME__
+
+#undef __TIMESTAMP__
+#define __TIMESTAMP__
+
+// FIXME: undefs don't work well with pchs yet, see PR31311
+// Once that's fixed, add -U__COUNTER__ to all command lines and check that
+// an attempt to use __COUNTER__ at the bottom produces an error in both non-pch
+// and pch case (works fine in the former case already).
+// Same for #undef __FILE__ right here and a use of that at the bottom.
+//#undef __FILE__
+
+// Also spot-check a predefine
+#undef __STDC_HOSTED__
+
+#else
+
+const char s[] = __DATE__ " " __TIME__ " " __TIMESTAMP__;
+
+const int d = __STDC_HOSTED__; // expected-error{{use of undeclared identifier '__STDC_HOSTED__'}}
+
+#endif
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -1953,7 +1953,7 @@
   }
 
   if (Latest)
-    PP.setLoadedMacroDirective(II, Latest);
+    PP.setLoadedMacroDirective(II, Earliest, Latest);
 }
 
 ASTReader::InputFileInfo
Index: lib/Lex/PPMacroExpansion.cpp
===================================================================
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -92,12 +92,33 @@
 }
 
 void Preprocessor::setLoadedMacroDirective(IdentifierInfo *II,
+                                           MacroDirective *ED,
                                            MacroDirective *MD) {
+  // Normally, when a macro is defined, it goes through appendMacroDirective()
+  // above, which chains a macro to previous defines, undefs, etc.
+  // However, in a pch, the whole macro history up to the end of the pch is
+  // stored, so ASTReader goes through this function instead.
+  // However, built-in macros are already registered in the Preprocessor
+  // ctor, and ASTWriter stops writing the macro chain at built-in macros,
+  // so in that case the chain from the pch needs to be spliced to the existing
+  // built-in.
+
   assert(II && MD);
   MacroState &StoredMD = CurSubmoduleState->Macros[II];
-  assert(!StoredMD.getLatest() &&
-         "the macro history was modified before initializing it from a pch");
-  StoredMD = MD;
+
+  if (auto *OldMD = StoredMD.getLatest()) {
+    // FIXME: shouldIgnoreMacro() in ASTWriter also stops at macros from the
+    // predefines buffer in module builds. Do we need to splice to those here
+    // too?
+    assert(OldMD->getMacroInfo()->isBuiltinMacro() &&
+           "only built-ins should have an entry here");
+    assert(!OldMD->getPrevious() && "builtin should only have a singe entry");
+    ED->setPrevious(OldMD);
+    StoredMD.setLatest(MD);
+  } else {
+    StoredMD = MD;
+  }
+
   // Setup the identifier as having associated macro history.
   II->setHasMacroDefinition(true);
   if (!MD->isDefined() && LeafModuleMacros.find(II) == LeafModuleMacros.end())
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -887,7 +887,8 @@
     return appendDefMacroDirective(II, MI, MI->getDefinitionLoc());
   }
   /// \brief Set a MacroDirective that was loaded from a PCH file.
-  void setLoadedMacroDirective(IdentifierInfo *II, MacroDirective *MD);
+  void setLoadedMacroDirective(IdentifierInfo *II, MacroDirective *ED,
+                               MacroDirective *MD);
 
   /// \brief Register an exported macro for a module and identifier.
   ModuleMacro *addModuleMacro(Module *Mod, IdentifierInfo *II, MacroInfo *Macro,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to