[Support] Remove output file checks from `LockFileManager` (#130395)

Currently, `LockFileManager` assumes the owner of the lock file creates
an output file. This is problematic for at least three reasons:

1. It is orthogonal to the main purpose of this class - mutual
exclusion. This makes creating an alternative implementation more
complicated than it needs to be.

2. Some clients (like the upstream `AMDGPUSplitModule.cpp` file) assume
the output file is not necessary. The owner of the lock file does not
write the file expected by `LockFileManager` and the processes waiting
for the non-owned lock file to be unlocked therefore assume the owner
has died. This means that the work gets repeated by each waiting
process, serially.

3. The documentation makes it sound like successfully waiting for a
non-owned lock file guarantees the output file to be present on the file
system. Implicitly-built modules rely on this. However, the module file
may disappear between `LockFileManager` performing the check and the
compiler loading the module (for example due to module cache pruning
with short intervals, or intervention from outside of Clang). The
compiler assumes this cannot happen, and fails the build if it does.

This PR solves this situation by removing the check, reflecting that in
the `LockFileManager` documentation, and fixing the time-of-check
time-of-use bug in implicit modules.

1 files changed, 15 insertions, 7 deletions

diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index c11c857..6098e2e3 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1406,7 +1406,7 @@ static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance,
                                       SourceLocation ImportLoc,
                                       SourceLocation ModuleNameLoc,
                                       Module *Module, StringRef ModuleFileName,
-                                      bool *OutOfDate) {
+                                      bool *OutOfDate, bool *Missing) {
   DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics();
 
   unsigned ModuleLoadCapabilities = ASTReader::ARR_Missing;
@@ -1427,6 +1427,12 @@ static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance,
     return false;
   }
 
+  // The caller wants to handle missing module files.
+  if (Missing && ReadResult == ASTReader::Missing) {
+    *Missing = true;
+    return false;
+  }
+
   // The ASTReader didn't diagnose the error, so conservatively report it.
   if (ReadResult == ASTReader::Missing || !Diags.hasErrorOccurred())
     Diags.Report(ModuleNameLoc, diag::err_module_not_built)
@@ -1452,7 +1458,7 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance,
 
   return readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc,
                                    Module, ModuleFileName,
-                                   /*OutOfDate=*/nullptr);
+                                   /*OutOfDate=*/nullptr, /*Missing=*/nullptr);
 }
 
 /// Compile a module in a separate compiler instance and read the AST,
@@ -1517,15 +1523,17 @@ static bool compileModuleAndReadASTBehindLock(
 
     // Read the module that was just written by someone else.
     bool OutOfDate = false;
+    bool Missing = false;
     if (readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc,
-                                  Module, ModuleFileName, &OutOfDate))
+                                  Module, ModuleFileName, &OutOfDate, &Missing))
       return true;
-    if (!OutOfDate)
+    if (!OutOfDate && !Missing)
       return false;
 
-    // The module may be out of date in the presence of file system races,
-    // or if one of its imports depends on header search paths that are not
-    // consistent with this ImportingInstance.  Try again...
+    // The module may be missing or out of date in the presence of file system
+    // races. It may also be out of date if one of its imports depends on header
+    // search paths that are not consistent with this ImportingInstance.
+    // Try again...
   }
 }