[clang] 8ca00c5 - Thread safety analysis: More consistent warning message

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-09-01T23:16:05+02:00
New Revision: 8ca00c5cdc0b86a433b80db633f3ff46e6547895

URL: https://github.com/llvm/llvm-project/commit/8ca00c5cdc0b86a433b80db633f3ff46e6547895
DIFF: https://github.com/llvm/llvm-project/commit/8ca00c5cdc0b86a433b80db633f3ff46e6547895.diff

LOG: Thread safety analysis: More consistent warning message

Other warning messages for negative capabilities also mention their
kind, and the double space was ugly.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D84603

Added: 
    

Modified: 
    clang/include/clang/Analysis/Analyses/ThreadSafety.h
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Analysis/ThreadSafety.cpp
    clang/lib/Sema/AnalysisBasedWarnings.cpp
    clang/test/SemaCXX/warn-thread-safety-analysis.cpp
    clang/test/SemaCXX/warn-thread-safety-negative.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0d3dda1256fbc..bfa9870a1e1f0 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -202,6 +202,14 @@ class ThreadSafetyHandler {
   virtual void handleNegativeNotHeld(StringRef Kind, Name LockName, Name Neg,
                                      SourceLocation Loc) {}
 
+  /// Warn when calling a function that a negative capability is not held.
+  /// \param D -- The decl for the function requiring the negative capability.
+  /// \param LockName -- The name for the lock expression, to be printed in the
+  /// diagnostic.
+  /// \param Loc -- The location of the protected operation.
+  virtual void handleNegativeNotHeld(const NamedDecl *D, Name LockName,
+                                     SourceLocation Loc) {}
+
   /// Warn when a function is called while an excluded mutex is locked. For
   /// example, the mutex may be locked inside the function.
   /// \param Kind -- the capability's name parameter (role, mutex, etc).

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7683d24d2821b..d856f784e0eea 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3477,6 +3477,9 @@ def warn_acquired_before_after_cycle : Warning<
 def warn_acquire_requires_negative_cap : Warning<
   "acquiring %0 '%1' requires negative capability '%2'">,
   InGroup<ThreadSafetyNegative>, DefaultIgnore;
+def warn_fun_requires_negative_cap : Warning<
+  "calling function %0 requires negative capability '%1'">,
+  InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
 
 // Thread safety warnings on pass by reference
 def warn_guarded_pass_by_reference : Warning<

diff  --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 1208eaf93e25d..64e0da9e64b12 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1641,8 +1641,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
     // Otherwise the negative requirement must be propagated to the caller.
     LDat = FSet.findLock(Analyzer->FactMan, Cp);
     if (!LDat) {
-      Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(),
-                                           LK_Shared, Loc);
+      Analyzer->Handler.handleNegativeNotHeld(D, Cp.toString(), Loc);
     }
     return;
   }

diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 5cc215e08ea83..37fd26d7c22d7 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1892,6 +1892,13 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
     Warnings.emplace_back(std::move(Warning), getNotes());
   }
 
+  void handleNegativeNotHeld(const NamedDecl *D, Name LockName,
+                             SourceLocation Loc) override {
+    PartialDiagnosticAt Warning(
+        Loc, S.PDiag(diag::warn_fun_requires_negative_cap) << D << LockName);
+    Warnings.emplace_back(std::move(Warning), getNotes());
+  }
+
   void handleFunExcludesLock(StringRef Kind, Name FunName, Name LockName,
                              SourceLocation Loc) override {
     PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_fun_excludes_mutex)

diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index bbcdd5b9e5f0c..91bd15def577d 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -4985,7 +4985,7 @@ class Foo {
   }
 
   void bar() {
-    bar2();       // expected-warning {{calling function 'bar2' requires holding  '!mu'}}
+    bar2();       // expected-warning {{calling function 'bar2' requires negative capability '!mu'}}
   }
 
   void bar2() EXCLUSIVE_LOCKS_REQUIRED(!mu) {

diff  --git a/clang/test/SemaCXX/warn-thread-safety-negative.cpp b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
index 9a495fa6cff9a..456fe16e6574e 100644
--- a/clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -50,7 +50,7 @@ class Foo {
   }
 
   void bar() {
-    baz();        // expected-warning {{calling function 'baz' requires holding  '!mu'}}
+    baz();        // expected-warning {{calling function 'baz' requires negative capability '!mu'}}
   }
 
   void baz() EXCLUSIVE_LOCKS_REQUIRED(!mu) {