dexonsmith created this revision.
dexonsmith added reviewers: jansvoboda11, Bigcheese, rsmith.
dexonsmith requested review of this revision.
Herald added a project: clang.
5cca622310c10fdf6f921b6cce26f91d9f14c762
<https://reviews.llvm.org/rG5cca622310c10fdf6f921b6cce26f91d9f14c762>
(https://reviews.llvm.org/D70556) 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).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D101667
Files:
clang/include/clang/Lex/ModuleLoader.h
clang/lib/Frontend/CompilerInstance.cpp
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1684,8 +1684,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
if (ModuleFilename.empty()) {
if (M && M->HasIncompatibleModuleFile) {
@@ -1697,8 +1696,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
<< ModuleName;
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Create an ASTReader on demand.
@@ -1809,8 +1807,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_cycle)
<< ModuleName << CyclePath;
// FIXME: Should this set ModuleBuildFailed = true?
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Check whether we have already attempted to build this module (but
@@ -1820,8 +1817,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Try to compile and then read the AST.
@@ -1832,8 +1828,7 @@
if (getPreprocessorOpts().FailedModules)
getPreprocessorOpts().FailedModules->addFailed(ModuleName);
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Okay, we've rebuilt and now loaded the module.
Index: clang/include/clang/Lex/ModuleLoader.h
===================================================================
--- clang/include/clang/Lex/ModuleLoader.h
+++ clang/include/clang/Lex/ModuleLoader.h
@@ -45,9 +45,6 @@
// The module exists but cannot be imported due to a configuration
mismatch.
ConfigMismatch,
-
- // We failed to load the module, but we shouldn't cache the failure.
- OtherUncachedFailure,
};
llvm::PointerIntPair<Module *, 2, LoadResultKind> Storage;
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1684,8 +1684,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
if (ModuleFilename.empty()) {
if (M && M->HasIncompatibleModuleFile) {
@@ -1697,8 +1696,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
<< ModuleName;
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Create an ASTReader on demand.
@@ -1809,8 +1807,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_cycle)
<< ModuleName << CyclePath;
// FIXME: Should this set ModuleBuildFailed = true?
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Check whether we have already attempted to build this module (but
@@ -1820,8 +1817,7 @@
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Try to compile and then read the AST.
@@ -1832,8 +1828,7 @@
if (getPreprocessorOpts().FailedModules)
getPreprocessorOpts().FailedModules->addFailed(ModuleName);
ModuleBuildFailed = true;
- // FIXME: Why is this not cached?
- return ModuleLoadResult::OtherUncachedFailure;
+ return nullptr;
}
// Okay, we've rebuilt and now loaded the module.
Index: clang/include/clang/Lex/ModuleLoader.h
===================================================================
--- clang/include/clang/Lex/ModuleLoader.h
+++ clang/include/clang/Lex/ModuleLoader.h
@@ -45,9 +45,6 @@
// The module exists but cannot be imported due to a configuration mismatch.
ConfigMismatch,
-
- // We failed to load the module, but we shouldn't cache the failure.
- OtherUncachedFailure,
};
llvm::PointerIntPair<Module *, 2, LoadResultKind> Storage;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
