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

[Alexander Kornienko]

On Sun, Mar 2, 2014 at 5:59 PM, Alp Toker <alp at nuanti.com> 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 :-/


Yes, there's a lot to unify and clean up in this area. If we can come up
with a proper interface to manage diagnostic ID spaces, so that tablegen'd
diagnostic tables can be registered dynamically, I'd be happy to clean up
clang-tidy and static analyzer to use this system.

In the simplest case, we'd need some analogues for struct StaticDiagInfoRec
and maybe struct StaticDiagCategoryRec and a method to register a block of
them and return the diagnostic ID of the first element, so that the client
code could use it as an offset to the local static IDs.

What do you think about this?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140303/0d51b19a/attachment.html>