Thread safety analysis: Improve documentation for scoped capabilities

[aaronpuchert]

Summary

They are for more powerful than the current documentation implies, this
adds

• adopting a lock,
• deferring a lock,
• manually unlocking the scoped capability,
• relocking the scoped capability, possibly in a different mode,
• try-relocking the scoped capability.

Also there is now a generic explanation how attributes on scoped
capabilities work. There has been confusion in the past about how to
annotate them (see e.g. PR33504), hopefully this clears things up.

[aaronpuchert]

Not sure about the added comments, I can also remove them if they're not helpful.

@aaron.ballman, this addresses your earlier comment D81332#2079489.

[aaron.ballman]

clang/docs/ThreadSafetyAnalysis.rst

928

This makes it sound like we're missing a diagnostic, but IIRC, this is intended behavior. Maybe we want to say there is purposefully no warning on double unlock and why? Or add a FIXME if I'm remembering wrong and this isn't intended behavior.

[aaronpuchert]

clang/docs/ThreadSafetyAnalysis.rst

914

s/t_mutex/Mutex/g

928

It is intentional (we have special handling for the destructor), and I also think it makes sense. I just wanted to clarify the difference between this and Unlock: calling Unlock twice is not allowed, but calling Unlock and then (implicitly) the destructor is allowed.

I could rephrase this as "There is no warning if the scope was already unlocked before." Could also add an "intentional" or "deliberate", but since it's documented I think the reader can infer that.

[aaronpuchert]

• More detailed description how scoped capabilities work.
• Make the comment wording more consistent with existing comments and the previous explanation.
• Properly implement the destructor in the presence of an Unlock() function: we need to keep track of the status then.
• Add try-acquire functions on scoped lock.
• Fix compiler errors.

[aaron.ballman]

Ah, this reads more clearly to me, thank you for the additional changes. LGTM!

clang/docs/ThreadSafetyAnalysis.rst

 *Previously*: ``SCOPED_LOCKABLE``
 
 ``SCOPED_CAPABILITY`` is an attribute on classes that implement RAII-style
 locking, in which a capability is acquired in the constructor, and released in
 the destructor.  Such classes require special handling because the constructor
 and destructor refer to the capability via different names; see the
 ``MutexLocker`` class in :ref:`mutexheader`, below.
 
+Scoped capabilities are treated as capabilities that are implicitly acquired
+on construction and released on destruction. They are associated with
+the set of (regular) capabilities named in thread safety attributes on the
+constructor. Acquire-type attributes on other member functions are treated as
+applying to that set of associated capabilities, while ``RELEASE`` implies that
+a function releases all associated capabilities in whatever mode they're held.
+
 
 TRY_ACQUIRE(<bool>, ...), TRY_ACQUIRE_SHARED(<bool>, ...)
 ---------------------------------------------------------
 
 *Previously:* ``EXCLUSIVE_TRYLOCK_FUNCTION``, ``SHARED_TRYLOCK_FUNCTION``
 
 These are attributes on a function or method that tries to acquire the given
 capability, and returns a boolean value indicating success or failure.
public:
 
     // Assert that is mutex is currently held for read operations.
     void AssertReaderHeld() ASSERT_SHARED_CAPABILITY(this);
     
     // For negative capabilities.
     const Mutex& operator!() const { return *this; }
   };
 
+  // Tag types for selecting a constructor.
+  struct adopt_lock_t {} inline constexpr adopt_lock = {};
+  struct defer_lock_t {} inline constexpr defer_lock = {};
+  struct shared_lock_t {} inline constexpr shared_lock = {};
 
   // MutexLocker is an RAII class that acquires a mutex in its constructor, and
   // releases it in its destructor.
   class SCOPED_CAPABILITY MutexLocker {
   private:
     Mutex* mut;
+    bool locked;
 
   public:
-    MutexLocker(Mutex *mu) ACQUIRE(mu) : mut(mu) {
+    // Acquire mu, implicitly acquire *this and associate it with mu.
+    MutexLocker(Mutex *mu) ACQUIRE(mu) : mut(mu), locked(true) {
       mu->Lock();
     }
+
+    // Assume mu is held, implicitly acquire *this and associate it with mu.
+    MutexLocker(Mutex *mu, adopt_lock_t) REQUIRES(mu) : mut(mu), locked(true) {}
+
+    // Acquire mu in shared mode, implicitly acquire *this and associate it with mu.
+    MutexLocker(Mutex *mu, shared_lock_t) ACQUIRE_SHARED(mu) : mut(mu), locked(true) {
+      mu->ReaderLock();
+    }
+
+    // Assume mu is held in shared mode, implicitly acquire *this and associate it with mu.
+    MutexLocker(Mutex *mu, adopt_lock_t, shared_lock_t) REQUIRES_SHARED(mu)
+      : mut(mu), locked(true) {}
+
+    // Assume mu is not held, implicitly acquire *this and associate it with mu.
+    MutexLocker(Mutex *mu, defer_lock_t) EXCLUDES(mu) : mut(mu), locked(false) {}
+
+    // Release *this and all associated mutexes, if they are still held.
+    // There is no warning if the scope was already unlocked before.
     ~MutexLocker() RELEASE() {
+      if (locked)
+        mut->GenericUnlock();
+    }
+
+    // Acquire all associated mutexes exclusively.
+    void Lock() ACQUIRE() {
+      mut->Lock();
+      locked = true;
+    }
+
+    // Try to acquire all associated mutexes exclusively.
+    bool TryLock() TRY_ACQUIRE(true) {
+      return locked = mut->TryLock();
+    }
+
+    // Acquire all associated mutexes in shared mode.
+    void ReaderLock() ACQUIRE_SHARED() {
+      mut->ReaderLock();
+      locked = true;
+    }
+
+    // Try to acquire all associated mutexes in shared mode.
+    bool ReaderTryLock() TRY_ACQUIRE_SHARED(true) {
+      return locked = mut->ReaderTryLock();
+    }
+
+    // Release all associated mutexes. Warn on double unlock.
+    void Unlock() RELEASE() {
       mut->Unlock();
+      locked = false;
+    }
+
+    // Release all associated mutexes. Warn on double unlock.
+    void ReaderUnlock() RELEASE() {
+      mut->ReaderUnlock();
+      locked = false;
     }
   };
 
 
   #ifdef USE_LOCK_STYLE_THREAD_SAFETY_ATTRIBUTES
   // The original version of thread safety analysis the following attribute
   // definitions.  These use a lock-based terminology.  They are still in use
   // by existing thread safety code, and will continue to be supported.