[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8544defdcb09: Thread safety analysis: Document how 
try-acquire is handled (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87065/new/

https://reviews.llvm.org/D87065

Files:
  clang/docs/ThreadSafetyAnalysis.rst


Index: clang/docs/ThreadSafetyAnalysis.rst
===
--- clang/docs/ThreadSafetyAnalysis.rst
+++ clang/docs/ThreadSafetyAnalysis.rst
@@ -414,6 +414,26 @@
 indicates success, and the remaining arguments are interpreted in the same way
 as ``ACQUIRE``.  See :ref:`mutexheader`, below, for example uses.
 
+Because the analysis doesn't support conditional locking, a capability is
+treated as acquired after the first branch on the return value of a try-acquire
+function.
+
+.. code-block:: c++
+
+  Mutex mu;
+  int a GUARDED_BY(mu);
+
+  void foo() {
+bool success = mu.TryLock();
+a = 0; // Warning, mu is not locked.
+if (success) {
+  a = 0;   // Ok.
+  mu.Unlock();
+} else {
+  a = 0;   // Warning, mu is not locked.
+}
+  }
+
 
 ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
 


Index: clang/docs/ThreadSafetyAnalysis.rst
===
--- clang/docs/ThreadSafetyAnalysis.rst
+++ clang/docs/ThreadSafetyAnalysis.rst
@@ -414,6 +414,26 @@
 indicates success, and the remaining arguments are interpreted in the same way
 as ``ACQUIRE``.  See :ref:`mutexheader`, below, for example uses.
 
+Because the analysis doesn't support conditional locking, a capability is
+treated as acquired after the first branch on the return value of a try-acquire
+function.
+
+.. code-block:: c++
+
+  Mutex mu;
+  int a GUARDED_BY(mu);
+
+  void foo() {
+bool success = mu.TryLock();
+a = 0; // Warning, mu is not locked.
+if (success) {
+  a = 0;   // Ok.
+  mu.Unlock();
+} else {
+  a = 0;   // Warning, mu is not locked.
+}
+  }
+
 
 ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for documenting this, that would be confusing without spelling 
it out explicitly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87065/new/

https://reviews.llvm.org/D87065

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

[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aaronpuchert requested review of this revision.

I don't think this is obvious, since try-acquire seemingly contradicts
our usual requirements of "no conditional locking".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87065

Files:
  clang/docs/ThreadSafetyAnalysis.rst


Index: clang/docs/ThreadSafetyAnalysis.rst
===
--- clang/docs/ThreadSafetyAnalysis.rst
+++ clang/docs/ThreadSafetyAnalysis.rst
@@ -414,6 +414,26 @@
 indicates success, and the remaining arguments are interpreted in the same way
 as ``ACQUIRE``.  See :ref:`mutexheader`, below, for example uses.
 
+Because the analysis doesn't support conditional locking, a capability is
+treated as acquired after the first branch on the return value of a try-acquire
+function.
+
+.. code-block:: c++
+
+  Mutex mu;
+  int a GUARDED_BY(mu);
+
+  void foo() {
+bool success = mu.TryLock();
+a = 0; // Warning, mu is not locked.
+if (success) {
+  a = 0;   // Ok.
+  mu.Unlock();
+} else {
+  a = 0;   // Warning, mu is not locked.
+}
+  }
+
 
 ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
 


Index: clang/docs/ThreadSafetyAnalysis.rst
===
--- clang/docs/ThreadSafetyAnalysis.rst
+++ clang/docs/ThreadSafetyAnalysis.rst
@@ -414,6 +414,26 @@
 indicates success, and the remaining arguments are interpreted in the same way
 as ``ACQUIRE``.  See :ref:`mutexheader`, below, for example uses.
 
+Because the analysis doesn't support conditional locking, a capability is
+treated as acquired after the first branch on the return value of a try-acquire
+function.
+
+.. code-block:: c++
+
+  Mutex mu;
+  int a GUARDED_BY(mu);
+
+  void foo() {
+bool success = mu.TryLock();
+a = 0; // Warning, mu is not locked.
+if (success) {
+  a = 0;   // Ok.
+  mu.Unlock();
+} else {
+  a = 0;   // Warning, mu is not locked.
+}
+  }
+
 
 ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits