[PATCH] D19577: [clang-tidy] Enhance misc-suspicious-string-compare by matching string.compare

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 19 16:50:24 PDT 2016


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





More information about the cfe-commits mailing list