[cfe-commits] r139368 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp test/SemaCXX/warn-thread-safety-analysis.cpp

[Chandler Carruth]

On Mon, Sep 12, 2011 at 3:57 PM, Caitlin Sadowski <supertri at google.com>wrote:

> Here is patch that goes back to the old separate diagnostics.
>

Generally fine, but one nit-pick:

diff --git a/lib/Sema/AnalysisBasedWarnings.cpp
b/lib/Sema/AnalysisBasedWarnings.cpp
index 6a7c5d3..37f4ffc 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -689,9 +689,21 @@ class ThreadSafetyReporter : public
clang::thread_safety::ThreadSafetyHandler {

   void handleNoMutexHeld(const NamedDecl *D, ProtectedOperationKind POK,
                          AccessKind AK, SourceLocation Loc) {
-    // FIXME: It would be nice if this case printed without single quotes
around
-    // the phrase 'any mutex'
-    handleMutexNotHeld(D, POK, "any mutex", getLockKindFromAccessKind(AK),
Loc);
+    assert(POK != POK_FunctionCall);
+    unsigned DiagID;
+    switch (POK) {

After the assert above, a switch seems heavy handed. If there are only ever
likely to be two candidates, then assert the *positive* cases and then use a
ternary:

assert((POK == POK_VarAccess || POK == POK_VarDereference) &&
       "...");
unsigned DiagID = POK == POK_VarAccess? ... : ...;

If there are likely to be more than 2, then I would make the switch actually
cover all cases and 'assert(0 && ...)' on each case that cannot happen.
Switch + default == bad. =]

+      case POK_VarAccess:
+        DiagID = diag::warn_variable_requires_any_lock;
+        break;
+      case POK_VarDereference:
+        DiagID = diag::warn_var_deref_requires_any_lock;
+        break;
+      default:
+        break;
+    }
+    PartialDiagnostic Warning = S.PDiag(DiagID)
+      << D->getName() << getLockKindFromAccessKind(AK);
+    Warnings.push_back(DelayedDiag(Loc, Warning));
   }



>
> Cheers,
>
> Caitlin
>
> On Fri, Sep 9, 2011 at 9:13 AM, Chandler Carruth <chandlerc at google.com>
> wrote:
> > On Fri, Sep 9, 2011 at 9:07 AM, Caitlin Sadowski <supertri at google.com>
> > wrote:
> >>
> >> +    // FIXME: It would be nice if this case printed without single
> quotes
> >> around
> >> +    // the phrase 'any mutex'
> >
> > I think you should use two diagnostics, or use a %select in the message.
> The
> > C++ code shouldn't be providing "any mutex", all the text should come
> from
> > the diagnostic message table.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110912/a0fa6b61/attachment.html>