Thread safety: Addin locks required and excluded attributes

[chandlerc, 2011-09-02 06:41:53]

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.cpp
File lib/Sema/AnalysisBasedWarnings.cpp (right):

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:1139: // case, or decide not to.
I don't think this duplication is enough to really be worth it.

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:1146: warnIfLockNotHeld(D, Exp, WRITTEN,
Lock, diag::warn_fun_requires_lock);
80-columns

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:1177: case attr::LockReturned: {
No curlies needed..

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

http://codereview.appspot.com/4964061/diff/1/test/SemaCXX/warn-thread-safety-...
test/SemaCXX/warn-thread-safety-analysis.cpp:245: void aa_elr_fun()
__attribute__((exclusive_locks_required(aa_mu)));
I'm a bit surprised at removing these tests...

[supertri, 2011-09-02 22:23:25]

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.cpp
File lib/Sema/AnalysisBasedWarnings.cpp (right):

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:1139: // case, or decide not to.
On 2011/09/02 06:41:53, chandlerc wrote:
> I don't think this duplication is enough to really be worth it.

Done.

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:1146: warnIfLockNotHeld(D, Exp, WRITTEN,
Lock, diag::warn_fun_requires_lock);
On 2011/09/02 06:41:53, chandlerc wrote:
> 80-columns

Done.

http://codereview.appspot.com/4964061/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:1177: case attr::LockReturned: {
On 2011/09/02 06:41:53, chandlerc wrote:
> No curlies needed..

Done.

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

http://codereview.appspot.com/4964061/diff/1/test/SemaCXX/warn-thread-safety-...
test/SemaCXX/warn-thread-safety-analysis.cpp:245: void aa_elr_fun()
__attribute__((exclusive_locks_required(aa_mu)));
On 2011/09/02 06:41:53, chandlerc wrote:
> I'm a bit surprised at removing these tests...

I thought I had just moved them down, but they may have been lost in the massive
rebase to separate out patches. I added them again at the bottom.

[chandlerc, 2011-09-02 23:10:56]

You also need to audit your variables. Many remain "LT" or "AT" and need to
become "LK" or "AK". Should be good to commit once that's done.

http://codereview.appspot.com/4964061/diff/3001/lib/Sema/AnalysisBasedWarning...
File lib/Sema/AnalysisBasedWarnings.cpp (right):

http://codereview.appspot.com/4964061/diff/3001/lib/Sema/AnalysisBasedWarning...
lib/Sema/AnalysisBasedWarnings.cpp:781: LK_SHARED,
As I said previously: LK_Shared

http://codereview.appspot.com/4964061/diff/3001/lib/Sema/AnalysisBasedWarning...
lib/Sema/AnalysisBasedWarnings.cpp:856: bool locksetContains(LockID Lock,
LockKind LockRequested) const {
KindRequested?

[chandlerc, 2011-09-02 23:14:32]

On 2011/09/02 23:10:56, chandlerc wrote:
> You also need to audit your variables. Many remain "LT" or "AT" and need to
> become "LK" or "AK". Should be good to commit once that's done.
> 
>
http://codereview.appspot.com/4964061/diff/3001/lib/Sema/AnalysisBasedWarning...
> File lib/Sema/AnalysisBasedWarnings.cpp (right):
> 
>
http://codereview.appspot.com/4964061/diff/3001/lib/Sema/AnalysisBasedWarning...
> lib/Sema/AnalysisBasedWarnings.cpp:781: LK_SHARED,
> As I said previously: LK_Shared
> 
>
http://codereview.appspot.com/4964061/diff/3001/lib/Sema/AnalysisBasedWarning...
> lib/Sema/AnalysisBasedWarnings.cpp:856: bool locksetContains(LockID Lock,
> LockKind LockRequested) const {
> KindRequested?

Ignore these comments. Wrong thread. They've been moved.