<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Mar 2, 2014 at 5:59 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><br>
On 02/03/2014 16:18, Alp Toker wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">
<br>
On 02/03/2014 12:25, Peter Collingbourne wrote:<br>
</div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Index: clang-tidy/ClangTidy.h<br>
==============================<u></u>==============================<u></u>=======<br>
--- clang-tidy/ClangTidy.h<br>
+++ clang-tidy/ClangTidy.h<br>
@@ -76,7 +76,8 @@<br>
    void setContext(ClangTidyContext *Ctx) { Context = Ctx; }<br>
      /// \brief Add a diagnostic with the check's name.<br>
-  DiagnosticBuilder diag(SourceLocation Loc, StringRef Description);<br>
+  DiagnosticBuilder diag(SourceLocation Loc, StringRef Description,<br>
+                         DiagnosticIDs::Level Level = DiagnosticIDs::Warning);<br>
</blockquote>
<br>
Could you order the parameters Loc, Level, FormatString and drop the default argument?<br>
<br>
That'll provide visual consistency with the output as well as internal consistency with clang's own getCustomDiagID(Level L, StringRef FormatString).<br>
<br>
That way it becomes kind of a shorthand for diag(getCustomDiagID(...)) << ... which is a step towards unifying built-in and custom diagnostic IDs.<br>
</div></blockquote>
<br>
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.<br>

<br>
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 :-/</blockquote><div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><div>What do you think about this?</div><div>  </div></div>
</div></div>