[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Aaron Ballman via Phabricator via cfe-commits]

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

In D72553#1815497 , @njames93 wrote:

> could this do with an alias into performance?

If it was 1997, I'd say yes, but I am not aware of any optimizer that does so 
poorly with pre/post increment to suggest this is actually a 
performance-related check still today. Do we have any evidence that at O1 
 or higher this has any impact 
whatsoever on performance? If not, then I'd say the alias shouldn't be there 
(because the check could be noisy on correct code bases).




Comment at: 
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp:56
+CheckFactories.registerCheck(
+"performance-prefer-pre-increment");
 CheckFactories.registerCheck(

I think the name is a bit unfortunate -- the check handles more than just 
incrementing. But there's not really a good neutral word like "crement" that 
covers both actions. "Adjustment" sort of comes close, but would be kind of 
awful for a check name. Alternative ideas are welcome here.



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:21
+namespace {
+AST_MATCHER_P(UnaryOperator, isOperator, UnaryOperatorKind, Opcode) {
+  return Node.getOpcode() == Opcode;

Oh, neat, we have `unaryOperator(hasOperatorName("++"))` but that doesn't quite 
cut it here. :-D



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:37-38
+void PreferPreIncrementCheck::registerMatchers(MatchFinder *Finder) {
+  // Ignore all unary ops with a parent decl or expr, those use the value
+  // returned. Reordering those would change the behaviour of the expression.
+  // FIXME: Add any more parents which could use the result of the operation.

Overloaded operators are a similar concern about changing the behavior of the 
expression. I think it's reasonably safe to assume that pre and post 
increment/decrement will typically do the same operations, but perhaps we 
should skip any overloaded operators that aren't idiomatic or aren't paired? 
e.g.,
```
struct S {
  S operator++(int);
}; // Only post-fix available, don't diagnose

struct T {
  int& operator++();
  int operator++(int);
}; // Not idiomatic return types, don't diagnose

struct U {
  S& operator++();
  T operator++(int);
}; // Oh god, this just hurts, don't diagnose
```



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:42
+  unless(anyOf(hasParent(decl()), hasParent(expr()),
+   hasParent(returnStmt()), hasParent(cxxThrowExpr(;
+  auto BoundExpr = expr().bind("ignore");

I think `hasParent(cxxThrowExpr())` is already covered by `hasParent(expr())`, 
right?

Also, this may not be quite right -- what about a case like this: `sizeof(i++)` 
where there's no performance reason to prefer one or the other?



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:88
+   bool IsIncrement, bool WarnOnly) {
+  // Warn for all occurances, but don't fix macro usage.
+  DiagnosticBuilder Diag =

Typo: occurances -> occurrences



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:90
+  DiagnosticBuilder Diag =
+  diag(Range.getBegin(), "Use pre-%0 instead of post-%0")
+  << (IsIncrement ? "increment" : "decrement");

clang-tidy diagnostics are not capitalized like that -- also, this diagnostic 
doesn't really say what's wrong with the user's code. How about: `pre-%0 may 
have better performance characteristics than post-%0` or something along those 
lines?



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:199-202
+- New alias :doc:`performance-prefer-pre-increment
+  ` to
+  :doc:`llvm-prefer-pre-increment
+  ` was added.

lebedev.ri wrote:
> njames93 wrote:
> > lebedev.ri wrote:
> > > Are we **really** **really** sure this is the correct relation direction?
> > > This isn't an llvm-specific guideline that may be applicable to other 
> > > code,
> > > but a known generic C++ guideline that llvm coding guide follows.
> > You're probably right, I added this to llvm first, then thought about 
> > alias. Which module should its primary be
> > I'd say performancepersonally. Cppcoreguidelines has [[ 
> > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#enforcement-8
> >  | 1 little note ]] about it but I dont think that justifies putting the 
> > check in there.
> I would agree with `performance` module.
> Putting it in cppcoreguidelines would be the same - why there, it's not an 
> Cppcoreguidelines invention?
I'm rather opposed to putting it 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 299443.
njames93 added a comment.

Rebased and fix issues relating to rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- -- \
+// RUN: -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+class IntIterator : public Iterator {
+  using Iterator::Iterator;
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++It; // fooNormal
+  It--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --It; // fooNormal
+  (*It)++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*It); // fooNormal
+  (*It)--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*It); // fooNormal
+  *It++; // fooNormal
+  *It--; // fooNormal
+
+  PostfixIterator PfIt([0]);
+  PfIt++;// fooPostfix
+  PfIt--;// fooPostfix
+  (*PfIt)++; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt); // fooPostfix
+  (*PfIt)--; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt); // fooPostfix
+  *PfIt++; // fooPostfix
+  *PfIt--; // fooPostfix
+
+  IntIterator IntIt([0]);
+  IntIt++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++IntIt;
+  IntIt--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --IntIt;
+  (*IntIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*IntIt);
+  (*IntIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*IntIt);
+  *IntIt++;
+  *IntIt--;
+}
+
+template 
+void tempDependent() {
+  typename T::iterator it;
+  it++; //depType
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++it; //depType
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-increment instead of post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-decrement instead of post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

Sorry for missing this review, i simply hadn't time :/
All my concerns are addressed and I think this is LGTM after rebasing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Running this over llvm with only clang and clang-tools-extra enabled this 
 was the 
output. ninja check-all ran successfully with no errors. Didn't try transform 
`TransformCxxOpCalls` or `TransformDependentType` as they can be more dangerous


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 244557.
njames93 added a comment.

- Fix order in release notes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- -- \
+// RUN: -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+class IntIterator : public Iterator {
+  using Iterator::Iterator;
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++It; // fooNormal
+  It--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --It; // fooNormal
+  (*It)++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*It); // fooNormal
+  (*It)--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*It); // fooNormal
+  *It++; // fooNormal
+  *It--; // fooNormal
+
+  PostfixIterator PfIt([0]);
+  PfIt++;// fooPostfix
+  PfIt--;// fooPostfix
+  (*PfIt)++; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt); // fooPostfix
+  (*PfIt)--; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt); // fooPostfix
+  *PfIt++; // fooPostfix
+  *PfIt--; // fooPostfix
+
+  IntIterator IntIt([0]);
+  IntIt++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++IntIt;
+  IntIt--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --IntIt;
+  (*IntIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*IntIt);
+  (*IntIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*IntIt);
+  *IntIt++;
+  *IntIt--;
+}
+
+template 
+void tempDependent(){
+  typename T::iterator it;
+  it++; //depType
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++it; //depType
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-increment instead of post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-decrement instead of post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+  for (int I = 10; I < 0; A = I--) {
+// 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 244555.
njames93 added a comment.

- Added option to enable transforming template dependent types


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- -- \
+// RUN: -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+class IntIterator : public Iterator {
+  using Iterator::Iterator;
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++It; // fooNormal
+  It--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --It; // fooNormal
+  (*It)++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*It); // fooNormal
+  (*It)--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*It); // fooNormal
+  *It++; // fooNormal
+  *It--; // fooNormal
+
+  PostfixIterator PfIt([0]);
+  PfIt++;// fooPostfix
+  PfIt--;// fooPostfix
+  (*PfIt)++; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt); // fooPostfix
+  (*PfIt)--; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt); // fooPostfix
+  *PfIt++; // fooPostfix
+  *PfIt--; // fooPostfix
+
+  IntIterator IntIt([0]);
+  IntIt++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++IntIt;
+  IntIt--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --IntIt;
+  (*IntIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*IntIt);
+  (*IntIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*IntIt);
+  *IntIt++;
+  *IntIt--;
+}
+
+template 
+void tempDependent(){
+  typename T::iterator it;
+  it++; //depType
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++it; //depType
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-increment instead of post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-decrement instead of post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62198 tests passed, 0 failed 
and 815 were skipped.

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 240584.
njames93 marked an inline comment as done.
njames93 added a comment.

- Added test case when the inc and dec operators are found in a base class


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- -- \
+// RUN: -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+class IntIterator : public Iterator{
+  using Iterator::Iterator;
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++It; // fooNormal
+  It--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --It; // fooNormal
+  (*It)++; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*It); // fooNormal
+  (*It)--; // fooNormal
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*It); // fooNormal
+  *It++; // fooNormal
+  *It--; // fooNormal
+
+  PostfixIterator PfIt([0]);
+  PfIt++; // fooPostfix
+  PfIt--; // fooPostfix
+  (*PfIt)++; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt); // fooPostfix
+  (*PfIt)--; // fooPostfix
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt); // fooPostfix
+  *PfIt++; // fooPostfix
+  *PfIt--; // fooPostfix
+
+  IntIterator IntIt([0]);
+  IntIt++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++IntIt;
+  IntIt--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --IntIt;
+  (*IntIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*IntIt);
+  (*IntIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*IntIt);
+  *IntIt++;
+  *IntIt--;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-increment instead of post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-decrement instead of post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+  for (int I = 10; I < 0; A = I--) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+
+  for (int I = 0; I < 10; INC(I)) {
+// 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:44
+};
+
+void foo() {

njames93 wrote:
> 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  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
Currently the base class provided operator overloads match normally.

For template dependent code it gets a little hazy. Basically if the type isn't 
known the operator will always appear as a `UnaryOperator`, maybe its safest to 
proceed by disabling fixes if the type isn't known, and maybe add an option to 
override that behaviour, WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62197 tests passed, 1 failed 
and 815 were skipped.

  failed: 
libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_class/lock.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 240402.
njames93 marked 5 inline comments as done.
njames93 added a comment.

- Address some nits, more test cases need adding though


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- -- \
+// RUN: -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++It;
+  It--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --It;
+  (*It)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*It);
+  (*It)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*It);
+
+  *It++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  *It--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+
+  PostfixIterator PfIt([0]);
+  PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++PfIt;
+  PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES-NOT: {{^}}  --PfIt;
+  (*PfIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt);
+  (*PfIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt);
+
+  *PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  *PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-increment instead of post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-decrement instead of post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+  for (int I = 10; I < 0; A = I--) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+
+  for (int I = 0; I < 10; INC(I)) {
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; DEC(I)) {
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: Use pre-decrement instead of post-decrement
+  }
+  for (int I = 0; I < 10; A = INC(I)) {
+   

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

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  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



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Yeah, the libc++ failures are not your fault :)
My 2 cents added.




Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:63
+
+  if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls)
+return;

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. 



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:86
+  DiagnosticBuilder Diag =
+  diag(Range.getBegin(), "Use Pre-%0 instead of Post-%0")
+  << (IsIncrement ? "increment" : "decrement");

i think `pre` and `post` instead of capitalized.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:11
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  Iterator ++() { return ++Current, *this; }

Please extract the common iterator class into a header and include it in the 
tests. this is done in other tests as well, just grep for `include`.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:44
+};
+
+void foo() {

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  void f() { T::iterator it; it++; }`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Gonna throw it out there, those pre merge fails aren't anything to do with this 
patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62129 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 239988.
njames93 added a comment.

- Rebase trunk and fix a few nits with test file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+template 
+class Iterator {
+  T *Current;
+
+public:
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  Iterator ++() { return ++Current, *this; }
+  Iterator operator++(int) {
+Iterator Copy = *this;
+++Current;
+return Copy;
+  }
+  Iterator () { return --Current, *this; }
+  Iterator operator--(int) {
+Iterator Copy = *this;
+--Current;
+return Copy;
+  }
+};
+
+template 
+class PostfixIterator {
+  T *Current;
+
+public:
+  PostfixIterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  PostfixIterator operator++(int) {
+PostfixIterator Copy = *this;
+++Current;
+return Copy;
+  }
+  PostfixIterator operator--(int) {
+PostfixIterator Copy = *this;
+--Current;
+return Copy;
+  }
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++It;
+  It--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --It;
+  (*It)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++(*It);
+  (*It)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --(*It);
+
+  *It++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  *It--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+
+  PostfixIterator PfIt([0]);
+  PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++PfIt;
+  PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES-NOT: {{^}}  --PfIt;
+  (*PfIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt);
+  (*PfIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt);
+
+  *PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  *PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use Pre-increment instead of Post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use Pre-decrement instead of Post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 61939 tests passed, 5 failed 
and 782 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61943 tests passed, 0 failed 
and 783 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 238740.
njames93 added a comment.

- Streamlined replacement application


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+template 
+class Iterator {
+  T *Current;
+
+public:
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  Iterator ++() { return ++Current, *this; }
+  Iterator operator++(int) {
+Iterator Copy = *this;
+++Current;
+return Copy;
+  }
+  Iterator () { return --Current, *this; }
+  Iterator operator--(int) {
+Iterator Copy = *this;
+--Current;
+return Copy;
+  }
+};
+
+template 
+class PostfixIterator {
+  T *Current;
+
+public:
+  PostfixIterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  PostfixIterator operator++(int) {
+PostfixIterator Copy = *this;
+++Current;
+return Copy;
+  }
+  PostfixIterator operator--(int) {
+PostfixIterator Copy = *this;
+--Current;
+return Copy;
+  }
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++It;
+  It--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --It;
+  (*It)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++(*It);
+  (*It)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --(*It);
+
+  *It++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  *It--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+
+  PostfixIterator PfIt([0]);
+  PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++PfIt;
+  PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES-NOT: {{^}}  --PfIt;
+  (*PfIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt);
+  (*PfIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt);
+
+  *PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  *PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use Pre-increment instead of Post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use Pre-decrement instead of Post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// 

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D72553#1820692 , @njames93 wrote:

> - moved check from llvm module to performance


Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61861 tests passed, 0 failed 
and 781 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553



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

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 238104.
njames93 added a comment.

- moved check from llvm module to performance


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72553/new/

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+template 
+class Iterator {
+  T *Current;
+
+public:
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  Iterator ++() { return ++Current, *this; }
+  Iterator operator++(int) {
+Iterator Copy = *this;
+++Current;
+return Copy;
+  }
+  Iterator () { return --Current, *this; }
+  Iterator operator--(int) {
+Iterator Copy = *this;
+--Current;
+return Copy;
+  }
+};
+
+template 
+class PostfixIterator {
+  T *Current;
+
+public:
+  PostfixIterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  PostfixIterator operator++(int) {
+PostfixIterator Copy = *this;
+++Current;
+return Copy;
+  }
+  PostfixIterator operator--(int) {
+PostfixIterator Copy = *this;
+--Current;
+return Copy;
+  }
+};
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++It;
+  It--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --It;
+  (*It)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++(*It);
+  (*It)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --(*It);
+
+  *It++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  *It--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+
+  PostfixIterator PfIt([0]);
+  PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++PfIt;
+  PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES-NOT: {{^}}  --PfIt;
+  (*PfIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt);
+  (*PfIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt);
+
+  *PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  *PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use Pre-increment instead of Post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use Pre-decrement instead of Post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {