r178109 - [modules] Before marking the module imported macros as ambiguous, check if this is a case where
Argyrios Kyrtzidis
akyrtzi at gmail.com
Tue Mar 26 18:25:35 PDT 2013
Author: akirtzidis
Date: Tue Mar 26 20:25:34 2013
New Revision: 178109
URL: http://llvm.org/viewvc/llvm-project?rev=178109&view=rev
Log:
[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.
Modified:
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTReader.cpp
Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=178109&r1=178108&r2=178109&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Tue Mar 26 20:25:34 2013
@@ -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.
///
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=178109&r1=178108&r2=178109&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Mar 26 20:25:34 2013
@@ -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);
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=178109&r1=178108&r2=178109&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Mar 26 20:25:34 2013
@@ -1481,8 +1481,9 @@ void ASTReader::resolvePendingMacro(Iden
// 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(Iden
}
if (!Hidden)
- installImportedMacro(II, MD);
+ installImportedMacro(II, MD, Owner);
}
void ASTReader::installPCHMacroDirectives(IdentifierInfo *II,
@@ -1561,17 +1562,62 @@ void ASTReader::installPCHMacroDirective
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
}
}
-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 H
}
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
// 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();
}
More information about the cfe-commits
mailing list