On 03/03/2014 12:53, Alexander Kornienko wrote:
On Sun, Mar 2, 2014 at 5:59 PM, Alp Toker <[email protected]
<mailto:[email protected]>> 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.
Right on.
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.
Agree, but with one significant distinction: It's those "analogues" that
created this problem in the first place where we have essentially two
parallel diagnostic systems in clang plus another one currently being
developed in LLVM core.
We really need to peel things back at this point so structures like
StaticDiag*Rec are shared by built-in and custom diagnostic code paths
instead of duplicated. They're generic descriptions after all so it
might be as straightforward as moving them out of the cpp to a .h file
and taking it from there. Ditto getting diag td files to include
"Diagnostic.td" instead of the reverse that exists now -- otherwise
maintaining out-of-tree diagnostic tds is a non-started.
As for registering blocks of diagnostic IDs, that's been kind of
unpleasant and doesn't scale well to plugins and external projects. My
early thought here is to represent diagnostic IDs as a 32-bit hash of
the Rec contents that'll be computed by TableGen at compile time, as
well as optionally at runtime for diagnostics that need to be defined
dynamically. Should solve various quirks that are getting swept under
the carpet today.
What do you think about this?
Keen to go ahead with it so long as we're treating the work as
much-needed cleanup, with clang-tools-extra getting the functionality
when it's ready. It is a non-trivial but the reduction in cruft and will
surely pay off.
Alp.
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
