[modules] Before marking the module imported macros as ambiguous, check if this is...

the system macro uses a not identical definition compared to a macro from the clang headers.

For example (these come from different modules):
   \#define LONG_MAX __LONG_MAX__ (clang's limits.h)
   \#define LONG_MAX 0x7fffffffffffffffL (system's limits.h)
in which case don't mark them ambiguous to avoid the "ambiguous macro expansion" warning.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178109 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Lex/ModuleMap.h b/include/clang/Lex/ModuleMap.h
index 33c92f59a44980de0b751a1c4a830a266a928fe6..1c9c67314ae047c8a502af82cd427d6296aa0513 100644 (file)
--- a/include/clang/Lex/ModuleMap.h
+++ b/include/clang/Lex/ModuleMap.h
@@ -177,6 +177,7 @@ public:
   void setBuiltinIncludeDir(const DirectoryEntry *Dir) {
     BuiltinIncludeDir = Dir;
   }
+  const DirectoryEntry *getBuiltinIncludeDir() { return BuiltinIncludeDir; }
 
   /// \brief Retrieve the module that owns the given header file, if any.
   ///

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 246abcf6d76b1bca38043b20e8e1314d8304f8a0..19d49183dd0ddeadd3536b85894777d10d58b6f7 100644 (file)
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -1206,7 +1206,7 @@ public:
                          bool Complain);
   
   /// \brief Make the names within this set of hidden names visible.
