From d3e7a2026c899e5364f3a8988fad2ee8d2478d23 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 21 Jul 2014 04:10:40 +0000 Subject: [PATCH] [modules] Fix some of the confusion when computing the override set for a macro introduced by finalization. This is still not entirely correct; more fixes to follow. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@213498 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ASTReader.cpp | 26 +++++++++---------- lib/Serialization/ASTWriter.cpp | 25 +++++++++--------- test/Modules/Inputs/macro-hiding/e1.h | 1 + test/Modules/Inputs/macro-hiding/e2.h | 2 ++ .../Inputs/macro-hiding/module.modulemap | 4 +++ test/Modules/macro-hiding.cpp | 12 ++++++++- test/Modules/macro-reexport/a1.h | 1 + test/Modules/macro-reexport/a2.h | 0 test/Modules/macro-reexport/b1.h | 0 test/Modules/macro-reexport/b2.h | 2 ++ test/Modules/macro-reexport/c1.h | 2 ++ test/Modules/macro-reexport/d1.h | 2 ++ test/Modules/macro-reexport/d2.h | 1 + .../Modules/macro-reexport/macro-reexport.cpp | 13 ++++++++++ test/Modules/macro-reexport/module.modulemap | 15 +++++++++++ 15 files changed, 79 insertions(+), 27 deletions(-) create mode 100644 test/Modules/Inputs/macro-hiding/e1.h create mode 100644 test/Modules/Inputs/macro-hiding/e2.h create mode 100644 test/Modules/macro-reexport/a1.h create mode 100644 test/Modules/macro-reexport/a2.h create mode 100644 test/Modules/macro-reexport/b1.h create mode 100644 test/Modules/macro-reexport/b2.h create mode 100644 test/Modules/macro-reexport/c1.h create mode 100644 test/Modules/macro-reexport/d1.h create mode 100644 test/Modules/macro-reexport/d2.h create mode 100644 test/Modules/macro-reexport/macro-reexport.cpp create mode 100644 test/Modules/macro-reexport/module.modulemap diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 419f6b3200..e3a16c3c5f 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -3323,8 +3323,7 @@ static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) { void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner, bool FromFinalization) { // FIXME: Only do this if Owner->NameVisibility == AllVisible. - for (unsigned I = 0, N = Names.HiddenDecls.size(); I != N; ++I) { - Decl *D = Names.HiddenDecls[I]; + for (Decl *D : Names.HiddenDecls) { bool wasHidden = D->Hidden; D->Hidden = false; @@ -3337,10 +3336,8 @@ void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner, assert((FromFinalization || Owner->NameVisibility >= Module::MacrosVisible) && "nothing to make visible?"); - for (HiddenMacrosMap::const_iterator I = Names.HiddenMacros.begin(), - E = Names.HiddenMacros.end(); - I != E; ++I) - installImportedMacro(I->first, I->second, Owner, FromFinalization); + for (const auto &Macro : Names.HiddenMacros) + installImportedMacro(Macro.first, Macro.second, Owner, FromFinalization); } void ASTReader::makeModuleVisible(Module *Mod, @@ -3374,9 +3371,12 @@ void ASTReader::makeModuleVisible(Module *Mod, // mark them as visible. HiddenNamesMapType::iterator Hidden = HiddenNamesMap.find(Mod); if (Hidden != HiddenNamesMap.end()) { - makeNamesVisible(Hidden->second, Hidden->first, - /*FromFinalization*/false); + auto HiddenNames = std::move(*Hidden); HiddenNamesMap.erase(Hidden); + makeNamesVisible(HiddenNames.second, HiddenNames.first, + /*FromFinalization*/false); + assert(HiddenNamesMap.find(Mod) == HiddenNamesMap.end() && + "making names visible added hidden names"); } // Push any exported modules onto the stack to be marked as visible. @@ -3899,12 +3899,12 @@ void ASTReader::InitializeContext() { } void ASTReader::finalizeForWriting() { - for (HiddenNamesMapType::iterator Hidden = HiddenNamesMap.begin(), - HiddenEnd = HiddenNamesMap.end(); - Hidden != HiddenEnd; ++Hidden) { - makeNamesVisible(Hidden->second, Hidden->first, /*FromFinalization*/true); + while (!HiddenNamesMap.empty()) { + auto HiddenNames = std::move(*HiddenNamesMap.begin()); + HiddenNamesMap.erase(HiddenNamesMap.begin()); + makeNamesVisible(HiddenNames.second, HiddenNames.first, + /*FromFinalization*/true); } - HiddenNamesMap.clear(); } /// \brief Given a cursor at the start of an AST file, scan ahead and drop the diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 9b44c54eb3..8293242603 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -3034,16 +3034,25 @@ class ASTIdentifierTableTrait { MacroDirective *getPublicSubmoduleMacro(MacroDirective *MD, SubmoduleID &ModID, OverriddenList &Overridden) { + Overridden.clear(); if (!MD) return nullptr; - Overridden.clear(); SubmoduleID OrigModID = ModID; Optional IsPublic; for (; MD; MD = MD->getPrevious()) { SubmoduleID ThisModID = getSubmoduleID(MD); if (ThisModID == 0) { IsPublic = Optional(); + + // If we have no directive location, this macro was installed when + // finalizing the ASTReader. + if (DefMacroDirective *DefMD = dyn_cast(MD)) + if (DefMD->getInfo()->getOwningModuleID()) + return MD; + // Skip imports that only produce #undefs for now. + // FIXME: We should still re-export them! + continue; } if (ThisModID != ModID) { @@ -3068,7 +3077,7 @@ class ASTIdentifierTableTrait { else SourceID = Writer.inferSubmoduleIDFromLocation(DefLoc); } - if (SourceID != OrigModID) + if (OrigModID && SourceID != OrigModID) Overridden.push_back(SourceID); } @@ -3095,17 +3104,7 @@ class ASTIdentifierTableTrait { } SubmoduleID getSubmoduleID(MacroDirective *MD) { - if (MD->getLocation().isValid()) - return Writer.inferSubmoduleIDFromLocation(MD->getLocation()); - - // If we have no directive location, this macro was installed when - // finalizing the ASTReader. - if (DefMacroDirective *DefMD = dyn_cast(MD)) - return DefMD->getInfo()->getOwningModuleID(); - - // Skip imports that only produce #undefs for now. - // FIXME: We should still re-export them! - return 0; + return Writer.inferSubmoduleIDFromLocation(MD->getLocation()); } public: diff --git a/test/Modules/Inputs/macro-hiding/e1.h b/test/Modules/Inputs/macro-hiding/e1.h new file mode 100644 index 0000000000..bd01708c5e --- /dev/null +++ b/test/Modules/Inputs/macro-hiding/e1.h @@ -0,0 +1 @@ +#include "a1.h" diff --git a/test/Modules/Inputs/macro-hiding/e2.h b/test/Modules/Inputs/macro-hiding/e2.h new file mode 100644 index 0000000000..f3a49c70ae --- /dev/null +++ b/test/Modules/Inputs/macro-hiding/e2.h @@ -0,0 +1,2 @@ +#include "a1.h" +inline void e1() { assert(true); } diff --git a/test/Modules/Inputs/macro-hiding/module.modulemap b/test/Modules/Inputs/macro-hiding/module.modulemap index 14ca9af86a..20632d35ae 100644 --- a/test/Modules/Inputs/macro-hiding/module.modulemap +++ b/test/Modules/Inputs/macro-hiding/module.modulemap @@ -12,3 +12,7 @@ module c { module d { module d1 { header "d1.h" export * } } +module e { + module e1 { header "e1.h" export * } + module e2 { header "e2.h" export * } +} diff --git a/test/Modules/macro-hiding.cpp b/test/Modules/macro-hiding.cpp index d0dadf319f..b166f4b194 100644 --- a/test/Modules/macro-hiding.cpp +++ b/test/Modules/macro-hiding.cpp @@ -62,6 +62,8 @@ // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/macro-hiding %s -DA1 -DA2 -DB1 -DB2 -DD1 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/macro-hiding %s -DA1 -DA2 -DB1 -DB2 -DC1 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/macro-hiding %s -DA1 -DA2 -DB1 -DB2 -DC1 -DD1 +// +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/macro-hiding %s -DE1 #ifdef A1 #include "a1.h" @@ -87,7 +89,15 @@ #include "d1.h" #endif -#if defined(A1) || defined(B2) || defined(C1) || defined(D1) +#ifdef E1 +#include "e1.h" +#endif + +#ifdef E2 +#include "e2.h" +#endif + +#if defined(A1) || defined(B2) || defined(C1) || defined(D1) || defined(E1) || defined(E2) void h() { assert(true); } #else void assert() {} diff --git a/test/Modules/macro-reexport/a1.h b/test/Modules/macro-reexport/a1.h new file mode 100644 index 0000000000..39933315f7 --- /dev/null +++ b/test/Modules/macro-reexport/a1.h @@ -0,0 +1 @@ +#define assert(x) a diff --git a/test/Modules/macro-reexport/a2.h b/test/Modules/macro-reexport/a2.h new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/Modules/macro-reexport/b1.h b/test/Modules/macro-reexport/b1.h new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/Modules/macro-reexport/b2.h b/test/Modules/macro-reexport/b2.h new file mode 100644 index 0000000000..26150458d3 --- /dev/null +++ b/test/Modules/macro-reexport/b2.h @@ -0,0 +1,2 @@ +#include "a2.h" +#define assert(x) b diff --git a/test/Modules/macro-reexport/c1.h b/test/Modules/macro-reexport/c1.h new file mode 100644 index 0000000000..d6a20e7419 --- /dev/null +++ b/test/Modules/macro-reexport/c1.h @@ -0,0 +1,2 @@ +#include "b1.h" +#define assert(x) c diff --git a/test/Modules/macro-reexport/d1.h b/test/Modules/macro-reexport/d1.h new file mode 100644 index 0000000000..fbd68d5de5 --- /dev/null +++ b/test/Modules/macro-reexport/d1.h @@ -0,0 +1,2 @@ +#include "c1.h" +#define assert(x) d diff --git a/test/Modules/macro-reexport/d2.h b/test/Modules/macro-reexport/d2.h new file mode 100644 index 0000000000..688f2d98a1 --- /dev/null +++ b/test/Modules/macro-reexport/d2.h @@ -0,0 +1 @@ +#include "b2.h" diff --git a/test/Modules/macro-reexport/macro-reexport.cpp b/test/Modules/macro-reexport/macro-reexport.cpp new file mode 100644 index 0000000000..47b15c2740 --- /dev/null +++ b/test/Modules/macro-reexport/macro-reexport.cpp @@ -0,0 +1,13 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fsyntax-only -DD2 -I. %s -fmodules-cache-path=%t -verify +// RUN: %clang_cc1 -fsyntax-only -DD2 -I. -fmodules %s -fmodules-cache-path=%t -verify +// RUN: %clang_cc1 -fsyntax-only -DC1 -I. %s -fmodules-cache-path=%t -verify +// RUN: %clang_cc1 -fsyntax-only -DC1 -I. -fmodules %s -fmodules-cache-path=%t -verify + +#ifdef D2 +#include "d2.h" +void f() { return assert(true); } // expected-error {{undeclared identifier 'b'}} +#else +#include "c1.h" +void f() { return assert(true); } // expected-error {{undeclared identifier 'c'}} +#endif diff --git a/test/Modules/macro-reexport/module.modulemap b/test/Modules/macro-reexport/module.modulemap new file mode 100644 index 0000000000..21585b692e --- /dev/null +++ b/test/Modules/macro-reexport/module.modulemap @@ -0,0 +1,15 @@ +module b { + module b2 { header "b2.h" export * } + module b1 { header "b1.h" export * } +} +module a { + module a1 { header "a1.h" export * } + module a2 { header "a2.h" export * } +} +module c { + module c1 { header "c1.h" export * } +} +module d { + module d1 { header "d1.h" export * } + module d2 { header "d2.h" export * } +} -- 2.40.0