Modules: Remove ModuleLoader::OtherUncachedFailure, NFC
5cca6223 refactored CompilerInstance::loadModule, splitting out findOrCompileModuleAndReadAST, but was careful to avoid making any functional changes. It added ModuleLoader::OtherUncachedFailure to facilitate this and left behind FIXMEs asking why certain failures weren't cached. After a closer look, I think we can just remove this and simplify the code. This changes the behaviour of the following (simplified) code from CompilerInstance::loadModule, causing a failure to be cached more often: ``` if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) return *MaybeModule; if (ModuleName == getLangOpts().CurrentModule) return MM.cacheModuleLoad(PP.lookupModule(...)); ModuleLoadResult Result = findOrCompileModuleAndReadAST(...); if (Result.isNormal()) // This will be 'true' more often. return MM.cacheModuleLoad(..., Module); return Result; ``` `MM` here is a ModuleMap owned by the Preprocessor. Here are the cases where `findOrCompileModuleAndReadAST` starts returning a "normal" failed result: - Emitted `diag::err_module_not_found`, where there's no module map found. - Emitted `diag::err_module_build_disabled`, where implicitly building modules is disabled. - Emitted `diag::err_module_cycle`, which detects module cycles in the implicit modules build system. - Emitted `diag::err_module_not_built`, which avoids building a module in this CompilerInstance if another one tried and failed already. - `compileModuleAndReadAST()` was called and failed to build. The four errors are all fatal, and last item also reports a fatal error, so it this extra caching has no functionality change... but even if it did, it seems fine to cache these failed results within a ModuleMap instance (note that each CompilerInstance has its own Preprocessor and ModuleMap). Differential Revision: https://reviews.llvm.org/D101667
Loading
Please sign in to comment