-  void makeNamesVisible(const HiddenNames &Names);
+  void makeNamesVisible(const HiddenNames &Names, Module *Owner);
   
   /// \brief Set the AST callbacks listener.
   void setListener(ASTReaderListener *listener) {
@@ -1629,7 +1629,8 @@ public:
   void installPCHMacroDirectives(IdentifierInfo *II,
                                  ModuleFile &M, uint64_t Offset);
 
-  void installImportedMacro(IdentifierInfo *II, MacroDirective *MD);
+  void installImportedMacro(IdentifierInfo *II, MacroDirective *MD,
+                            Module *Owner);
 
   /// \brief Retrieve the macro with the given ID.
   MacroInfo *getMacro(serialization::MacroID ID);

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 649b4fcdd43649306d1bc48ae4edc152f51547f5..55e333a17b6972103de76de9bc6941da06e5b832 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -1481,8 +1481,9 @@ void ASTReader::resolvePendingMacro(IdentifierInfo *II,
 
   // Determine whether this macro definition is visible.
   bool Hidden = false;
+  Module *Owner = 0;
   if (SubModID) {
-    if (Module *Owner = getSubmodule(SubModID)) {
+    if ((Owner = getSubmodule(SubModID))) {
       if (Owner->NameVisibility == Module::Hidden) {
         // The owning module is not visible, and this macro definition
         // should not be, either.
@@ -1496,7 +1497,7 @@ void ASTReader::resolvePendingMacro(IdentifierInfo *II,
   }
 
   if (!Hidden)
-    installImportedMacro(II, MD);
+    installImportedMacro(II, MD, Owner);
 }
 
 void ASTReader::installPCHMacroDirectives(IdentifierInfo *II,
@@ -1561,17 +1562,62 @@ void ASTReader::installPCHMacroDirectives(IdentifierInfo *II,
   PP.setLoadedMacroDirective(II, Latest);
 }
 
-void ASTReader::installImportedMacro(IdentifierInfo *II, MacroDirective *MD) {
+/// \brief For the given macro definitions, check if they are both in system
+/// modules and if one of the two is in the clang builtin headers.
+static bool isSystemAndClangMacro(MacroInfo *PrevMI, MacroInfo *NewMI,
+                                  Module *NewOwner, ASTReader &Reader) {
+  assert(PrevMI && NewMI);
+  if (!NewOwner)
+    return false;
+  Module *PrevOwner = 0;
+  if (SubmoduleID PrevModID = PrevMI->getOwningModuleID())
+    PrevOwner = Reader.getSubmodule(PrevModID);
+  if (!PrevOwner)
+    return false;
+  if (PrevOwner == NewOwner)
+    return false;
+  if (!PrevOwner->IsSystem || !NewOwner->IsSystem)
+    return false;
+
+  SourceManager &SM = Reader.getSourceManager();
+  FileID PrevFID = SM.getFileID(PrevMI->getDefinitionLoc());
+  FileID NewFID = SM.getFileID(NewMI->getDefinitionLoc());
+  const FileEntry *PrevFE = SM.getFileEntryForID(PrevFID);
+  const FileEntry *NewFE = SM.getFileEntryForID(NewFID);
+  if (PrevFE == 0 || NewFE == 0)
+    return false;
+
+  Preprocessor &PP = Reader.getPreprocessor();
+  ModuleMap &ModMap = PP.getHeaderSearchInfo().getModuleMap();
+  const DirectoryEntry *BuiltinDir = ModMap.getBuiltinIncludeDir();
+
+  return (PrevFE->getDir() == BuiltinDir) != (NewFE->getDir() == BuiltinDir);
+}
+
+void ASTReader::installImportedMacro(IdentifierInfo *II, MacroDirective *MD,
+                                     Module *Owner) {
   assert(II && MD);
 
   DefMacroDirective *DefMD = cast<DefMacroDirective>(MD);
   MacroDirective *Prev = PP.getMacroDirective(II);
   if (Prev) {
     MacroDirective::DefInfo PrevDef = Prev->getDefinition();
-    if (DefMD->getInfo() != PrevDef.getMacroInfo() &&
-        !PrevDef.getMacroInfo()->isIdenticalTo(*DefMD->getInfo(), PP)) {
-      PrevDef.getDirective()->setAmbiguous(true);
-      DefMD->setAmbiguous(true);
+    MacroInfo *PrevMI = PrevDef.getMacroInfo();
+    MacroInfo *NewMI = DefMD->getInfo();
+    if (NewMI != PrevMI && !PrevMI->isIdenticalTo(*NewMI, PP)) {
+      // Before marking the macros as ambiguous, check if this is a case where
+      // the system macro uses a not identical definition compared to a macro
+      // from the clang headers. For example:
+      //   #define LONG_MAX __LONG_MAX__ (clang's limits.h)
+      //   #define LONG_MAX 0x7fffffffffffffffL (system's limits.h)
+      // in which case don't mark them to avoid the "ambiguous macro expansion"
+      // warning.
+      // FIXME: This should go away if the system headers get "fixed" to use
+      // identical definitions.
+      if (!isSystemAndClangMacro(PrevMI, NewMI, Owner, *this)) {
+        PrevDef.getDirective()->setAmbiguous(true);
+        DefMD->setAmbiguous(true);
+      }
     }
   }
   
@@ -2718,7 +2764,7 @@ static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) {
   }
 }
 
-void ASTReader::makeNamesVisible(const HiddenNames &Names) {
+void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner) {
   for (unsigned I = 0, N = Names.size(); I != N; ++I) {
     switch (Names[I].getKind()) {
     case HiddenName::Declaration: {
@@ -2735,7 +2781,7 @@ void ASTReader::makeNamesVisible(const HiddenNames &Names) {
     }
     case HiddenName::MacroVisibility: {
       std::pair<IdentifierInfo *, MacroDirective *> Macro = Names[I].getMacro();
-      installImportedMacro(Macro.first, Macro.second);
+      installImportedMacro(Macro.first, Macro.second, Owner);
       break;
     }
     }
@@ -2771,7 +2817,7 @@ void ASTReader::makeModuleVisible(Module *Mod,
     // mark them as visible.
     HiddenNamesMapType::iterator Hidden = HiddenNamesMap.find(Mod);
     if (Hidden != HiddenNamesMap.end()) {
-      makeNamesVisible(Hidden->second);
+      makeNamesVisible(Hidden->second, Hidden->first);
       HiddenNamesMap.erase(Hidden);
     }
     
@@ -3266,7 +3312,7 @@ void ASTReader::finalizeForWriting() {
   for (HiddenNamesMapType::iterator Hidden = HiddenNamesMap.begin(),
                                  HiddenEnd = HiddenNamesMap.end();
        Hidden != HiddenEnd; ++Hidden) {
-    makeNamesVisible(Hidden->second);
+    makeNamesVisible(Hidden->second, Hidden->first);
   }
   HiddenNamesMap.clear();
 }