r356704 - Refactor handling of #include directives to cleanly separate the

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 21 12:44:17 PDT 2019


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))




More information about the cfe-commits mailing list