From 3a9c42c423b98b4857478c6200b5f81717f7b252 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Wed, 27 Mar 2013 01:25:34 +0000 Subject: [PATCH] [modules] Before marking the module imported 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 (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. llvm-svn: 178109 --- clang/include/clang/Lex/ModuleMap.h | 1 + clang/include/clang/Serialization/ASTReader.h | 5 +- clang/lib/Serialization/ASTReader.cpp | 68 ++++++++++++++++++++++----- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 33c92f5..1c9c673 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/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/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 246abcf..19d4918 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/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/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 649b4fc..55e333a 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/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(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 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(); } -- 2.7.4