[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check
Nathan James via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 25 17:02:33 PST 2020
njames93 added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:63
+
+ if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls)
+ return;
----------------
JonasToth wrote:
> Why would you deactivate the check for c-code?
> I think the `for(int i = 0; i < 10; ++i)` could be a requirement in c-coding styles, too.
I only deactivate operator calls for c-code as they aren't a thing in c. The unary operation expr still runs in c-code
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:44
+};
+
+void foo() {
----------------
JonasToth wrote:
> Test-cases that I would like to see:
>
> - only the post-fix operator is overloaded for the class --> best-case this is detected and a fix is not provided
> - iterator-inheritance: the base-class provides the operator-overloads --> does matching work? There might be an implicit cast for example
> - the iterator-type is type-dependent --> maybe fixing should not be done or even the warning should not be emitted, because there might be only a post-fix available in some instantiations (see point 1). I do mean something like this `template <typename T> void f() { T::iterator it; it++; }`
There are test cases for only post fix operator overloading. basically it doesn't warn or provided a fix it as that isn't valid. I feel like there could be a seperate check that detects classes that overload operator++(int) but not operator++() but thats not what this check is for.
I'll take a look at the other cases tomorrow
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72553/new/
https://reviews.llvm.org/D72553
More information about the cfe-commits
mailing list