alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

In https://reviews.llvm.org/D19577#413888, @etienneb wrote:

> In https://reviews.llvm.org/D19577#413526, @alexfh wrote:
>
> > I'm somewhat reluctant to add LLVM-specific checks to misc-. I would prefer 
> > either to split the LLVM-specific part to a separate check in llvm/ (which 
> > might derive from this check or somehow reuse the logic) or make the check 
> > configurable enough so that these checks can be enabled for LLVM in the 
> > config file.
>
>
> How should we solve adding other code base to checkers.
>  It will be common to have these code base: chromium, llvm, boost, stl.
>
> Should we use the local/global to configure them?


(we might have discussed this offline, but I'm not sure now, so posting it here)

I don't think that checks should know about specifics of different code bases, 
since this doesn't scale well. One of alternative approaches should be used 
instead, depending on the amount and complexity of the project-specific parts:

1. Provide enough configuration options to support known and unknown code bases 
(within certain limits). If needed, register checks with a project-specific 
alias and corresponding option values. Alternatively, specify options in the 
project's .clang-tidy file. Example: google-readability-namespace-comments 
check.
2. Make the check extensible via inheritance. Example: llvm-header-guard check.
3. Pull out some utility functions/matchers/classes/... and make a separate 
check for each code base.

One more thing to consider is that it's usually better to keep the overlap in 
patterns detected by the different variants of the check minimal. E.g. a check 
detecting a certain problematic pattern applied to LLVM containers should not 
complain about the same pattern applied to STL containers to avoid duplicate 
warnings in case both checks are turned on (consider a code base that uses 
LLVM, boost and STL, and needs three variants of some check).


https://reviews.llvm.org/D19577



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to