[clang] 7c57a9b - Modules: Simplify how DisableGeneratingGlobalModuleIndex is set, likely NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2021-05-13T10:39:40-07:00
New Revision: 7c57a9bd7d4c976b7a824472c427433359200e02

URL: https://github.com/llvm/llvm-project/commit/7c57a9bd7d4c976b7a824472c427433359200e02
DIFF: https://github.com/llvm/llvm-project/commit/7c57a9bd7d4c976b7a824472c427433359200e02.diff

LOG: Modules: Simplify how DisableGeneratingGlobalModuleIndex is set, likely NFC

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

Added: 
    

Modified: 
    clang/lib/Frontend/CompilerInstance.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 54917b3fb0fb..a623580bac7f 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -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);
   }