|
|
Patch Set 1 #
Total comments: 6
Total messages: 4
|
delesley
http://codereview.appspot.com/4968075/diff/1/lib/Analysis/ThreadSafety.cpp File lib/Analysis/ThreadSafety.cpp (right): http://codereview.appspot.com/4968075/diff/1/lib/Analysis/ThreadSafety.cpp#newcode172 lib/Analysis/ThreadSafety.cpp:172: buildMutexID(Parent, Parent); This should be buildMutexID(Parent, 0).
|
14 years, 4 months ago (2011年09月07日 20:10:10 UTC) #1 | |||||||||||||||||||||||||||||||||||||||||||
http://codereview.appspot.com/4968075/diff/1/lib/Analysis/ThreadSafety.cpp File lib/Analysis/ThreadSafety.cpp (right): http://codereview.appspot.com/4968075/diff/1/lib/Analysis/ThreadSafety.cpp#ne... lib/Analysis/ThreadSafety.cpp:172: buildMutexID(Parent, Parent); This should be buildMutexID(Parent, 0).
http://codereview.appspot.com/4968075/diff/1/lib/Analysis/ThreadSafety.cpp File lib/Analysis/ThreadSafety.cpp (right): http://codereview.appspot.com/4968075/diff/1/lib/Analysis/ThreadSafety.cpp#ne... lib/Analysis/ThreadSafety.cpp:172: buildMutexID(Parent, Parent); On 2011年09月07日 20:10:11, delesley wrote: > This should be buildMutexID(Parent, 0). Done.
Good to go w/ these comments addressed. http://codereview.appspot.com/4968075/diff/1/include/clang/Analysis/Analyses/... File include/clang/Analysis/Analyses/ThreadSafety.h (right): http://codereview.appspot.com/4968075/diff/1/include/clang/Analysis/Analyses/... include/clang/Analysis/Analyses/ThreadSafety.h:50: virtual void handleBogusLockExp(SourceLocation Loc) {} "Bogus" isn't really a good technical term here. ;] How about "Invalid"? http://codereview.appspot.com/4968075/diff/1/include/clang/Basic/DiagnosticSe... File include/clang/Basic/DiagnosticSemaKinds.td (right): http://codereview.appspot.com/4968075/diff/1/include/clang/Basic/DiagnosticSe... include/clang/Basic/DiagnosticSemaKinds.td:1427: "cannot resolve lock expression">, This doesn't really tell the user what went wrong. Why not? What does it even mean to ben 'unable to resolve'? Not sure how much we can do here right away. Just explaining what resolve means might help: "cannot resolve lock expression to a lockable object" or some such..
Committed! http://codereview.appspot.com/4968075/diff/1/include/clang/Analysis/Analyses/... File include/clang/Analysis/Analyses/ThreadSafety.h (right): http://codereview.appspot.com/4968075/diff/1/include/clang/Analysis/Analyses/... include/clang/Analysis/Analyses/ThreadSafety.h:50: virtual void handleBogusLockExp(SourceLocation Loc) {} On 2011年09月09日 03:50:18, chandlerc wrote: > "Bogus" isn't really a good technical term here. ;] How about "Invalid"? Done. http://codereview.appspot.com/4968075/diff/1/include/clang/Basic/DiagnosticSe... File include/clang/Basic/DiagnosticSemaKinds.td (right): http://codereview.appspot.com/4968075/diff/1/include/clang/Basic/DiagnosticSe... include/clang/Basic/DiagnosticSemaKinds.td:1427: "cannot resolve lock expression">, On 2011年09月09日 03:50:18, chandlerc wrote: > This doesn't really tell the user what went wrong. Why not? What does it even > mean to ben 'unable to resolve'? Not sure how much we can do here right away. > Just explaining what resolve means might help: "cannot resolve lock expression > to a lockable object" or some such.. Done.