[PATCH] Add a level parameter to ClangTidyCheck::diag.

[Peter Collingbourne]

On Sun, Mar 02, 2014 at 04:59:27PM +0000, Alp Toker wrote:
>
> On 02/03/2014 16:18, Alp Toker wrote:
>>
>> On 02/03/2014 12:25, Peter Collingbourne wrote:
>>>
>>> Index: clang-tidy/ClangTidy.h
>>> ===================================================================
>>> --- clang-tidy/ClangTidy.h
>>> +++ clang-tidy/ClangTidy.h
>>> @@ -76,7 +76,8 @@
>>>     void setContext(ClangTidyContext *Ctx) { Context = Ctx; }
>>>       /// \brief Add a diagnostic with the check's name.
>>> -  DiagnosticBuilder diag(SourceLocation Loc, StringRef Description);
>>> +  DiagnosticBuilder diag(SourceLocation Loc, StringRef Description,
>>> +                         DiagnosticIDs::Level Level =  
>>> DiagnosticIDs::Warning);
>>
>> Could you order the parameters Loc, Level, FormatString and drop the  
>> default argument?
>>
>> That'll provide visual consistency with the output as well as internal  
>> consistency with clang's own getCustomDiagID(Level L, StringRef  
>> FormatString).
>>
>> That way it becomes kind of a shorthand for diag(getCustomDiagID(...))  
>> << ... which is a step towards unifying built-in and custom diagnostic  
>> IDs.
>
> So it looks like there's a convention of listing the diag Level _after_  
> the Message clang-tools-extra, and diag(Loc, "message") without  
> specifying a Level. Neither looks like a good idea but if the plan is to  
> keep that convention then I guess your patch is OK.
>
> It's a failing of clang's diag/tablegen system that it wasn't made  
> reusable and ended up getting re-rolled in external projects, each with  
> slightly different interfaces :-/

I think the plan is to eventually use tablegen here; see the comments above
ClangDiagContext::diag.

Anyway, committed now, r202668.

Thanks,
-- 
Peter