Teach the PCH validator to check the preprocessor options, especially
authorDouglas Gregor <dgregor@apple.com>
Wed, 24 Oct 2012 23:41:50 +0000 (23:41 +0000)
committerDouglas Gregor <dgregor@apple.com>
Wed, 24 Oct 2012 23:41:50 +0000 (23:41 +0000)
the macros that are #define'd or #undef'd on the command line. This
checking happens much earlier than the current macro-definition
checking and is far cleaner, because it does a direct comparison
rather than a diff of the predefines buffers. Moreover, it allows us
to use the result of this check to skip over PCH files within a
directory that have non-matching -D's or -U's on the command
line. Finally, it improves the diagnostics a bit for mismatches,
fixing <rdar://problem/8612222>.

The old predefines-buffer diff'ing will go away in a subsequent commit.

llvm-svn: 166641

clang/include/clang/Basic/DiagnosticSerializationKinds.td
clang/include/clang/Serialization/ASTReader.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/PCH/fuzzy-pch.c
clang/test/PCH/pch-dir.c

index 435760b..978a35d 100644 (file)
@@ -45,6 +45,25 @@ def warn_pch_different_branch : Error<
     "PCH file built from a different branch (%0) than the compiler (%1)">;
 def err_pch_with_compiler_errors : Error<
     "PCH file contains compiler errors">;
+    
+    
+def err_pch_macro_def_undef : Error<
+    "macro '%0' was %select{defined|undef'd}1 in the precompiled header but "
+    "%select{undef'd|defined}1 on the command line">;
+def err_pch_macro_def_conflict : Error<
+    "definition of macro '%0' differs between the precompiled header ('%1') "
+    "and the command line ('%2')">;
+def err_pch_include_opt_missing : Error<
+    "precompiled header depends on '%select{-include|-imacros}0 %1' option "
+    "that is missing from the command line">;
+def err_pch_include_opt_conflict : Error<
+    "precompiled header option '%select{-include|-imacros}0 %1' conflicts with "
+    "corresponding option '%select{-include|-imacros}0 %2' on command line">;
+def err_pch_undef : Error<
+    "%select{command line contains|precompiled header was built with}0 "
+    "'-undef' but %select{precompiled header was not built with it|"
+    "it is not present on the command line}0">;
+
 def warn_cmdline_conflicting_macro_def : Error<
     "definition of the macro '%0' conflicts with the definition used to "
     "build the precompiled header">;
index d285ca5..3b5cf8d 100644 (file)
@@ -203,6 +203,8 @@ public:
                                    bool Complain);
   virtual bool ReadTargetOptions(const TargetOptions &TargetOpts,
                                  bool Complain);
+  virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+                                       bool Complain);
   virtual bool ReadPredefinesBuffer(const PCHPredefinesBlocks &Buffers,
                                     StringRef OriginalFileName,
                                     std::string &SuggestedPredefines,
@@ -1209,7 +1211,8 @@ public:
   static bool isAcceptableASTFile(StringRef Filename,
                                   FileManager &FileMgr,
                                   const LangOptions &LangOpts,
-                                  const TargetOptions &TargetOpts);
+                                  const TargetOptions &TargetOpts,
+                                  const PreprocessorOptions &PPOpts);
 
   /// \brief Returns the suggested contents of the predefines buffer,
   /// which contains a (typically-empty) subset of the predefines
