Expand the set of expressions that can be used to denote locks.

[Richard Smith, 2012-03-03 00:12:46]

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp
File lib/Analysis/ThreadSafety.cpp (right):

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:92: unsigned NumArgs, Expr **FunArgs) {
This function should probably eventually be recast as a StmtVisitor -- no need
to do so yet, but something to keep in mind.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:134: Expr** CallArgs = CMCE->getArgs();
' **' not '** ', please.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:135: for (unsigned i = 0; i < NumCallArgs; ++i) {
'I', not 'i'.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:137: }
No need for braces here.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:139: buildMutexID(CE->getCallee(), D, Parent,
NumArgs, FunArgs);
Can you unify this with the CXXMemberCallExpr case? (This should be
straightforward since the code is so similar and CXXMemberCallExpr derives from
CallExpr).

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:151: buildMutexID(CE->getSubExpr(), D, Parent,
NumArgs, FunArgs);
I imagine you should also be skipping past ParenExprs. You could replace this
check with an 'Exp = Exp->IgnoreParenCasts();' at the top of this function.

http://codereview.appspot.com/5695049/diff/1/test/SemaCXX/warn-thread-safety-...
File test/SemaCXX/warn-thread-safety-analysis.cpp (right):

http://codereview.appspot.com/5695049/diff/1/test/SemaCXX/warn-thread-safety-...
test/SemaCXX/warn-thread-safety-analysis.cpp:778: // FIXME -- derive new tests
for unhandled expressions
How about __attribute__((guarded_by((throw, mua[0])))) ?

[delesley, 2012-03-05 18:30:11]

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp
File lib/Analysis/ThreadSafety.cpp (right):

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:92: unsigned NumArgs, Expr **FunArgs) {

Agreed.  I need to redo the whole thing -- I'll do it then.

On 2012/03/03 00:12:46, Richard Smith wrote:
> This function should probably eventually be recast as a StmtVisitor -- no need
> to do so yet, but something to keep in mind.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:134: Expr** CallArgs = CMCE->getArgs();
On 2012/03/03 00:12:46, Richard Smith wrote:
> ' **' not '** ', please.

Done.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:135: for (unsigned i = 0; i < NumCallArgs; ++i) {
Action pending result of upcoming duel with Chandler.  

On 2012/03/03 00:12:46, Richard Smith wrote:
> 'I', not 'i'.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:137: }
On 2012/03/03 00:12:46, Richard Smith wrote:
> No need for braces here.

Done.

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:139: buildMutexID(CE->getCallee(), D, Parent,
NumArgs, FunArgs);
Not easily.  I can unify the for loop, but the rest of the code is different, so
I'll still need to check which one I'm processing.  

On 2012/03/03 00:12:46, Richard Smith wrote:
> Can you unify this with the CXXMemberCallExpr case? (This should be
> straightforward since the code is so similar and CXXMemberCallExpr derives
from
> CallExpr).

http://codereview.appspot.com/5695049/diff/1/lib/Analysis/ThreadSafety.cpp#ne...
lib/Analysis/ThreadSafety.cpp:151: buildMutexID(CE->getSubExpr(), D, Parent,
NumArgs, FunArgs);
On 2012/03/03 00:12:46, Richard Smith wrote:
> I imagine you should also be skipping past ParenExprs. You could replace this
> check with an 'Exp = Exp->IgnoreParenCasts();' at the top of this function.

Done.