From: Richard Smith Date: Thu, 21 May 2015 01:20:10 +0000 (+0000) Subject: [modules] If we re-enter a submodule from within itself (when submodule X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e450a51e6598edd1a633100622a988f1576e7b16;p=clang [modules] If we re-enter a submodule from within itself (when submodule visibility is enabled) or leave and re-enter it, restore the macro and module visibility state from last time we were in that submodule. This allows mutually-#including header files to stand a chance at being modularized with local visibility enabled. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@237871 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index 4562741658..ea15dbdf21 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -391,7 +391,7 @@ class Preprocessor : public RefCountedBase { // FIXME: Find a spare bit on IdentifierInfo and store a // HasModuleMacros flag. if (!II->hasMacroDefinition() || !PP.getLangOpts().Modules || - !PP.VisibleModules.getGeneration()) + !PP.CurSubmoduleState->VisibleModules.getGeneration()) return nullptr; auto *Info = State.dyn_cast(); @@ -401,7 +401,7 @@ class Preprocessor : public RefCountedBase { State = Info; } - if (PP.VisibleModules.getGeneration() != + if (PP.CurSubmoduleState->VisibleModules.getGeneration() != Info->ActiveModuleMacrosGeneration) PP.updateModuleMacroInfo(II, *Info); return Info; @@ -483,33 +483,50 @@ class Preprocessor : public RefCountedBase { } }; - typedef llvm::DenseMap MacroMap; - /// For each IdentifierInfo that was associated with a macro, we /// keep a mapping to the history of all macro definitions and #undefs in /// the reverse order (the latest one is in the head of the list). - MacroMap Macros; + /// + /// This mapping lives within the \p CurSubmoduleState. + typedef llvm::DenseMap MacroMap; friend class ASTReader; + struct SubmoduleState; + /// \brief Information about a submodule that we're currently building. struct BuildingSubmoduleInfo { - BuildingSubmoduleInfo(Module *M, SourceLocation ImportLoc) - : M(M), ImportLoc(ImportLoc) {} + BuildingSubmoduleInfo(Module *M, SourceLocation ImportLoc, + SubmoduleState *OuterSubmoduleState) + : M(M), ImportLoc(ImportLoc), OuterSubmoduleState(OuterSubmoduleState) { + } /// The module that we are building. Module *M; /// The location at which the module was included. SourceLocation ImportLoc; - /// The macros that were visible before we entered the module. + /// The previous SubmoduleState. + SubmoduleState *OuterSubmoduleState; + }; + SmallVector BuildingSubmoduleStack; + + /// \brief Information about a submodule's preprocessor state. + struct SubmoduleState { + /// The macros for the submodule. MacroMap Macros; - /// The set of modules that was visible in the surrounding submodule. + /// The set of modules that are visible within the submodule. VisibleModuleSet VisibleModules; - // FIXME: CounterValue? // FIXME: PragmaPushMacroInfo? }; - SmallVector BuildingSubmoduleStack; + std::map Submodules; + + /// The preprocessor state for preprocessing outside of any submodule. + SubmoduleState NullSubmoduleState; + + /// The current submodule state. Will be \p NullSubmoduleState if we're not + /// in a submodule. + SubmoduleState *CurSubmoduleState; /// The set of known macros exported from modules. llvm::FoldingSet ModuleMacros; @@ -519,9 +536,6 @@ class Preprocessor : public RefCountedBase { llvm::DenseMap> LeafModuleMacros; - /// The current set of visible modules. - VisibleModuleSet VisibleModules; - /// \brief Macros that we want to warn because they are not used at the end /// of the translation unit. /// @@ -767,7 +781,7 @@ public: if (!II->hasMacroDefinition()) return MacroDefinition(); - MacroState &S = Macros[II]; + MacroState &S = CurSubmoduleState->Macros[II]; auto *MD = S.getLatest(); while (MD && isa(MD)) MD = MD->getPrevious(); @@ -781,7 +795,7 @@ public: if (!II->hadMacroDefinition()) return MacroDefinition(); - MacroState &S = Macros[II]; + MacroState &S = CurSubmoduleState->Macros[II]; MacroDirective::DefInfo DI; if (auto *MD = S.getLatest()) DI = MD->findDirectiveAtLoc(Loc, getSourceManager()); @@ -1018,7 +1032,7 @@ public: void makeModuleVisible(Module *M, SourceLocation Loc); SourceLocation getModuleImportLoc(Module *M) const { - return VisibleModules.getImportLoc(M); + return CurSubmoduleState->VisibleModules.getImportLoc(M); } /// \brief Lex a string literal, which may be the concatenation of multiple diff --git a/lib/Lex/PPLexerChange.cpp b/lib/Lex/PPLexerChange.cpp index f1d0593c8e..027daae4be 100644 --- a/lib/Lex/PPLexerChange.cpp +++ b/lib/Lex/PPLexerChange.cpp @@ -610,39 +610,51 @@ void Preprocessor::HandleMicrosoftCommentPaste(Token &Tok) { } void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc) { - // Save the current state for future imports. - BuildingSubmoduleStack.push_back(BuildingSubmoduleInfo(M, ImportLoc)); - auto &Info = BuildingSubmoduleStack.back(); - Info.Macros.swap(Macros); - // Save our visible modules set. This is guaranteed to clear the set. - if (getLangOpts().ModulesLocalVisibility) { - Info.VisibleModules = std::move(VisibleModules); - - // Resolve as much of the module definition as we can now, before we enter - // one if its headers. - // FIXME: Can we enable Complain here? - ModuleMap &ModMap = getHeaderSearchInfo().getModuleMap(); - ModMap.resolveExports(M, /*Complain=*/false); - ModMap.resolveUses(M, /*Complain=*/false); - ModMap.resolveConflicts(M, /*Complain=*/false); - - // This module is visible to itself. - makeModuleVisible(M, ImportLoc); + if (!getLangOpts().ModulesLocalVisibility) { + // Just track that we entered this submodule. + BuildingSubmoduleStack.push_back( + BuildingSubmoduleInfo(M, ImportLoc, CurSubmoduleState)); + return; } - // Determine the set of starting macros for this submodule. - // FIXME: If we re-enter a submodule, should we restore its MacroDirectives? - auto &StartingMacros = getLangOpts().ModulesLocalVisibility - ? BuildingSubmoduleStack[0].Macros - : Info.Macros; - - // Restore to the starting state. - // FIXME: Do this lazily, when each macro name is first referenced. - for (auto &Macro : StartingMacros) { - MacroState MS(Macro.second.getLatest()); - MS.setOverriddenMacros(*this, Macro.second.getOverriddenMacros()); - Macros.insert(std::make_pair(Macro.first, std::move(MS))); + // Resolve as much of the module definition as we can now, before we enter + // one of its headers. + // FIXME: Can we enable Complain here? + // FIXME: Can we do this when local visibility is disabled? + ModuleMap &ModMap = getHeaderSearchInfo().getModuleMap(); + ModMap.resolveExports(M, /*Complain=*/false); + ModMap.resolveUses(M, /*Complain=*/false); + ModMap.resolveConflicts(M, /*Complain=*/false); + + // If this is the first time we've entered this module, set up its state. + auto R = Submodules.emplace(std::piecewise_construct, std::make_tuple(M), + std::make_tuple()); + auto &State = R.first->second; + bool FirstTime = R.second; + if (FirstTime) { + // Determine the set of starting macros for this submodule; take these + // from the "null" module (the predefines buffer). + auto &StartingMacros = NullSubmoduleState.Macros; + + // Restore to the starting state. + // FIXME: Do this lazily, when each macro name is first referenced. + for (auto &Macro : StartingMacros) { + MacroState MS(Macro.second.getLatest()); + MS.setOverriddenMacros(*this, Macro.second.getOverriddenMacros()); + State.Macros.insert(std::make_pair(Macro.first, std::move(MS))); + } } + + // Track that we entered this module. + BuildingSubmoduleStack.push_back( + BuildingSubmoduleInfo(M, ImportLoc, CurSubmoduleState)); + + // Switch to this submodule as the current submodule. + CurSubmoduleState = &State; + + // This module is visible to itself. + if (FirstTime) + makeModuleVisible(M, ImportLoc); } void Preprocessor::LeaveSubmodule() { @@ -652,15 +664,15 @@ void Preprocessor::LeaveSubmodule() { SourceLocation ImportLoc = Info.ImportLoc; // Create ModuleMacros for any macros defined in this submodule. - for (auto &Macro : Macros) { + for (auto &Macro : CurSubmoduleState->Macros) { auto *II = const_cast(Macro.first); - auto &OuterInfo = Info.Macros[II]; // Find the starting point for the MacroDirective chain in this submodule. - auto *OldMD = OuterInfo.getLatest(); - if (getLangOpts().ModulesLocalVisibility && - BuildingSubmoduleStack.size() > 1) { - auto &PredefMacros = BuildingSubmoduleStack[0].Macros; + MacroDirective *OldMD = nullptr; + if (getLangOpts().ModulesLocalVisibility) { + // FIXME: It'd be better to start at the state from when we most recently + // entered this submodule, but it doesn't really matter. + auto &PredefMacros = NullSubmoduleState.Macros; auto PredefMacroIt = PredefMacros.find(Macro.first); if (PredefMacroIt == PredefMacros.end()) OldMD = nullptr; @@ -708,23 +720,15 @@ void Preprocessor::LeaveSubmodule() { break; } } - - // Maintain a single macro directive chain if we're not tracking - // per-submodule macro visibility. - if (!getLangOpts().ModulesLocalVisibility) - OuterInfo.setLatest(Macro.second.getLatest()); } - // Put back the old macros. - std::swap(Info.Macros, Macros); - + // Put back the outer module's state, if we're tracking it. if (getLangOpts().ModulesLocalVisibility) - VisibleModules = std::move(Info.VisibleModules); + CurSubmoduleState = Info.OuterSubmoduleState; BuildingSubmoduleStack.pop_back(); // A nested #include makes the included submodule visible. - if (!BuildingSubmoduleStack.empty() || - !getLangOpts().ModulesLocalVisibility) + if (!BuildingSubmoduleStack.empty() || !getLangOpts().ModulesLocalVisibility) makeModuleVisible(LeavingMod, ImportLoc); } diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp index aa4e8b6764..9046ad51c1 100644 --- a/lib/Lex/PPMacroExpansion.cpp +++ b/lib/Lex/PPMacroExpansion.cpp @@ -37,15 +37,16 @@ MacroDirective * Preprocessor::getLocalMacroDirectiveHistory(const IdentifierInfo *II) const { if (!II->hadMacroDefinition()) return nullptr; - auto Pos = Macros.find(II); - return Pos == Macros.end() ? nullptr : Pos->second.getLatest(); + auto Pos = CurSubmoduleState->Macros.find(II); + return Pos == CurSubmoduleState->Macros.end() ? nullptr + : Pos->second.getLatest(); } void Preprocessor::appendMacroDirective(IdentifierInfo *II, MacroDirective *MD){ assert(MD && "MacroDirective should be non-zero!"); assert(!MD->getPrevious() && "Already attached to a MacroDirective history."); - MacroState &StoredMD = Macros[II]; + MacroState &StoredMD = CurSubmoduleState->Macros[II]; auto *OldMD = StoredMD.getLatest(); MD->setPrevious(OldMD); StoredMD.setLatest(MD); @@ -62,7 +63,7 @@ void Preprocessor::appendMacroDirective(IdentifierInfo *II, MacroDirective *MD){ void Preprocessor::setLoadedMacroDirective(IdentifierInfo *II, MacroDirective *MD) { assert(II && MD); - MacroState &StoredMD = Macros[II]; + MacroState &StoredMD = CurSubmoduleState->Macros[II]; assert(!StoredMD.getLatest() && "the macro history was modified before initializing it from a pch"); StoredMD = MD; @@ -124,9 +125,11 @@ ModuleMacro *Preprocessor::getModuleMacro(Module *Mod, IdentifierInfo *II) { void Preprocessor::updateModuleMacroInfo(const IdentifierInfo *II, ModuleMacroInfo &Info) { - assert(Info.ActiveModuleMacrosGeneration != VisibleModules.getGeneration() && + assert(Info.ActiveModuleMacrosGeneration != + CurSubmoduleState->VisibleModules.getGeneration() && "don't need to update this macro name info"); - Info.ActiveModuleMacrosGeneration = VisibleModules.getGeneration(); + Info.ActiveModuleMacrosGeneration = + CurSubmoduleState->VisibleModules.getGeneration(); auto Leaf = LeafModuleMacros.find(II); if (Leaf == LeafModuleMacros.end()) { @@ -146,7 +149,7 @@ void Preprocessor::updateModuleMacroInfo(const IdentifierInfo *II, Leaf->second.end()); while (!Worklist.empty()) { auto *MM = Worklist.pop_back_val(); - if (VisibleModules.isVisible(MM->getOwningModule())) { + if (CurSubmoduleState->VisibleModules.isVisible(MM->getOwningModule())) { // We only care about collecting definitions; undefinitions only act // to override other definitions. if (MM->getMacroInfo()) @@ -200,8 +203,8 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) { if (LeafIt != LeafModuleMacros.end()) Leaf = LeafIt->second; const MacroState *State = nullptr; - auto Pos = Macros.find(II); - if (Pos != Macros.end()) + auto Pos = CurSubmoduleState->Macros.find(II); + if (Pos != CurSubmoduleState->Macros.end()) State = &Pos->second; llvm::errs() << "MacroState " << State << " " << II->getNameStart(); @@ -236,7 +239,8 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) { if (Active.count(MM)) llvm::errs() << " active"; - else if (!VisibleModules.isVisible(MM->getOwningModule())) + else if (!CurSubmoduleState->VisibleModules.isVisible( + MM->getOwningModule())) llvm::errs() << " hidden"; else if (MM->getMacroInfo()) llvm::errs() << " overridden"; diff --git a/lib/Lex/Preprocessor.cpp b/lib/Lex/Preprocessor.cpp index 9e89497597..7e33f1ccb4 100644 --- a/lib/Lex/Preprocessor.cpp +++ b/lib/Lex/Preprocessor.cpp @@ -73,7 +73,8 @@ Preprocessor::Preprocessor(IntrusiveRefCntPtr PPOpts, ModuleImportExpectsIdentifier(false), CodeCompletionReached(0), MainFileDir(nullptr), SkipMainFilePreamble(0, true), CurPPLexer(nullptr), CurDirLookup(nullptr), CurLexerKind(CLK_Lexer), CurSubmodule(nullptr), - Callbacks(nullptr), MacroArgCache(nullptr), Record(nullptr), + Callbacks(nullptr), CurSubmoduleState(&NullSubmoduleState), + MacroArgCache(nullptr), Record(nullptr), MIChainHead(nullptr), DeserialMIChainHead(nullptr) { OwnsHeaderSearch = OwnsHeaders; @@ -266,7 +267,9 @@ void Preprocessor::PrintStats() { llvm::errs() << "\n Macro Expanded Tokens: " << llvm::capacity_in_bytes(MacroExpandedTokens); llvm::errs() << "\n Predefines Buffer: " << Predefines.capacity(); - llvm::errs() << "\n Macros: " << llvm::capacity_in_bytes(Macros); + // FIXME: List information for all submodules. + llvm::errs() << "\n Macros: " + << llvm::capacity_in_bytes(CurSubmoduleState->Macros); llvm::errs() << "\n #pragma push_macro Info: " << llvm::capacity_in_bytes(PragmaPushMacroInfo); llvm::errs() << "\n Poison Reasons: " @@ -283,14 +286,16 @@ Preprocessor::macro_begin(bool IncludeExternalMacros) const { ExternalSource->ReadDefinedMacros(); } - return Macros.begin(); + return CurSubmoduleState->Macros.begin(); } size_t Preprocessor::getTotalMemory() const { return BP.getTotalMemory() + llvm::capacity_in_bytes(MacroExpandedTokens) + Predefines.capacity() /* Predefines buffer. */ - + llvm::capacity_in_bytes(Macros) + // FIXME: Include sizes from all submodules, and include MacroInfo sizes, + // and ModuleMacros. + + llvm::capacity_in_bytes(CurSubmoduleState->Macros) + llvm::capacity_in_bytes(PragmaPushMacroInfo) + llvm::capacity_in_bytes(PoisonReasons) + llvm::capacity_in_bytes(CommentHandlers); @@ -304,7 +309,7 @@ Preprocessor::macro_end(bool IncludeExternalMacros) const { ExternalSource->ReadDefinedMacros(); } - return Macros.end(); + return CurSubmoduleState->Macros.end(); } /// \brief Compares macro tokens with a specified token value sequence. @@ -781,7 +786,7 @@ void Preprocessor::LexAfterModuleImport(Token &Result) { } void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) { - VisibleModules.setVisible( + CurSubmoduleState->VisibleModules.setVisible( M, Loc, [](Module *) {}, [&](ArrayRef Path, Module *Conflict, StringRef Message) { // FIXME: Include the path in the diagnostic. diff --git a/test/Modules/Inputs/submodule-visibility/cycle1.h b/test/Modules/Inputs/submodule-visibility/cycle1.h new file mode 100644 index 0000000000..05e85aef10 --- /dev/null +++ b/test/Modules/Inputs/submodule-visibility/cycle1.h @@ -0,0 +1,8 @@ +#ifndef CYCLE1 +#define CYCLE1 + +#include "cycle2.h" + +struct C1 {}; + +#endif diff --git a/test/Modules/Inputs/submodule-visibility/cycle2.h b/test/Modules/Inputs/submodule-visibility/cycle2.h new file mode 100644 index 0000000000..de9fd8e01a --- /dev/null +++ b/test/Modules/Inputs/submodule-visibility/cycle2.h @@ -0,0 +1,8 @@ +#ifndef CYCLE2 +#define CYCLE2 + +#include "cycle1.h" + +struct C2 {}; + +#endif diff --git a/test/Modules/Inputs/submodule-visibility/module.modulemap b/test/Modules/Inputs/submodule-visibility/module.modulemap index 447a1f42d4..2e13344dc6 100644 --- a/test/Modules/Inputs/submodule-visibility/module.modulemap +++ b/test/Modules/Inputs/submodule-visibility/module.modulemap @@ -1 +1,6 @@ module x { module a { header "a.h" } module b { header "b.h" } } + +module cycles { + module cycle1 { header "cycle1.h" } + module cycle2 { header "cycle2.h" } +} diff --git a/test/Modules/submodule-visibility-cycles.cpp b/test/Modules/submodule-visibility-cycles.cpp new file mode 100644 index 0000000000..fca8df9f77 --- /dev/null +++ b/test/Modules/submodule-visibility-cycles.cpp @@ -0,0 +1,10 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -fmodules-local-submodule-visibility -fmodules-cache-path=%t -I%S/Inputs/submodule-visibility -verify %s + +#include "cycle1.h" +C1 c1; +C2 c2; // expected-error {{must be imported}} expected-error {{}} +// expected-note@cycle2.h:6 {{here}} + +#include "cycle2.h" +C2 c3;