On Mon, Sep 12, 2011 at 3:57 PM, Caitlin Sadowski <[email protected]>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 <[email protected]>
> wrote:
> > On Fri, Sep 9, 2011 at 9:07 AM, Caitlin Sadowski <[email protected]>
> > 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.
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
