This patch added the support for method "compare" on string-like classes.
The LLVM stringRef is supported. The checker assume that StringRef is returning -1, 0 or 1.
Which is not the case for other functions returning <0, 0 or >0.
Paths
| Differential D19577
[clang-tidy] Enhance misc-suspicious-string-compare by matching string.compare Needs RevisionPublic Authored by etienneb on Apr 26 2016, 7:53 PM.
Details
Summary This patch added the support for method "compare" on string-like classes. The LLVM stringRef is supported. The checker assume that StringRef is returning -1, 0 or 1.
Diff Detail Event TimelineComment Actions 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.
This revision now requires changes to proceed.Apr 27 2016, 4:21 AM etienneb edited edge metadata. etienneb marked an inline comment as done. Comment Actionsaddress alexfh comments, add a macro FP. Comment Actions
How should we solve adding other code base to checkers. Should we use the local/global to configure them? alexfh edited edge metadata. Comment Actions
(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:
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). This revision now requires changes to proceed.Jul 19 2016, 4:50 PM
Revision Contents
Diff 55240 clang-tidy/misc/SuspiciousStringCompareCheck.cpp
test/clang-tidy/misc-suspicious-string-compare.cpp
|
This is inconsistent with the code above. I'd also use ofClass(hasName("::llvm::StringRef")), hasAnyName("compare", ...).