The general architecture looks good here. A few comments:
1) I agree with Chandler that we want to avoiding adding another diagnostic
mechanism to Clang. There's a lot of extra baggage that each diagnostic
mechanism brings, and I'd much rather we extend/refactor Clang's primary
mechanism to support what clang-tidy needs.
2) I find the grouping of checks under specific projects (Google and LLVM) to
be odd. I think what we want is just a bunch of checks with descriptive names
(explicit constructors, sorted includes, end-of-namespace comments, etc.), and
some very lightweight mechanism that composes a set of checks (based on their
names) into a "coding convention". That will be the point of configurability
later, to compose sets of checks into your own coding style. Yes, there may be
some weird checks that make sense for a particular project, and we can prefix
those if we have to, but overall I expect most checks to be universal.
================
Comment at: clang-tidy/google/GoogleModule.cpp:37
@@ +36,3 @@
+ if (!Ctor->isExplicit() && !Ctor->isImplicit() &&
+ Ctor->getNumParams() == 1) {
+ SourceLocation Loc = Ctor->getLocation();
----------------
This won't handle constructors that have > 1 arguments where the second and
later arguments have defaults. Use Ctor->getNumParams() >= 1 &&
Ctor->getMinRequiredArguments() <= 1
================
Comment at: clang-tidy/google/GoogleModule.cpp:42
@@ +41,3 @@
+ tooling::Replacement(*Result.SourceManager, Loc, 0, "explicit "));
+ Context->reportError(ClangTidyError(
+ *Result.SourceManager, Loc,
----------------
Actually fixing this issue can break code. Is that something for the tooling
infrastructure to deal with? Should it be indicated somehow in the diagnostic?
================
Comment at: clang-tidy/llvm/LLVMModule.cpp:46
@@ +45,3 @@
+ // FIXME: Check comment content.
+ IsProperlyTerminated = true;
+ }
----------------
Early return here.
================
Comment at: clang-tidy/llvm/LLVMModule.cpp:51
@@ +50,3 @@
+ if (!ND->isAnonymousNamespace())
+ Fix = Fix.append(" ").append(ND->getNameAsString());
+ tooling::Replacements Replacements;
----------------
else "Fix = " // anonymous namespace"; ?
http://llvm-reviews.chandlerc.com/D884
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
