Adding support for guarded_by/var and pt_guarded_by/var annotations

[chandlerc, 2011-08-29 17:52:41]

http://codereview.appspot.com/4954041/diff/1/include/clang/Basic/DiagnosticSe...
File include/clang/Basic/DiagnosticSemaKinds.td (right):

http://codereview.appspot.com/4954041/diff/1/include/clang/Basic/DiagnosticSe...
include/clang/Basic/DiagnosticSemaKinds.td:1411: "accessing dereferenced
variable '%0' requires lock '%1'">,
I wonder if this would be read better as:

accessing the value pointed to by variable '...' requires lock '...'

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

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:823: /// \param LockExp The lock expression
corresponding to the lock to be added
Why did this comment leave?

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

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:855: if (DR)
Always put these inside the condition:

if (const DeclRefExpr *DR = dyn_cast<...>(...))
  return DR->getDecl();

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:862: return NULL;
s/NULL/0/

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:867: /// \param Exp An expression that has
been read or written.
Parameter commets shouldn't be placed in the middle of the comment block.

This entire comment is really hard to read. It could probably be cleaned up some
to flow better and/or be formatted better.

[supertri, 2011-08-29 18:40:51]

Made these changes. Ready to commit?

http://codereview.appspot.com/4954041/diff/1/include/clang/Basic/DiagnosticSe...
File include/clang/Basic/DiagnosticSemaKinds.td (right):

http://codereview.appspot.com/4954041/diff/1/include/clang/Basic/DiagnosticSe...
include/clang/Basic/DiagnosticSemaKinds.td:1411: "accessing dereferenced
variable '%0' requires lock '%1'">,
On 2011/08/29 17:52:41, chandlerc wrote:
> I wonder if this would be read better as:
> 
> accessing the value pointed to by variable '...' requires lock '...'

Changed to:

accessing the value pointed to by '...' requires lock '...'

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

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:823: /// \param LockExp The lock expression
corresponding to the lock to be added
On 2011/08/29 17:52:41, chandlerc wrote:
> Why did this comment leave?

Comment is back!

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

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:855: if (DR)
On 2011/08/29 17:52:41, chandlerc wrote:
> Always put these inside the condition:
> 
> if (const DeclRefExpr *DR = dyn_cast<...>(...))
>   return DR->getDecl();

Done.

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:862: return NULL;
On 2011/08/29 17:52:41, chandlerc wrote:
> s/NULL/0/

Done.

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:867: /// \param Exp An expression that has
been read or written.
On 2011/08/29 17:52:41, chandlerc wrote:
> Parameter commets shouldn't be placed in the middle of the comment block.
> 
> This entire comment is really hard to read. It could probably be cleaned up
some
> to flow better and/or be formatted better.

I deleted the "However" block and re-arranged the comment.

[chandlerc, 2011-08-29 20:31:14]

LGTM

(modulo the comment below)

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

http://codereview.appspot.com/4954041/diff/1/lib/Sema/AnalysisBasedWarnings.c...
lib/Sema/AnalysisBasedWarnings.cpp:867: /// \param Exp An expression that has
been read or written.
On 2011/08/29 18:40:51, supertri wrote:
> On 2011/08/29 17:52:41, chandlerc wrote:
> > Parameter commets shouldn't be placed in the middle of the comment block.
> > 
> > This entire comment is really hard to read. It could probably be cleaned up
> some
> > to flow better and/or be formatted better.
> 
> I deleted the "However" block and re-arranged the comment.

I think we do need to document the limitations of this approach as currently
implemented.