Modules: Simplify how DisableGeneratingGlobalModuleIndex is set, likely NFC
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Fri, 30 Apr 2021 22:23:47 +0000 (15:23 -0700)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 13 May 2021 17:39:40 +0000 (10:39 -0700)
DisableGeneratingGlobalModuleIndex was being set by
CompilerInstance::findOrCompileModuleAndReadAST most of (but not all of)
the times it returned `nullptr` as a "normal" failure. Pull that up to
the caller, CompilerInstance::loadModule, to simplify the code. This
resolves a number of FIXMEs added during the refactoring in
5cca622310c10fdf6f921b6cce26f91d9f14c762.

The extra cases where this is set are all some version of a fatal error,
and the only client of the field, shouldBuildGlobalModuleIndex, seems
to be unreachable in that case. Even if there is some corner case where
this has an effect, it seems like the right/consistent behaviour.

Differential Revision: https://reviews.llvm.org/D101672

clang/lib/Frontend/CompilerInstance.cpp

index 54917b3..a623580 100644 (file)
@@ -1683,7 +1683,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
     // We can't find a module, error out here.
     getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
         << ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
-    DisableGeneratingGlobalModuleIndex = true;
     return nullptr;
   }
   if (ModuleFilename.empty()) {
@@ -1695,7 +1694,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
 
     getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
         << ModuleName;
-    DisableGeneratingGlobalModuleIndex = true;
     return nullptr;
   }
 
@@ -1742,7 +1740,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
         if (*ModuleFile == M->getASTFile())
           return M;
 
-    DisableGeneratingGlobalModuleIndex = true;
     getDiagnostics().Report(ModuleNameLoc, diag::err_module_prebuilt)
         << ModuleName;
     return ModuleLoadResult();
@@ -1764,14 +1761,12 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
     LLVM_FALLTHROUGH;
   case ASTReader::VersionMismatch:
   case ASTReader::HadErrors:
-    // FIXME: Should this set DisableGeneratingGlobalModuleIndex = true?
     ModuleLoader::HadFatalFailure = true;
     // FIXME: The ASTReader will already have complained, but can we shoehorn
     // that diagnostic information into a more useful form?
     return ModuleLoadResult();
 
   case ASTReader::Failure:
-    // FIXME: Should this set DisableGeneratingGlobalModuleIndex = true?
     ModuleLoader::HadFatalFailure = true;
     return ModuleLoadResult();
   }
@@ -1781,7 +1776,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
     // We don't know the desired configuration for this module and don't
     // necessarily even have a module map. Since ReadAST already produces
     // diagnostics for these two cases, we simply error out here.
-    DisableGeneratingGlobalModuleIndex = true;
     return ModuleLoadResult();
   }
 
@@ -1806,7 +1800,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
 
     getDiagnostics().Report(ModuleNameLoc, diag::err_module_cycle)
         << ModuleName << CyclePath;
-    // FIXME: Should this set DisableGeneratingGlobalModuleIndex = true?
     return nullptr;
   }
 
@@ -1816,7 +1809,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
       getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {
     getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
         << ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
-    DisableGeneratingGlobalModuleIndex = true;
     return nullptr;
   }
 
@@ -1827,7 +1819,6 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
            "undiagnosed error in compileModuleAndReadAST");
     if (getPreprocessorOpts().FailedModules)
       getPreprocessorOpts().FailedModules->addFailed(ModuleName);
-    DisableGeneratingGlobalModuleIndex = true;
     return nullptr;
   }
 
@@ -1878,11 +1869,10 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   } else {
     ModuleLoadResult Result = findOrCompileModuleAndReadAST(
         ModuleName, ImportLoc, ModuleNameLoc, IsInclusionDirective);
-    // FIXME: Can we pull 'DisableGeneratingGlobalModuleIndex = true' out of
-    // the return sequences for findOrCompileModuleAndReadAST and do it here
-    // (as long as the result is not a config mismatch)?  See FIXMEs there.
     if (!Result.isNormal())
       return Result;
+    if (!Result)
+      DisableGeneratingGlobalModuleIndex = true;
     Module = Result;
     MM.cacheModuleLoad(*Path[0].first, Module);
   }