aaronpuchert added a comment.

Imagine having a producer loop, where we check a loop condition while holding a 
mutex, but release it in the loop body to let other producers/consumers do 
their work. In that scenario it makes sense to allow "relocking" a scope.

  RelockableScope Scope(mu);
  while (WorkToDo()) {
      Scope.Unlock();
      // Produce something...
      Scope.Lock();
      PublishWork();
  }



================
Comment at: lib/Analysis/ThreadSafety.cpp:960-961
+        FSet.removeLock(FactMan, !UnderCp);
+        FSet.addLock(FactMan, llvm::make_unique<LockableFactEntry>(UnderCp, LK,
+                                                                   RelockLoc));
+      }
----------------
Looks a bit weird, but `clang-format` told me to do that.


================
Comment at: lib/Analysis/ThreadSafety.cpp:1318-1321
+    // FIXME: It's possible to manually destruct the scope and then relock it.
+    // Should that be a separate warning? For now we pretend nothing happened.
+    // It's undefined behavior, so not related to thread safety.
+    return;
----------------
Not sure about this part, but it's probably not worth worrying about. A user 
would have to call a member function on a scoped object after manually ending 
its lifetime by calling the destructor, see test 
`RelockableScopedLock::destructLock`.


================
Comment at: lib/Analysis/ThreadSafety.cpp:1324-1325
+
+  // We should only land here if Cp is a scoped capability, so if we have any
+  // fact, it must be a ScopedLockableFactEntry.
+  auto SLDat = static_cast<const ScopedLockableFactEntry *>(LDat);
----------------
I hope this reasoning is sufficient to justify the following static_cast, 
otherwise I need to introduce LLVM-style RTTI into `FactEntry`.


Repository:
  rC Clang

https://reviews.llvm.org/D49885



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to