[PATCH] Initial clang-tidy architecture

Manuel Klimek klimek at google.com
Tue Jun 4 01:16:59 PDT 2013


On Tue, Jun 4, 2013 at 10:17 AM, Daniel Jasper <djasper at google.com> wrote:

>
>
> ================
> Comment at: clang-tidy/ClangTidyModule.h:29
> @@ +28,3 @@
> +  virtual ~ClangTidyModule() {}
> +  virtual void addChecks(ClangTidyCheckVector *Checks) = 0;
> +};
> ----------------
> Manuel Klimek wrote:
> > Daniel Jasper wrote:
> > > Manuel Klimek wrote:
> > > > I don't think the typedef buys us much here, but I might be alone
> with my opinion...
> > > As you have just asked my to change it above and that change would
> already have required me to change more than a handful of instances, I
> think you have proven yourself wrong ;-).
> > Well, I don't consider changing 10 lines of code a big issue; when I say
> I think it doesn't buy us much, I think that it doesn't aid readability :P
> Wait until we have 100 such modules ;-)...
>
> ================
> Comment at: clang-tidy/ClangTidyModule.h:21
> @@ +20,3 @@
> +
> +typedef std::pair<StringRef, ClangTidyCheck *> NamedCheck;
> +typedef SmallVector<NamedCheck, 16> ClangTidyCheckVector;
> ----------------
> Manuel Klimek wrote:
> > Daniel Jasper wrote:
> > > Manuel Klimek wrote:
> > > > I'd probably use a getName() in ClangTidyCheck instead.
> > > The point is that I want this to be a configuration feature of the
> Module instead of an implementation detail of the check. I think we might
> arrive at a point where the same Check-class is used with slightly
> different options in several modules and thereby with different names.
> > Wouldn't that mean that it's even more strongly coupled to the Check
> class? Anyway, I don't feel too strongly about it.
> I don't understand what you mean by that?
>
> Imagine, e.g. we would have a namespace comment check in both LLVM and
> Google and only the actually inserted comment would be slightly different.
> I would then put the NamespaceCommentCheck class into a common directory,
> make the inserted comment a parameter and let both the LLVMModule and the
> GoogleModule export it, e.g. as llvm-namespace-comments and
> google-namespace-comments.
>
> But I also don't feel strongly and think that we can only make more
> educated decisions once we have a few more checks implemented.
>

+1 to the last sentence :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130604/7b1fc1aa/attachment.html>


More information about the cfe-commits mailing list