Author: rsmith Date: Thu Mar 21 12:44:17 2019 New Revision: 356704 URL: http://llvm.org/viewvc/llvm-project?rev=356704&view=rev Log: Refactor handling of #include directives to cleanly separate the "skipped header because it should be imported as a module" cases from the "skipped header because of some other reason" cases.
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp Modified: cfe/trunk/lib/Lex/PPDirectives.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=356704&r1=356703&r2=356704&view=diff ============================================================================== --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) +++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Mar 21 12:44:17 2019 @@ -1813,26 +1813,26 @@ void Preprocessor::HandleIncludeDirectiv return; } - // Should we enter the source file? Set to false if either the source file is + // Should we enter the source file? Set to Skip if either the source file is // known to have no effect beyond its effect on module visibility -- that is, - // if it's got an include guard that is already defined or is a modular header - // we've imported or already built. - bool ShouldEnter = true; + // if it's got an include guard that is already defined, set to Import if it + // is a modular header we've already built and should import. + enum { Enter, Import, Skip, IncludeLimitReached } Action = Enter; if (PPOpts->SingleFileParseMode) - ShouldEnter = false; + Action = IncludeLimitReached; // If we've reached the max allowed include depth, it is usually due to an // include cycle. Don't enter already processed files again as it can lead to // reaching the max allowed include depth again. - if (ShouldEnter && HasReachedMaxIncludeDepth && File && + if (Action == Enter && HasReachedMaxIncludeDepth && File && HeaderInfo.getFileInfo(File).NumIncludes) - ShouldEnter = false; + Action = IncludeLimitReached; // Determine whether we should try to import the module for this #include, if // there is one. Don't do so if precompiled module support is disabled or we // are processing this module textually (because we're building the module). - if (ShouldEnter && File && SuggestedModule && getLangOpts().Modules && + if (Action == Enter && File && SuggestedModule && getLangOpts().Modules && !isForModuleBuilding(SuggestedModule.getModule(), getLangOpts().CurrentModule, getLangOpts().ModuleName)) { @@ -1872,9 +1872,9 @@ void Preprocessor::HandleIncludeDirectiv assert((Imported == nullptr || Imported == SuggestedModule.getModule()) && "the imported module is different than the suggested one"); - if (Imported) - ShouldEnter = false; - else if (Imported.isMissingExpected()) { + if (Imported) { + Action = Import; + } else if (Imported.isMissingExpected()) { // We failed to find a submodule that we assumed would exist (because it // was in the directory of an umbrella header, for instance), but no // actual module containing it exists (because the umbrella header is @@ -1907,13 +1907,18 @@ void Preprocessor::HandleIncludeDirectiv // Ask HeaderInfo if we should enter this #include file. If not, #including // this file will have no effect. - bool SkipHeader = false; - if (ShouldEnter && File && + if (Action == Enter && File && !HeaderInfo.ShouldEnterIncludeFile(*this, File, isImport, getLangOpts().Modules, SuggestedModule.getModule())) { - ShouldEnter = false; - SkipHeader = true; + // Even if we've already preprocessed this header once and know that we + // don't need to see its contents again, we still need to import it if it's + // modular because we might not have imported it from this submodule before. + // + // FIXME: We don't do this when compiling a PCH because the AST + // serialization layer can't cope with it. This means we get local + // submodule visibility semantics wrong in that case. + Action = (SuggestedModule && !getLangOpts().CompilingPCH) ? Import : Skip; } if (Callbacks) { @@ -1922,8 +1927,9 @@ void Preprocessor::HandleIncludeDirectiv HashLoc, IncludeTok, LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, FilenameRange, File, SearchPath, RelativePath, - ShouldEnter ? nullptr : SuggestedModule.getModule(), FileCharacter); - if (SkipHeader && !SuggestedModule.getModule()) + Action == Import ? SuggestedModule.getModule() : nullptr, + FileCharacter); + if (Action == Skip) Callbacks->FileSkipped(*File, FilenameTok, FileCharacter); } @@ -1968,28 +1974,33 @@ void Preprocessor::HandleIncludeDirectiv } } - // If we don't need to enter the file, stop now. - if (!ShouldEnter) { + switch (Action) { + case Skip: + // If we don't need to enter the file, stop now. + return; + + case IncludeLimitReached: + // If we reached our include limit and don't want to enter any more files, + // don't go any further. + return; + + case Import: { // If this is a module import, make it visible if needed. - if (auto *M = SuggestedModule.getModule()) { - // When building a pch, -fmodule-name tells the compiler to textually - // include headers in the specified module. But it is possible that - // ShouldEnter is false because we are skipping the header. In that - // case, We are not importing the specified module. - if (SkipHeader && getLangOpts().CompilingPCH && - isForModuleBuilding(M, getLangOpts().CurrentModule, - getLangOpts().ModuleName)) - return; - - makeModuleVisible(M, HashLoc); - - if (IncludeTok.getIdentifierInfo()->getPPKeywordID() != - tok::pp___include_macros) - EnterAnnotationToken(DirectiveRange, tok::annot_module_include, M); - } + Module *M = SuggestedModule.getModule(); + assert(M && "no module to import"); + + makeModuleVisible(M, HashLoc); + + if (IncludeTok.getIdentifierInfo()->getPPKeywordID() != + tok::pp___include_macros) + EnterAnnotationToken(DirectiveRange, tok::annot_module_include, M); return; } + case Enter: + break; + } + // Check that we don't have infinite #include recursion. if (IncludeMacroStack.size() == MaxAllowedIncludeStackDepth-1) { Diag(FilenameTok, diag::err_pp_include_too_deep); @@ -2024,6 +2035,11 @@ void Preprocessor::HandleIncludeDirectiv // When building a pch, -fmodule-name tells the compiler to textually // include headers in the specified module. We are not building the // specified module. + // + // FIXME: This is the wrong way to handle this. We should produce a PCH + // that behaves the same as the header would behave in a compilation using + // that PCH, which means we should enter the submodule. We need to teach + // the AST serialization layer to deal with the resulting AST. if (getLangOpts().CompilingPCH && isForModuleBuilding(M, getLangOpts().CurrentModule, getLangOpts().ModuleName)) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits