Refactoring error messages

[chandlerc, 2011-09-15 00:40:53]

LGTM w/ the comments below...

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
File include/clang/Analysis/Analyses/ThreadSafety.h (right):

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:63: /// \enum
SK_LockedSomePredecessors -- a mutex is locked in some but not all
I wonder if there is a less technical description here... LockedSomePaths?

Also, rather than "Some" maybe we want a better word. We want a word that more
clearly is strictly greater than zero, and strictly less than all... Not coming
up with anything better than "some" though...

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:66: /// function.
Same enum comment comment as the other patch...

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:99: /// 2, or a mutex is only
held at the start of some loop iterations,
Typically formal lists place the conjunction on the prior item:

1. Foo is true, or
2. Bar is true, or
3. Baz is true.

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:104: /// \param LEK -- which of
the three above cases we should warn for
ErrorKind?

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

http://codereview.appspot.com/4964080/diff/1/include/clang/Basic/DiagnosticSe...
include/clang/Basic/DiagnosticSemaKinds.td:1398: "mutex '%0' is still held at
the end of function '%1'">,
I'm sad to see this bit of information leave the diagnostic... can we get it
back?

[supertri, 2011-09-15 17:21:28]

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
File include/clang/Analysis/Analyses/ThreadSafety.h (right):

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:63: /// \enum
SK_LockedSomePredecessors -- a mutex is locked in some but not all
On 2011/09/15 00:40:53, chandlerc wrote:
> I wonder if there is a less technical description here... LockedSomePaths?
> 
> Also, rather than "Some" maybe we want a better word. We want a word that more
> clearly is strictly greater than zero, and strictly less than all... Not
coming
> up with anything better than "some" though...

Also could not come up with anything better (and neither could the person
sitting next to me on the bus). Changed to LockedSomePaths.

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:66: /// function.
On 2011/09/15 00:40:53, chandlerc wrote:
> Same enum comment comment as the other patch...

Done.

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:99: /// 2, or a mutex is only
held at the start of some loop iterations,
On 2011/09/15 00:40:53, chandlerc wrote:
> Typically formal lists place the conjunction on the prior item:
> 
> 1. Foo is true, or
> 2. Bar is true, or
> 3. Baz is true.

Yes, yes they do.

http://codereview.appspot.com/4964080/diff/1/include/clang/Analysis/Analyses/...
include/clang/Analysis/Analyses/ThreadSafety.h:104: /// \param LEK -- which of
the three above cases we should warn for
On 2011/09/15 00:40:53, chandlerc wrote:
> ErrorKind?

Done.

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

http://codereview.appspot.com/4964080/diff/1/include/clang/Basic/DiagnosticSe...
include/clang/Basic/DiagnosticSemaKinds.td:1398: "mutex '%0' is still held at
the end of function '%1'">,
On 2011/09/15 00:40:53, chandlerc wrote:
> I'm sad to see this bit of information leave the diagnostic... can we get it
> back?

Added a FIXME for now. It may be easier to put back after subsequent refactoring
changes. Once DeLesley has finished implementing code to deal with all gcc test
cases, you guys should think carefully about the error message text.