[clang] [llvm] [Support] Return `LockFileManager` errors right away (PR #130627)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 10 15:58:35 PDT 2025


https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/130627

>From 88b5bbd1303480dc40db3ea76d0f831a69ff1957 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Sat, 8 Mar 2025 21:51:51 -0800
Subject: [PATCH 1/5] [Support] Return `LockFileManager` errors right away

---
 clang/lib/Frontend/CompilerInstance.cpp       | 23 +++--
 clang/lib/Serialization/GlobalModuleIndex.cpp | 17 ++--
 llvm/include/llvm/Support/LockFileManager.h   | 33 +------
 llvm/lib/Support/LockFileManager.cpp          | 91 ++++++-------------
 llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp  | 20 ++--
 5 files changed, 59 insertions(+), 125 deletions(-)

diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index c11c857ea0606..fe351e6b41342 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1477,29 +1477,28 @@ static bool compileModuleAndReadASTBehindLock(
   llvm::sys::fs::create_directories(Dir);
 
   while (true) {
-    llvm::LockFileManager Locked(ModuleFileName);
-    switch (Locked) {
-    case llvm::LockFileManager::LFS_Error:
+    llvm::LockFileManager Lock(ModuleFileName);
+    bool Owned;
+    if (llvm::Error Err = Lock.tryLock().moveInto(Owned)) {
       // ModuleCache takes care of correctness and locks are only necessary for
       // performance. Fallback to building the module in case of any lock
       // related errors.
       Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure)
-          << Module->Name << Locked.getErrorMessage();
+          << Module->Name << toString(std::move(Err));
       // Clear out any potential leftover.
-      Locked.unsafeRemoveLockFile();
-      [[fallthrough]];
-    case llvm::LockFileManager::LFS_Owned:
+      Lock.unsafeRemoveLockFile();
+      return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
+                                        ModuleNameLoc, Module, ModuleFileName);
+    }
+    if (Owned) {
       // We're responsible for building the module ourselves.
       return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
                                          ModuleNameLoc, Module, ModuleFileName);
-
-    case llvm::LockFileManager::LFS_Shared:
-      break; // The interesting case.
     }
 
     // Someone else is responsible for building the module. Wait for them to
     // finish.
-    switch (Locked.waitForUnlock()) {
+    switch (Lock.waitForUnlock()) {
     case llvm::LockFileManager::Res_Success:
       break; // The interesting case.
     case llvm::LockFileManager::Res_OwnerDied:
@@ -1511,7 +1510,7 @@ static bool compileModuleAndReadASTBehindLock(
       Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout)
           << Module->Name;
       // Clear the lock file so that future invocations can make progress.
-      Locked.unsafeRemoveLockFile();
+      Lock.unsafeRemoveLockFile();
       continue;
     }
 
diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp
index 4b920fccecac3..1e2272c48bd04 100644
--- a/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -849,22 +849,21 @@ GlobalModuleIndex::writeIndex(FileManager &FileMgr,
 
   // Coordinate building the global index file with other processes that might
   // try to do the same.
-  llvm::LockFileManager Locked(IndexPath);
-  switch (Locked) {
-  case llvm::LockFileManager::LFS_Error:
+  llvm::LockFileManager Lock(IndexPath);
+  bool Owned;
+  if (llvm::Error Err = Lock.tryLock().moveInto(Owned)) {
+    llvm::consumeError(std::move(Err));
     return llvm::createStringError(std::errc::io_error, "LFS error");
-
-  case llvm::LockFileManager::LFS_Owned:
-    // We're responsible for building the index ourselves. Do so below.
-    break;
-
-  case llvm::LockFileManager::LFS_Shared:
+  }
+  if (!Owned) {
     // Someone else is responsible for building the index. We don't care
     // when they finish, so we're done.
     return llvm::createStringError(std::errc::device_or_resource_busy,
                                    "someone else is building the index");
   }
 
+  // We're responsible for building the index ourselves.
+
   // The module index builder.
   GlobalModuleIndexBuilder Builder(FileMgr, PCHContainerRdr);
 
diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index 92c7ceed6a929..e687f3b1dfa53 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -9,6 +9,7 @@
 #define LLVM_SUPPORT_LOCKFILEMANAGER_H
 
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Error.h"
 #include <optional>
 #include <system_error>
 #include <utility> // for std::pair
@@ -26,19 +27,6 @@ class StringRef;
 /// operation.
 class LockFileManager {
 public:
-  /// Describes the state of a lock file.
-  enum LockFileState {
-    /// The lock file has been created and is owned by this instance
-    /// of the object.
-    LFS_Owned,
-    /// The lock file already exists and is owned by some other
-    /// instance.
-    LFS_Shared,
-    /// An error occurred while trying to create or find the lock
-    /// file.
-    LFS_Error
-  };
-
   /// Describes the result of waiting for the owner to release the lock.
   enum WaitForUnlockResult {
     /// The lock was released successfully.
@@ -55,8 +43,6 @@ class LockFileManager {
   SmallString<128> UniqueLockFileName;
 
   std::optional<std::pair<std::string, int>> Owner;
-  std::error_code ErrorCode;
-  std::string ErrorDiagMsg;
 
   LockFileManager(const LockFileManager &) = delete;
   LockFileManager &operator=(const LockFileManager &) = delete;
@@ -71,10 +57,10 @@ class LockFileManager {
   LockFileManager(StringRef FileName);
   ~LockFileManager();
 
-  /// Determine the state of the lock file.
-  LockFileState getState() const;
-
-  operator LockFileState() const { return getState(); }
+  /// Tries to acquire the lock without blocking.
+  /// \returns true if the lock was successfully acquired, false if the lock is
+  /// already held by someone else, or \c Error in case of unexpected failure.
+  Expected<bool> tryLock();
 
   /// For a shared lock, wait until the owner releases the lock.
   /// Total timeout for the file to appear is ~1.5 minutes.
@@ -84,15 +70,6 @@ class LockFileManager {
   /// Remove the lock file.  This may delete a different lock file than
   /// the one previously read if there is a race.
   std::error_code unsafeRemoveLockFile();
-
-  /// Get error message, or "" if there is no error.
-  std::string getErrorMessage() const;
-
-  /// Set error and error message
-  void setError(const std::error_code &EC, StringRef ErrorMsg = "") {
-    ErrorCode = EC;
-    ErrorDiagMsg = ErrorMsg.str();
-  }
 };
 
 } // end namespace llvm
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 9a45a9966458e..1ec206415a13c 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -157,42 +157,35 @@ class RemoveUniqueLockFileOnSignal {
 
 } // end anonymous namespace
 
-LockFileManager::LockFileManager(StringRef FileName)
-{
-  this->FileName = FileName;
-  if (std::error_code EC = sys::fs::make_absolute(this->FileName)) {
-    std::string S("failed to obtain absolute path for ");
-    S.append(std::string(this->FileName));
-    setError(EC, S);
-    return;
-  }
-  LockFileName = this->FileName;
+LockFileManager::LockFileManager(StringRef FileName) : FileName(FileName) {}
+
+Expected<bool> LockFileManager::tryLock() {
+  SmallString<128> AbsoluteFileName(FileName);
+  if (std::error_code EC = sys::fs::make_absolute(AbsoluteFileName))
+    return createStringError("failed to obtain absolute path for " +
+                             AbsoluteFileName);
+  LockFileName = AbsoluteFileName;
   LockFileName += ".lock";
 
   // If the lock file already exists, don't bother to try to create our own
   // lock file; it won't work anyway. Just figure out who owns this lock file.
   if ((Owner = readLockFile(LockFileName)))
-    return;
+    return false;
 
   // Create a lock file that is unique to this instance.
   UniqueLockFileName = LockFileName;
   UniqueLockFileName += "-%%%%%%%%";
   int UniqueLockFileID;
   if (std::error_code EC = sys::fs::createUniqueFile(
-          UniqueLockFileName, UniqueLockFileID, UniqueLockFileName)) {
-    std::string S("failed to create unique file ");
-    S.append(std::string(UniqueLockFileName));
-    setError(EC, S);
-    return;
-  }
+          UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
+    return createStringError("failed to create unique file " +
+                             UniqueLockFileName);
 
   // Write our process ID to our unique lock file.
   {
     SmallString<256> HostID;
-    if (auto EC = getHostID(HostID)) {
-      setError(EC, "failed to get host id");
-      return;
-    }
+    if (auto EC = getHostID(HostID))
+      return createStringError(EC, "failed to get host id");
 
     raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
     Out << HostID << ' ' << sys::Process::getProcessId();
@@ -201,13 +194,12 @@ LockFileManager::LockFileManager(StringRef FileName)
     if (Out.has_error()) {
       // We failed to write out PID, so report the error, remove the
       // unique lock file, and fail.
-      std::string S("failed to write to ");
-      S.append(std::string(UniqueLockFileName));
-      setError(Out.error(), S);
+      Error Err = createStringError(Out.error(),
+                                    "failed to write to " + UniqueLockFileName);
       sys::fs::remove(UniqueLockFileName);
       // Don't call report_fatal_error.
       Out.clear_error();
-      return;
+      return std::move(Err);
     }
   }
 
@@ -221,23 +213,19 @@ LockFileManager::LockFileManager(StringRef FileName)
         sys::fs::create_link(UniqueLockFileName, LockFileName);
     if (!EC) {
       RemoveUniqueFile.lockAcquired();
-      return;
+      return true;
     }
 
-    if (EC != errc::file_exists) {
-      std::string S("failed to create link ");
-      raw_string_ostream OSS(S);
-      OSS << LockFileName.str() << " to " << UniqueLockFileName.str();
-      setError(EC, S);
-      return;
-    }
+    if (EC != errc::file_exists)
+      return createStringError(EC, "failed to create link " + LockFileName +
+                                       " to " + UniqueLockFileName);
 
     // Someone else managed to create the lock file first. Read the process ID
     // from the lock file.
     if ((Owner = readLockFile(LockFileName))) {
       // Wipe out our unique lock file (it's useless now)
       sys::fs::remove(UniqueLockFileName);
-      return;
+      return false;
     }
 
     if (!sys::fs::exists(LockFileName)) {
@@ -248,39 +236,14 @@ LockFileManager::LockFileManager(StringRef FileName)
 
     // There is a lock file that nobody owns; try to clean it up and get
     // ownership.
-    if ((EC = sys::fs::remove(LockFileName))) {
-      std::string S("failed to remove lockfile ");
-      S.append(std::string(UniqueLockFileName));
-      setError(EC, S);
-      return;
-    }
-  }
-}
-
-LockFileManager::LockFileState LockFileManager::getState() const {
-  if (Owner)
-    return LFS_Shared;
-
-  if (ErrorCode)
-    return LFS_Error;
-
-  return LFS_Owned;
-}
-
-std::string LockFileManager::getErrorMessage() const {
-  if (ErrorCode) {
-    std::string Str(ErrorDiagMsg);
-    std::string ErrCodeMsg = ErrorCode.message();
-    raw_string_ostream OSS(Str);
-    if (!ErrCodeMsg.empty())
-      OSS << ": " << ErrCodeMsg;
-    return Str;
+    if ((EC = sys::fs::remove(LockFileName)))
+      return createStringError(EC, "failed to remove lockfile " +
+                                       UniqueLockFileName);
   }
-  return "";
 }
 
 LockFileManager::~LockFileManager() {
-  if (getState() != LFS_Owned)
+  if (Owner)
     return;
 
   // Since we own the lock, remove the lock file and our own unique lock file.
@@ -293,7 +256,7 @@ LockFileManager::~LockFileManager() {
 
 LockFileManager::WaitForUnlockResult
 LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
-  if (getState() != LFS_Shared)
+  if (!Owner)
     return Res_Success;
 
   // Since we don't yet have an event-based method to wait for the lock file,
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
index 65aee2f70104b..65cbb13628301 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
@@ -1545,18 +1545,16 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
                       << "'\n");
 
     while (true) {
-      llvm::LockFileManager Locked(LockFilePath.str());
-      switch (Locked) {
-      case LockFileManager::LFS_Error:
+      llvm::LockFileManager Lock(LockFilePath.str());
+      bool Owned;
+      if (Error Err = Lock.tryLock().moveInto(Owned)) {
+        consumeError(std::move(Err));
         LLVM_DEBUG(
             dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug "
                       "output may be mangled by other processes\n");
-        Locked.unsafeRemoveLockFile();
-        break;
-      case LockFileManager::LFS_Owned:
-        break;
-      case LockFileManager::LFS_Shared: {
-        switch (Locked.waitForUnlock()) {
+        Lock.unsafeRemoveLockFile();
+      } else if (!Owned) {
+        switch (Lock.waitForUnlock()) {
         case LockFileManager::Res_Success:
           break;
         case LockFileManager::Res_OwnerDied:
@@ -1566,11 +1564,9 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
               dbgs()
               << "[amdgpu-split-module] unable to acquire lockfile, debug "
                  "output may be mangled by other processes\n");
-          Locked.unsafeRemoveLockFile();
+          Lock.unsafeRemoveLockFile();
           break; // give up
         }
-        break;
-      }
       }
 
       splitAMDGPUModule(TTIGetter, M, N, ModuleCallback);

>From 45975fe6bf5575a5d77272349ba51404f7282b83 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 10 Mar 2025 09:12:11 -0700
Subject: [PATCH 2/5] clang-format

---
 clang/lib/Frontend/CompilerInstance.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index fe351e6b41342..db72575676641 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1488,7 +1488,7 @@ static bool compileModuleAndReadASTBehindLock(
       // Clear out any potential leftover.
       Lock.unsafeRemoveLockFile();
       return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
-                                        ModuleNameLoc, Module, ModuleFileName);
+                                         ModuleNameLoc, Module, ModuleFileName);
     }
     if (Owned) {
       // We're responsible for building the module ourselves.

>From 8a983fcd4e5e792eec7212a3f3b9954e84518668 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 10 Mar 2025 13:35:12 -0700
Subject: [PATCH 3/5] Keep more state, add asserts, update tests

---
 llvm/include/llvm/Support/LockFileManager.h   | 15 ++++++---
 llvm/lib/Support/LockFileManager.cpp          | 32 +++++++++++++------
 .../unittests/Support/LockFileManagerTest.cpp |  9 +++---
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index e687f3b1dfa53..364d4c360c8ee 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -42,19 +42,26 @@ class LockFileManager {
   SmallString<128> LockFileName;
   SmallString<128> UniqueLockFileName;
 
-  std::optional<std::pair<std::string, int>> Owner;
+  struct OwnerUnknown {};
+  struct OwnedByUs{};
+  struct OwnedByAnother {
+    std::string OwnerHostName;
+    int OwnerPID;
+  };
+  std::variant<OwnerUnknown, OwnedByUs, OwnedByAnother> Owner;
 
   LockFileManager(const LockFileManager &) = delete;
   LockFileManager &operator=(const LockFileManager &) = delete;
 
-  static std::optional<std::pair<std::string, int>>
-  readLockFile(StringRef LockFileName);
+  static std::optional<OwnedByAnother> readLockFile(StringRef LockFileName);
 
   static bool processStillExecuting(StringRef Hostname, int PID);
 
 public:
-
+  /// Does not try to acquire the lock.
   LockFileManager(StringRef FileName);
+
+  /// Unlocks the lock if previously acquired by \c tryLock().
   ~LockFileManager();
 
   /// Tries to acquire the lock without blocking.
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 1ec206415a13c..76ef78e275642 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -51,7 +51,7 @@ using namespace llvm;
 /// \param LockFileName The name of the lock file to read.
 ///
 /// \returns The process ID of the process that owns this lock file
-std::optional<std::pair<std::string, int>>
+std::optional<LockFileManager::OwnedByAnother>
 LockFileManager::readLockFile(StringRef LockFileName) {
   // Read the owning host and PID out of the lock file. If it appears that the
   // owning process is dead, the lock file is invalid.
@@ -69,8 +69,10 @@ LockFileManager::readLockFile(StringRef LockFileName) {
   PIDStr = PIDStr.substr(PIDStr.find_first_not_of(' '));
   int PID;
   if (!PIDStr.getAsInteger(10, PID)) {
-    auto Owner = std::make_pair(std::string(Hostname), PID);
-    if (processStillExecuting(Owner.first, Owner.second))
+    OwnedByAnother Owner;
+    Owner.OwnerHostName = Hostname;
+    Owner.OwnerPID = PID;
+    if (processStillExecuting(Owner.OwnerHostName, Owner.OwnerPID))
       return Owner;
   }
 
@@ -157,9 +159,13 @@ class RemoveUniqueLockFileOnSignal {
 
 } // end anonymous namespace
 
-LockFileManager::LockFileManager(StringRef FileName) : FileName(FileName) {}
+LockFileManager::LockFileManager(StringRef FileName)
+    : FileName(FileName), Owner(OwnerUnknown{}) {}
 
 Expected<bool> LockFileManager::tryLock() {
+  assert(std::holds_alternative<OwnerUnknown>(Owner) &&
+         "lock has already been attempted");
+
   SmallString<128> AbsoluteFileName(FileName);
   if (std::error_code EC = sys::fs::make_absolute(AbsoluteFileName))
     return createStringError("failed to obtain absolute path for " +
@@ -169,8 +175,10 @@ Expected<bool> LockFileManager::tryLock() {
 
   // If the lock file already exists, don't bother to try to create our own
   // lock file; it won't work anyway. Just figure out who owns this lock file.
-  if ((Owner = readLockFile(LockFileName)))
+  if (auto LockFileOwner = readLockFile(LockFileName)) {
+    Owner = std::move(*LockFileOwner);
     return false;
+  }
 
   // Create a lock file that is unique to this instance.
   UniqueLockFileName = LockFileName;
@@ -213,6 +221,7 @@ Expected<bool> LockFileManager::tryLock() {
         sys::fs::create_link(UniqueLockFileName, LockFileName);
     if (!EC) {
       RemoveUniqueFile.lockAcquired();
+      Owner = OwnedByUs{};
       return true;
     }
 
@@ -222,9 +231,10 @@ Expected<bool> LockFileManager::tryLock() {
 
     // Someone else managed to create the lock file first. Read the process ID
     // from the lock file.
-    if ((Owner = readLockFile(LockFileName))) {
+    if (auto LockFileOwner = readLockFile(LockFileName)) {
       // Wipe out our unique lock file (it's useless now)
       sys::fs::remove(UniqueLockFileName);
+      Owner = std::move(*LockFileOwner);
       return false;
     }
 
@@ -243,7 +253,7 @@ Expected<bool> LockFileManager::tryLock() {
 }
 
 LockFileManager::~LockFileManager() {
-  if (Owner)
+  if (!std::holds_alternative<OwnedByUs>(Owner))
     return;
 
   // Since we own the lock, remove the lock file and our own unique lock file.
@@ -256,8 +266,9 @@ LockFileManager::~LockFileManager() {
 
 LockFileManager::WaitForUnlockResult
 LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
-  if (!Owner)
-    return Res_Success;
+  auto *LockFileOwner = std::get_if<OwnedByAnother>(&Owner);
+  assert(LockFileOwner &&
+         "waiting for lock to be unlocked without knowing the owner");
 
   // Since we don't yet have an event-based method to wait for the lock file,
   // use randomized exponential backoff, similar to Ethernet collision
@@ -278,7 +289,8 @@ LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
     }
 
     // If the process owning the lock died without cleaning up, just bail out.
-    if (!processStillExecuting((*Owner).first, (*Owner).second))
+    if (!processStillExecuting(LockFileOwner->OwnerHostName,
+                               LockFileOwner->OwnerPID))
       return Res_OwnerDied;
   }
 
diff --git a/llvm/unittests/Support/LockFileManagerTest.cpp b/llvm/unittests/Support/LockFileManagerTest.cpp
index 552053d46e843..627b2daef650c 100644
--- a/llvm/unittests/Support/LockFileManagerTest.cpp
+++ b/llvm/unittests/Support/LockFileManagerTest.cpp
@@ -9,6 +9,7 @@
 #include "llvm/Support/LockFileManager.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gtest/gtest.h"
 #include <memory>
@@ -27,12 +28,12 @@ TEST(LockFileManagerTest, Basic) {
   {
     // The lock file should not exist, so we should successfully acquire it.
     LockFileManager Locked1(LockedFile);
-    EXPECT_EQ(LockFileManager::LFS_Owned, Locked1.getState());
+    EXPECT_THAT_EXPECTED(Locked1.tryLock(), HasValue(true));
 
     // Attempting to reacquire the lock should fail.  Waiting on it would cause
     // deadlock, so don't try that.
     LockFileManager Locked2(LockedFile);
-    EXPECT_NE(LockFileManager::LFS_Owned, Locked2.getState());
+    EXPECT_THAT_EXPECTED(Locked2.tryLock(), HasValue(false));
   }
 
   // Now that the lock is out of scope, the file should be gone.
@@ -68,7 +69,7 @@ TEST(LockFileManagerTest, LinkLockExists) {
     // The lock file doesn't point to a real file, so we should successfully
     // acquire it.
     LockFileManager Locked(LockedFile);
-    EXPECT_EQ(LockFileManager::LFS_Owned, Locked.getState());
+    EXPECT_THAT_EXPECTED(Locked.tryLock(), HasValue(true));
   }
 
   // Now that the lock is out of scope, the file should be gone.
@@ -93,7 +94,7 @@ TEST(LockFileManagerTest, RelativePath) {
   {
     // The lock file should not exist, so we should successfully acquire it.
     LockFileManager Locked(LockedFile);
-    EXPECT_EQ(LockFileManager::LFS_Owned, Locked.getState());
+    EXPECT_THAT_EXPECTED(Locked.tryLock(), HasValue(true));
     EXPECT_TRUE(sys::fs::exists(FileLock.str()));
   }
 

>From 797d0a41f1fdc0fc406dfd318ca401a84233795e Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 10 Mar 2025 15:44:28 -0700
Subject: [PATCH 4/5] clang-format

---
 llvm/include/llvm/Support/LockFileManager.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index 364d4c360c8ee..aa8fa78e7664e 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -43,7 +43,7 @@ class LockFileManager {
   SmallString<128> UniqueLockFileName;
 
   struct OwnerUnknown {};
-  struct OwnedByUs{};
+  struct OwnedByUs {};
   struct OwnedByAnother {
     std::string OwnerHostName;
     int OwnerPID;

>From ad7c25bb9042de52ae8eb055aeedd288e193053c Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 10 Mar 2025 15:58:17 -0700
Subject: [PATCH 5/5] Add missing includes

---
 llvm/include/llvm/Support/LockFileManager.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index aa8fa78e7664e..8c800acc9461f 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -11,8 +11,9 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Error.h"
 #include <optional>
+#include <string>
 #include <system_error>
-#include <utility> // for std::pair
+#include <variant>
 
 namespace llvm {
 class StringRef;



More information about the cfe-commits mailing list