Index: modularize/Modularize.cpp
===================================================================
--- modularize/Modularize.cpp	(revision 183718)
+++ modularize/Modularize.cpp	(working copy)
@@ -39,7 +39,23 @@
 //     (file):(row):(column)
 //     (file):(row):(column)
 //
-// error: header '(file)' has different contents dependening on how it was
+// The following message might appear if preprocessor conditional directives
+// resolve differently.
+//
+// warning: The instances of header (file) have different contents after
+//              preprocessing:
+//     When included or nested in these top-level headers:
+//         (file list)
+//     This preprocessor directive is mismatched (the parentheses show the
+//              condition after macro substitution):
+//         (file):(line):1: #if SYMBOL == 1  (#if 1 == 1)
+//     When included or nested in these top-level headers:
+//         (file list)
+//     This preprocessor directive is mismatched (the parentheses show the
+//              condition after macro substitution):
+//         (file):(line):1: #if SYMBOL == 1  (#if 2 == 1)
+//
+// error: header '(file)' has different contents depending on how it was
 //   included
 //
 // The latter might be followed by messages like the following:
@@ -53,20 +69,20 @@
 //
 // Some ideas:
 //
-// 1. Try to figure out the preprocessor conditional directives that
-// contribute to problems.
-//
-// 2. Check for correct and consistent usage of extern "C" {} and other
+// 1. Check for correct and consistent usage of extern "C" {} and other
 // directives. Warn about #include inside extern "C" {}.
 //
-// 3. What else?
+// 2. What else?
 //
-// General clean-up and refactoring:
+// General clean-up, refactoring, and fixing:
 //
 // 1. The Location class seems to be something that we might
 // want to design to be applicable to a wider range of tools, and stick it
 // somewhere into Tooling/ in mainline
 //
+// 2. The header preprocessing instance checking has some limitations that
+// need to be address.  It currently can only handle non-function-like macros.
+//
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTConsumer.h"
@@ -90,11 +106,31 @@
 #include <iterator>
 #include <string>
 #include <vector>
+#include "ModularizeMasterHeaderTracker.h"
+#include "ModularizePPCallbacks.h"
 
 using namespace clang::tooling;
 using namespace clang;
 using namespace llvm;
+using namespace Modularize;
 
