From dbbc5236395171104128620dd4aed4084ba92c68 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 14 May 2015 02:25:44 +0000 Subject: [PATCH] [modules] Rearrange preprocessor module visibility handling, no observable change intended. llvm-svn: 237331 --- clang/lib/Lex/PPDirectives.cpp | 63 +++++++++++++++++++++------------ clang/lib/Lex/PPLexerChange.cpp | 17 +++++---- clang/lib/Lex/Preprocessor.cpp | 2 +- 3 files changed, 51 insertions(+), 31 deletions(-) diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index d1c890627615..a41ed8a1b489 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -1439,6 +1439,8 @@ bool Preprocessor::ConcatenateIncludeName(SmallString<128> &FilenameBuffer, static void EnterAnnotationToken(Preprocessor &PP, SourceLocation Begin, SourceLocation End, tok::TokenKind Kind, void *AnnotationVal) { + // FIXME: Produce this as the current token directly, rather than + // allocating a new token for it. Token *Tok = new Token[1]; Tok[0].startToken(); Tok[0].setKind(Kind); @@ -1558,8 +1560,8 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr, HeaderInfo.getHeaderSearchOpts().ModuleMaps ? &SuggestedModule : nullptr); - if (Callbacks) { - if (!File) { + if (!File) { + if (Callbacks) { // Give the clients a chance to recover. SmallString<128> RecoveryPath; if (Callbacks->FileNotFound(Filename, RecoveryPath)) { @@ -1579,18 +1581,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, } } } - - if (!SuggestedModule || !getLangOpts().Modules) { - // Notify the callback object that we've seen an inclusion directive. - Callbacks->InclusionDirective(HashLoc, IncludeTok, - LangOpts.MSVCCompat ? NormalizedPath.c_str() - : Filename, - isAngled, FilenameRange, File, SearchPath, - RelativePath, /*ImportedModule=*/nullptr); - } - } - if (!File) { if (!SuppressIncludeNotFoundError) { // If the file could not be located and it was included via angle // brackets, we can attempt a lookup as though it were a quoted path to @@ -1611,19 +1602,34 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename; } - if (!File) - return; } + bool IsModuleImport = SuggestedModule && getLangOpts().Modules && + SuggestedModule.getModule()->getTopLevelModuleName() != + getLangOpts().ImplementationOfModule; + + if (Callbacks && !IsModuleImport) { + // Notify the callback object that we've seen an inclusion directive. + // For a module import, we delay this until we've loaded the module. + // FIXME: Why? + Callbacks->InclusionDirective(HashLoc, IncludeTok, + LangOpts.MSVCCompat ? NormalizedPath.c_str() + : Filename, + isAngled, FilenameRange, File, SearchPath, + RelativePath, /*ImportedModule=*/nullptr); + } + + if (!File) + return; + // If we are supposed to import a module rather than including the header, // do so now. - if (SuggestedModule && getLangOpts().Modules && - SuggestedModule.getModule()->getTopLevelModuleName() != - getLangOpts().ImplementationOfModule) { + if (IsModuleImport) { // Compute the module access path corresponding to this module. // FIXME: Should we have a second loadModule() overload to avoid this // extra lookup step? @@ -1682,11 +1688,11 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, // Load the module to import its macros. We'll make the declarations // visible when the parser gets here. + // FIXME: Pass SuggestedModule in here rather than converting it to a path + // and making the module loader convert it back again. ModuleLoadResult Imported = TheModuleLoader.loadModule( IncludeTok.getLocation(), Path, Module::Hidden, /*IsIncludeDirective=*/true); - if (Imported) - makeModuleVisible(Imported, IncludeTok.getLocation()); assert((Imported == nullptr || Imported == SuggestedModule.getModule()) && "the imported module is different than the suggested one"); @@ -1712,11 +1718,12 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, SearchPath, RelativePath, Imported); } + // Make the macros from this module visible now. + makeModuleVisible(Imported, IncludeTok.getLocation()); + if (IncludeKind != 3) { // Let the parser know that we hit a module import, and it should // make the module visible. - // FIXME: Produce this as the current token directly, rather than - // allocating a new token for it. EnterAnnotationToken(*this, HashLoc, End, tok::annot_module_include, Imported); } @@ -1731,7 +1738,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, } } - if (Callbacks && SuggestedModule) { + if (Callbacks && IsModuleImport) { // We didn't notify the callback object that we've seen an inclusion // directive before. Now that we are parsing the include normally and not // turning it to a module import, notify the callback object. @@ -1753,6 +1760,16 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, if (!HeaderInfo.ShouldEnterIncludeFile(*this, File, isImport)) { if (Callbacks) Callbacks->FileSkipped(*File, FilenameTok, FileCharacter); + + // If this is a module import, make it visible if needed. + if (IsModuleImport) { + makeModuleVisible(SuggestedModule.getModule(), HashLoc); + + if (IncludeTok.getIdentifierInfo()->getPPKeywordID() != + tok::pp___include_macros) + EnterAnnotationToken(*this, HashLoc, End, tok::annot_module_include, + SuggestedModule.getModule()); + } return; } diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp index a27bcd000a40..4af68b0b80a3 100644 --- a/clang/lib/Lex/PPLexerChange.cpp +++ b/clang/lib/Lex/PPLexerChange.cpp @@ -636,6 +636,9 @@ void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc) { void Preprocessor::LeaveSubmodule() { auto &Info = BuildingSubmoduleStack.back(); + Module *LeavingMod = Info.M; + SourceLocation ImportLoc = Info.ImportLoc; + // Create ModuleMacros for any macros defined in this submodule. for (auto &Macro : Macros) { auto *II = const_cast(Macro.first); @@ -665,7 +668,7 @@ void Preprocessor::LeaveSubmodule() { // visibility, since there can be no such directives in our list. if (!getLangOpts().ModulesLocalVisibility) { Module *Mod = getModuleContainingLocation(MD->getLocation()); - if (Mod != Info.M) + if (Mod != LeavingMod) break; } @@ -688,8 +691,8 @@ void Preprocessor::LeaveSubmodule() { // Don't bother creating a module macro if it would represent a #undef // that doesn't override anything. if (Def || !Macro.second.getOverriddenMacros().empty()) - addModuleMacro(Info.M, II, Def, Macro.second.getOverriddenMacros(), - IsNew); + addModuleMacro(LeavingMod, II, Def, + Macro.second.getOverriddenMacros(), IsNew); break; } } @@ -706,9 +709,9 @@ void Preprocessor::LeaveSubmodule() { if (getLangOpts().ModulesLocalVisibility) VisibleModules = std::move(Info.VisibleModules); - // A nested #include makes the included submodule visible. - if (BuildingSubmoduleStack.size() > 1) - makeModuleVisible(Info.M, Info.ImportLoc); - BuildingSubmoduleStack.pop_back(); + + // A nested #include makes the included submodule visible. + if (!BuildingSubmoduleStack.empty() || !getLangOpts().ModulesLocalVisibility) + makeModuleVisible(LeavingMod, ImportLoc); } diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp index 21dccd127bf8..87e6b5251c38 100644 --- a/clang/lib/Lex/Preprocessor.cpp +++ b/clang/lib/Lex/Preprocessor.cpp @@ -775,7 +775,7 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) { }); // Add this module to the imports list of the currently-built submodule. - if (!BuildingSubmoduleStack.empty()) + if (!BuildingSubmoduleStack.empty() && M != BuildingSubmoduleStack.back().M) BuildingSubmoduleStack.back().M->Imports.insert(M); }