dexonsmith created this revision.
dexonsmith added reviewers: bruno, Bigcheese, jkorous, rsmith.
Herald added a subscriber: ributzka.
Refactor the logic on CompilerInstance::loadModule and a couple of
surrounding methods in order to clarify what's going on.
- Rename ModuleLoader::loadModuleFromSource to compileModuleFromSource and fix
its documentation, since it never loads a module. It just creates/compiles one.
- Rename one of the overloads of compileModuleImpl to compileModule, making it
more obvious which one calls the other.
- Rename compileAndLoadModule to compileModuleAndReadAST. This clarifies the
relationship between this helper and its caller, CompilerInstance::loadModule
(the old name implied the opposite relationship). It also (correctly)
indicates that more needs to be done to load the module than this function is
responsible for.
- Split findOrCompileModuleAndReadAST out of loadModule. Besides reducing
nesting for this code thanks to early returns and the like, this refactor
clarifies the logic in loadModule, particularly around calls to
ModuleMap::cacheModuleLoad and ModuleMap::getCachedModuleLoad.
findOrCompileModuleAndReadAST also breaks early if the initial ReadAST call
returns Missing or OutOfDate, allowing the last ditch call to
compileModuleAndReadAST to come at the end of the function body.
- Additionally split out selectModuleSource, clarifying the logic due to
early returns.
- Add ModuleLoadResult::isNormal and OtherUncachedFailure, so that loadModule
knows whether to cache the result. OtherUncachedFailure was added to keep this
patch NFC, but there's a chance that these cases were uncached by accident,
through copy/paste/modify failures. These should be audited as a follow-up
(maybe we can eliminate this case).
- Do *not* lift the setting of `ModuleLoadFailed = true` to loadModule
because there isn't a clear pattern for when it's set. This should be
reconsidered in a follow-up, in case it would be correct to set
`ModuleLoadFailed` whenever no module is returned and the result is either
Normal or OtherUncachedFailure.
- Add some header documentation where it was missing, and fix it where it was
wrong.
This should have no functionality change.
https://reviews.llvm.org/D70556
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Lex/ModuleLoader.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Lex/Pragma.cpp
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -848,8 +848,8 @@
CurLexer->getBuffer().begin() <= End &&
End <= CurLexer->getBuffer().end() &&
"module source range not contained within same file buffer");
- TheModuleLoader.loadModuleFromSource(Loc, ModuleName->getName(),
- StringRef(Start, End - Start));
+ TheModuleLoader.createModuleFromSource(Loc, ModuleName->getName(),
+ StringRef(Start, End - Start));
}
void Preprocessor::HandlePragmaHdrstop(Token ) {
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1179,13 +1179,12 @@
return nullptr;
}
-/// Compile a module file for the given module, using the options
-/// provided by the importing compiler instance. Returns true if the module
-/// was built without errors.
-static bool compileModuleImpl(CompilerInstance ,
- SourceLocation ImportLoc,
- Module *Module,
- StringRef ModuleFileName) {
+/// Compile a module file for the given module in a separate compiler instance,
+/// using the options provided by the importing compiler instance. Returns true
+/// if the module was built without errors.
+static bool compileModule(CompilerInstance ,
+ SourceLocation ImportLoc, Module *Module,
+ StringRef ModuleFileName) {
InputKind IK(getLanguageFromOptions(ImportingInstance.getLangOpts()),
InputKind::ModuleMap);
@@ -1245,10 +1244,17 @@
return Result;
}
-static bool compileAndLoadModule(CompilerInstance ,
- SourceLocation ImportLoc,
- SourceLocation ModuleNameLoc, Module *Module,
- StringRef ModuleFileName) {
+/// Compile a module in a separate compiler instance and read the AST,
+/// returning true if the module compiles without errors.
+///
+/// Uses a lock file manager and exponential backoff to reduce the chances that
+/// multiple instances will compete to create the same module. On timeout,
+/// deletes the lock file in order to avoid deadlock from crashing processes or
+/// bugs in the lock file manager.
+static bool