+// Option to specify a prefix to be prepended to the header names.
+cl::opt<std::string> HeaderPrefix(
+    "prefix", cl::init(""),
+    cl::desc(
+        "Prepend header file paths with this prefix."
+        " If not specified,"
+        " the files are considered to be relative to the header list file."));
+
+// Option to disable the header preprocessing check.
+cl::opt<bool> NoPreprocessingCheck(
+    "no-preprocessing-check",
+    cl::desc(
+        "If this option is specified, no header preprocessing check"
+        " will be performed. This is a fallback in case there are problems"
+        " with the checking itself."),
+    cl::init(false));
+
 // Option to specify a file name for a list of header files to check.
 cl::opt<std::string>
 ListFileName(cl::Positional,
@@ -104,20 +140,12 @@
 cl::list<std::string> CC1Arguments(
     cl::ConsumeAfter, cl::desc("<arguments to be passed to front end>..."));
 
-// Option to specify a prefix to be prepended to the header names.
-cl::opt<std::string> HeaderPrefix(
-    "prefix", cl::init(""),
-    cl::desc(
-        "Prepend header file paths with this prefix."
-        " If not specified,"
-        " the files are considered to be relative to the header list file."));
-
 // Read the header list file and collect the header file names.
-error_code GetHeaderFileNames(SmallVectorImpl<std::string> &headerFileNames,
-                              StringRef listFileName, StringRef headerPrefix) {
+error_code getHeaderFileNames(SmallVectorImpl<std::string> &HeaderFileNames,
+                              StringRef ListFileName, StringRef headerPrefix) {
 
   // By default, use the path component of the list file name.
-  SmallString<256> headerDirectory(listFileName);
+  SmallString<256> headerDirectory(ListFileName);
   sys::path::remove_filename(headerDirectory);
 
   // Get the prefix if we have one.
@@ -142,16 +170,16 @@
     // Ignore comments and empty lines.
     if (line.empty() || (line[0] == '#'))
       continue;
-    SmallString<256> headerFileName;
+    SmallString<256> HeaderFileName;
     // Prepend header file name prefix if it's not absolute.
     if (sys::path::is_absolute(line))
-      headerFileName = line;
+      HeaderFileName = line;
     else {
-      headerFileName = headerDirectory;
-      sys::path::append(headerFileName, line);
+      HeaderFileName = headerDirectory;
+      sys::path::append(HeaderFileName, line);
     }
     // Save the resulting header file path.
-    headerFileNames.push_back(headerFileName.str());
+    HeaderFileNames.push_back(HeaderFileName.str());
   }
 
   return error_code::success();
@@ -380,8 +408,14 @@
 
 class CollectEntitiesConsumer : public ASTConsumer {
 public:
-  CollectEntitiesConsumer(EntityMap &Entities, Preprocessor &PP)
-      : Entities(Entities), PP(PP) {}
+  CollectEntitiesConsumer(
+	      EntityMap &Entities,
+		    ModularizeMasterHeaderTracker &masterHeaderTracker,
+		    Preprocessor &PP, StringRef InFile)
+      : Entities(Entities), MasterHeaderTracker(masterHeaderTracker), PP(PP) {
+    if (!masterHeaderTracker.isDisabled())
+      PP.addPPCallbacks(new ModularizePPCallbacks(PP, InFile, masterHeaderTracker));
+  }
 
   virtual void HandleTranslationUnit(ASTContext &Ctx) {
     SourceManager &SM = Ctx.getSourceManager();
@@ -406,30 +440,39 @@
   }
 private:
   EntityMap &Entities;
+  ModularizeMasterHeaderTracker &MasterHeaderTracker;
   Preprocessor &PP;
 };
 
 class CollectEntitiesAction : public SyntaxOnlyAction {
 public:
-  CollectEntitiesAction(EntityMap &Entities) : Entities(Entities) {}
+  CollectEntitiesAction(
+	  EntityMap &Entities, ModularizeMasterHeaderTracker &masterHeaderTracker)
+	: Entities(Entities), MasterHeaderTracker(masterHeaderTracker) {}
 protected:
   virtual clang::ASTConsumer *
   CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
-    return new CollectEntitiesConsumer(Entities, CI.getPreprocessor());
+    return new CollectEntitiesConsumer(
+		Entities, MasterHeaderTracker, CI.getPreprocessor(), InFile);
   }
 private:
   EntityMap &Entities;
+  ModularizeMasterHeaderTracker &MasterHeaderTracker;
 };
 
 class ModularizeFrontendActionFactory : public FrontendActionFactory {
 public:
-  ModularizeFrontendActionFactory(EntityMap &Entities) : Entities(Entities) {}
+  ModularizeFrontendActionFactory(
+	  EntityMap &Entities,
+	  ModularizeMasterHeaderTracker &masterHeaderTracker)
+	: Entities(Entities), MasterHeaderTracker(masterHeaderTracker) {}
 
   virtual CollectEntitiesAction *create() {
-    return new CollectEntitiesAction(Entities);
+    return new CollectEntitiesAction(Entities, MasterHeaderTracker);
   }
 private:
   EntityMap &Entities;
+  ModularizeMasterHeaderTracker &MasterHeaderTracker;
 };
 
 int main(int argc, const char **argv) {
@@ -445,7 +488,7 @@
 
   // Get header file names.
   SmallVector<std::string, 32> Headers;
-  if (error_code ec = GetHeaderFileNames(Headers, ListFileName, HeaderPrefix)) {
+  if (error_code ec = getHeaderFileNames(Headers, ListFileName, HeaderPrefix)) {
     errs() << argv[0] << ": error: Unable to get header list '" << ListFileName
            << "': " << ec.message() << '\n';
     return 1;
@@ -458,10 +501,14 @@
   Compilations.reset(
       new FixedCompilationDatabase(Twine(PathBuf), CC1Arguments));
 
+  // Create the master header tracker.
+  ModularizeMasterHeaderTracker MasterHeaderTracker(NoPreprocessingCheck);
+
   // Parse all of the headers, detecting duplicates.
   EntityMap Entities;
   ClangTool Tool(*Compilations, Headers);
-  int HadErrors = Tool.run(new ModularizeFrontendActionFactory(Entities));
+  int HadErrors = Tool.run(
+	  new ModularizeFrontendActionFactory(Entities, MasterHeaderTracker));
 
   // Create a place to save duplicate entity locations, separate bins per kind.
   typedef SmallVector<Location, 8> LocationArray;
@@ -509,6 +556,10 @@
     }
   }
 
+  // Report on header preprocessing mismatches.
+  if (!MasterHeaderTracker.report())
+	  HadErrors = 1;
+
   // Complain about any headers that have contents that differ based on how
   // they are included.
   // FIXME: Could we provide information about which preprocessor conditionals
@@ -524,7 +575,7 @@
 
     HadErrors = 1;
     errs() << "error: header '" << H->first->getName()
-           << "' has different contents dependening on how it was included\n";
+           << "' has different contents depending on how it was included\n";
     for (unsigned I = 0, N = H->second.size(); I != N; ++I) {
       errs() << "note: '" << H->second[I].Name << "' in " << H->second[I]
           .Loc.File->getName() << " at " << H->second[I].Loc.Line << ":"