index b7b93a9..fa1655d 100644 (file)
@@ -243,7 +243,8 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
         // Check whether this is an acceptable AST file.
         if (ASTReader::isAcceptableASTFile(Dir->path(), FileMgr,
                                            CI.getLangOpts(),
-                                           CI.getTargetOpts())) {
+                                           CI.getTargetOpts(),
+                                           CI.getPreprocessorOpts())) {
           for (unsigned I = 0, N = PPOpts.Includes.size(); I != N; ++I) {
             if (PPOpts.Includes[I] == PPOpts.ImplicitPCHInclude) {
               PPOpts.Includes[I] = Dir->path();
index ccadfbd..4d0ad91 100644 (file)
@@ -187,8 +187,8 @@ static bool checkTargetOptions(const TargetOptions &TargetOpts,
 bool
 PCHValidator::ReadLanguageOptions(const LangOptions &LangOpts,
                                   bool Complain) {
-  const LangOptions &PPLangOpts = PP.getLangOpts();
-  return checkLanguageOptions(LangOpts, PPLangOpts,
+  const LangOptions &ExistingLangOpts = PP.getLangOpts();
+  return checkLanguageOptions(LangOpts, ExistingLangOpts,
                               Complain? &Reader.Diags : 0);
 }
 
@@ -200,6 +200,119 @@ bool PCHValidator::ReadTargetOptions(const TargetOptions &TargetOpts,
 }
 
 namespace {
+  typedef llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/> >
+    MacroDefinitionsMap;
+}
+
+/// \brief Collect the macro definitions provided by the given preprocessor
+/// options.
+static void collectMacroDefinitions(const PreprocessorOptions &PPOpts,
+                                    MacroDefinitionsMap &Macros,
+                                    SmallVectorImpl<StringRef> *MacroNames = 0){
+  for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
+    StringRef Macro = PPOpts.Macros[I].first;
+    bool IsUndef = PPOpts.Macros[I].second;
+
+    std::pair<StringRef, StringRef> MacroPair = Macro.split('=');
+    StringRef MacroName = MacroPair.first;
+    StringRef MacroBody = MacroPair.second;
+
+    // For an #undef'd macro, we only care about the name.
+    if (IsUndef) {
+      if (MacroNames && !Macros.count(MacroName))
+        MacroNames->push_back(MacroName);
+
+      Macros[MacroName] = std::make_pair("", true);
+      continue;
+    }
+
+    // For a #define'd macro, figure out the actual definition.
+    if (MacroName.size() == Macro.size())
+      MacroBody = "1";
+    else {
+      // Note: GCC drops anything following an end-of-line character.
+      StringRef::size_type End = MacroBody.find_first_of("\n\r");
+      MacroBody = MacroBody.substr(0, End);
+    }
+
+    if (MacroNames && !Macros.count(MacroName))
+      MacroNames->push_back(MacroName);
+    Macros[MacroName] = std::make_pair(MacroBody, false);
+  }
+}
+         
+/// \brief Check the preprocessor options deserialized from the control block
+/// against the preprocessor options in an existing preprocessor.
+///
+/// \param Diags If non-null, produce diagnostics for any mismatches incurred.
+static bool checkPreprocessorOptions(const PreprocessorOptions &PPOpts,
+                                     const PreprocessorOptions &ExistingPPOpts,
+                                     DiagnosticsEngine *Diags) {
+  // Check macro definitions.
+  MacroDefinitionsMap ASTFileMacros;
+  collectMacroDefinitions(PPOpts, ASTFileMacros);
+  MacroDefinitionsMap ExistingMacros;
+  SmallVector<StringRef, 4> ExistingMacroNames;
+  collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
+
+  for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
+    // Dig out the macro definition in the existing preprocessor options.
+    StringRef MacroName = ExistingMacroNames[I];
+    std::pair<StringRef, bool> Existing = ExistingMacros[MacroName];
+
+    // Check whether we know anything about this macro name or not.
+    llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/> >::iterator Known
+      = ASTFileMacros.find(MacroName);
+    if (Known == ASTFileMacros.end()) {
+      // FIXME: Check whether this identifier was referenced anywhere in the
+      // AST file. If so, we should reject the AST file. Unfortunately, this
+      // information isn't in the control block. What shall we do about it?
+      continue;
+    }
+
+    // If the macro was defined in one but undef'd in the other, we have a
+    // conflict.
+    if (Existing.second != Known->second.second) {
+      if (Diags) {
+        Diags->Report(diag::err_pch_macro_def_undef)
+          << MacroName << Known->second.second;
+      }
+      return true;
+    }
+
+    // If the macro was #undef'd in both, or if the macro bodies are identical,
+    // it's fine.
+    if (Existing.second || Existing.first == Known->second.first)
+      continue;
+
+    // The macro bodies differ; complain.
+    if (Diags) {
+      Diags->Report(diag::err_pch_macro_def_conflict)
+        << MacroName << Known->second.first << Existing.first;
+    }
+    return true;
+  }
+
+  // Check whether we're using predefines.
+  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines) {
+    if (Diags) {
+      Diags->Report(diag::err_pch_undef) << ExistingPPOpts.UsePredefines;
+    }
+    return true;
+  }
+
+  return false;
+}
+
+bool PCHValidator::ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+                                           bool Complain) {
+  const PreprocessorOptions &ExistingPPOpts = PP.getPreprocessorOpts();
+
+  return checkPreprocessorOptions(PPOpts, ExistingPPOpts,
+                                  Complain? &Reader.Diags : 0);
+}
+
+namespace {
   struct EmptyStringRef {
     bool operator ()(StringRef r) const { return r.empty(); }
   };
@@ -3389,12 +3502,15 @@ namespace {
   class SimplePCHValidator : public ASTReaderListener {
     const LangOptions &ExistingLangOpts;
     const TargetOptions &ExistingTargetOpts;
+    const PreprocessorOptions &ExistingPPOpts;
 
   public:
     SimplePCHValidator(const LangOptions &ExistingLangOpts,
-                       const TargetOptions &ExistingTargetOpts)
+                       const TargetOptions &ExistingTargetOpts,
+                       const PreprocessorOptions &ExistingPPOpts)
       : ExistingLangOpts(ExistingLangOpts),
-        ExistingTargetOpts(ExistingTargetOpts)
+        ExistingTargetOpts(ExistingTargetOpts),
+        ExistingPPOpts(ExistingPPOpts)
     {
     }
 
@@ -3406,13 +3522,18 @@ namespace {
                                    bool Complain) {
       return checkTargetOptions(ExistingTargetOpts, TargetOpts, 0);
     }
+    virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+                                         bool Complain) {
+      return checkPreprocessorOptions(ExistingPPOpts, PPOpts, 0);
+    }
   };
 }
 
 bool ASTReader::isAcceptableASTFile(StringRef Filename,
                                     FileManager &FileMgr,
                                     const LangOptions &LangOpts,
-                                    const TargetOptions &TargetOpts) {
+                                    const TargetOptions &TargetOpts,
+                                    const PreprocessorOptions &PPOpts) {
   // Open the AST file.
   std::string ErrStr;
   OwningPtr<llvm::MemoryBuffer> Buffer;
@@ -3436,7 +3557,7 @@ bool ASTReader::isAcceptableASTFile(StringRef Filename,
     return false;
   }
 
-  SimplePCHValidator Validator(LangOpts, TargetOpts);
+  SimplePCHValidator Validator(LangOpts, TargetOpts, PPOpts);
   RecordData Record;
   bool InControlBlock = false;
   while (!Stream.AtEndOfStream()) {
@@ -3952,6 +4073,7 @@ bool ASTReader::ParsePreprocessorOptions(const RecordData &Record,
     PPOpts.MacroIncludes.push_back(ReadString(Record, Idx));
   }
 
+  PPOpts.UsePredefines = Record[Idx++];
   PPOpts.ImplicitPCHInclude = ReadString(Record, Idx);
   PPOpts.ImplicitPTHInclude = ReadString(Record, Idx);
   PPOpts.ObjCXXARCStandardLibrary =
index 50e6763..ea10086 100644 (file)
@@ -1153,6 +1153,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
   for (unsigned I = 0, N = PPOpts.MacroIncludes.size(); I != N; ++I)
     AddString(PPOpts.MacroIncludes[I], Record);
 
+  Record.push_back(PPOpts.UsePredefines);
   AddString(PPOpts.ImplicitPCHInclude, Record);
   AddString(PPOpts.ImplicitPTHInclude, Record);
   Record.push_back(static_cast<unsigned>(PPOpts.ObjCXXARCStandardLibrary));
index 5675753..7296d1d 100644 (file)
@@ -1,8 +1,14 @@
 // Test with pch.
 // RUN: %clang_cc1 -emit-pch -DFOO -o %t %S/variables.h
 // RUN: %clang_cc1 -DBAR=int -include-pch %t -fsyntax-only -pedantic %s
-// RUN: %clang_cc1 -DFOO -DBAR=int -include-pch %t -Werror %s
-// RUN: not %clang_cc1 -DFOO -DBAR=int -DX=5 -include-pch %t -Werror %s 
+// RUN: %clang_cc1 -DFOO -DBAR=int -include-pch %t %s
+// RUN: not %clang_cc1 -DFOO=blah -DBAR=int -include-pch %t %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+// RUN: not %clang_cc1 -UFOO -include-pch %t %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: not %clang_cc1 -DFOO -undef -include-pch %t %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-UNDEF %s < %t.err
 
 BAR bar = 17;
 
@@ -17,3 +23,9 @@ BAR bar = 17;
 #ifndef BAR
 #  error BAR was not defined
 #endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
+
+// CHECK-UNDEF: command line contains '-undef' but precompiled header was not built with it
+
index 73a7ba9..ae841ce 100644 (file)
@@ -1,14 +1,19 @@
 // RUN: mkdir -p %t.h.gch
-// RUN: %clang -x c-header %S/pch-dir.h -o %t.h.gch/c.gch 
+// RUN: %clang -x c-header %S/pch-dir.h -DFOO=foo -o %t.h.gch/c.gch 
+// RUN: %clang -x c-header %S/pch-dir.h -DFOO=bar -o %t.h.gch/cbar.gch 
 // RUN: %clang -x c++-header -std=c++98 %S/pch-dir.h -o %t.h.gch/cpp.gch 
-// RUN: %clang -include %t.h -fsyntax-only %s -Xclang -print-stats 2> %t.clog
+// RUN: %clang -include %t.h -DFOO=foo -fsyntax-only %s -Xclang -print-stats 2> %t.clog
 // RUN: FileCheck -check-prefix=C %s < %t.clog
+// RUN: %clang -include %t.h -DFOO=bar -DBAR=bar -fsyntax-only %s -Xclang -ast-print > %t.cbarlog
+// RUN: FileCheck -check-prefix=CBAR %s < %t.cbarlog
 // RUN: %clang -x c++ -include %t.h -std=c++98 -fsyntax-only %s -Xclang -print-stats 2> %t.cpplog
 // RUN: FileCheck -check-prefix=CPP %s < %t.cpplog
 
 // RUN: not %clang -x c++ -std=c++11 -include %t.h -fsyntax-only %s 2> %t.cpp11log
 // RUN: FileCheck -check-prefix=CPP11 %s < %t.cpp11log
 
+// CHECK-CBAR: int bar
+int FOO;
 
 int get() {
 #ifdef __cplusplus