[clang-tidy] Add performance-prefer-preincrement check

[njames93]

Summary

Adds a new clang-tidy check that detects postfix increments and decrements that can be swapped for their prefix counterparts. Tried to be as thorough as possible with detecting if the result of the operation is used by a parent. I ran this check over llvm and clang lib and the result would build OK.

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-preincrement.rst

30 ↗

(On Diff #237480)

Please highlight default value with single back-ticks.

[Eugene.Zelenko]

It'll be reasonable to submit patches generated by check into trunk. May be after branching release.

[njames93]

• Small refactor

[njames93]

could this do with an alias into performance?

[Eugene.Zelenko]

In D72553#1815497, @njames93 wrote:

could this do with an alias into performance?

Sure. At least such tricky questions were asked on interviews decade ago :-)

[Mordante]

I only have some naming nits

• incriment -> increment
• decriment -> decrement

Maybe rename the check llvm-prefer-preincrement -> llvm-prefer-pre-increment
and rename the files and classes PreferPreincrementCheck -> PreferPreIncrementCheck

clang-tools-extra/clang-tidy/llvm/PreferPreincrementCheck.cpp

38 ↗

(On Diff #237490)

Does the checker give a lot of false positives?

[njames93]

• Added alias for performance module
• renamed check to prefer-pre-increment

[njames93]

• Renamed incriment in test cases to increment

[njames93]

clang-tools-extra/clang-tidy/llvm/PreferPreincrementCheck.cpp

38 ↗

(On Diff #237490)

First time I tried it there were lots of false positives. But after adding all those Stmt checks it appears to do a rather good job

[njames93]

• 99% sure I have all incriments renamed now

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst

9

Please fix check name.

[Mordante]

In D72553#1815558, @njames93 wrote:

• 99% sure I have all incriments renamed now

I checked and can't find them anymore, thanks.

[njames93]

• Fix perf alias documentation

[njames93]

• Rebased onto trunk fixing merge issues

[merge_guards_bot]

Unit tests: pass. 61775 tests passed, 0 failed and 780 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

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

[njames93]

• fix spelling mistake

[lebedev.ri]

Happy to see this check!

clang-tools-extra/docs/ReleaseNotes.rst

188–191

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.

[merge_guards_bot]

Unit tests: pass. 61861 tests passed, 0 failed and 781 were skipped.

clang-tidy: unknown.

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

[njames93]

clang-tools-extra/docs/ReleaseNotes.rst

188–191

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 1 little note about it but I dont think that justifies putting the check in there.

[lebedev.ri]

clang-tools-extra/docs/ReleaseNotes.rst

188–191

I would agree with performance module.
Putting it in cppcoreguidelines would be the same - why there, it's not an Cppcoreguidelines invention?

[njames93]

• moved check from llvm module to performance

[lebedev.ri]

In D72553#1820692, @njames93 wrote:

• moved check from llvm module to performance

Thank you!

[merge_guards_bot]

Unit tests: pass. 61861 tests passed, 0 failed and 781 were skipped.

clang-tidy: unknown.

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

[njames93]

• Streamlined replacement application

[merge_guards_bot]

Unit tests: pass. 61943 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

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

[merge_guards_bot]

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

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

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.

[njames93]

• Rebase trunk and fix a few nits with test file

[merge_guards_bot]

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

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

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.

[njames93]

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

[JonasToth]

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

clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp

63 ↗

(On Diff #239988)

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.

86 ↗

(On Diff #239988)

i think pre and post instead of capitalized.

clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp

11 ↗

(On Diff #239988)

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.

44 ↗

(On Diff #239988)

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++; }

[njames93]

• Address some nits, more test cases need adding though

clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp

63 ↗

(On Diff #239988)

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

clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp

44 ↗

(On Diff #239988)

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

[merge_guards_bot]

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

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

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.

[njames93]

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

clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp

44 ↗

(On Diff #239988)

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?

[merge_guards_bot]

Unit tests: pass. 62198 tests passed, 0 failed and 815 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

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.

[njames93]

• Added option to enable transforming template dependent types

[njames93]

• Fix order in release notes

[njames93]

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

[JonasToth]

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

[njames93]

Rebased and fix issues relating to rebase

[aaron.ballman]

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

clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

56

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.

clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp

21 ↗

(On Diff #299443)

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

37–38 ↗

(On Diff #299443)

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

42 ↗

(On Diff #299443)

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?

88 ↗

(On Diff #299443)

Typo: occurances -> occurrences

90 ↗

(On Diff #299443)

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?

clang-tools-extra/docs/ReleaseNotes.rst

188–191

I'm rather opposed to putting it in the performance module without evidence that this actually impacts performance. I find that this module tends to be really chatty already, and this check is likely to trigger *a lot* on real world code bases, so I want to make sure it's justified as a performance issue (looking around online suggests that this is not the case any longer in most situations and this has become more of a readability issue than a performance one).

clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst

8

isnt used which -> isn't used and

clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyLLVMModule
   HeaderGuardCheck.cpp
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
+  PreferPreIncrementCheck.cpp
   PreferRegisterOverUnsignedCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

 //===--- LLVMTidyModule.cpp - clang-tidy ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/NamespaceCommentCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
+#include "PreferPreIncrementCheck.h"
 #include "PreferRegisterOverUnsignedCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace llvm_check {
 
 class LLVMModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
     CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
     CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "llvm-namespace-comment");
     CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>(
         "llvm-prefer-isa-or-dyn-cast-in-conditionals");
+    CheckFactories.registerCheck<PreferPreIncrementCheck>(
+        "llvm-prefer-pre-increment");
     CheckFactories.registerCheck<PreferRegisterOverUnsignedCheck>(
         "llvm-prefer-register-over-unsigned");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
   }
 };
 
 // Register the LLVMTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<LLVMModule> X("llvm-module",

clang-tools-extra/clang-tidy/llvm/PreferPreIncrementCheck.h

+//===--- PreferPreIncrementCheck.h - clang-tidy -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERPREINCREMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERPREINCREMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+///  Flags usages of postfix increment and decrement which could be swapped for
+///  their prefix alternatives.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm-prefer-pre-increment.html
+class PreferPreIncrementCheck : public ClangTidyCheck {
+public:
+  PreferPreIncrementCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool TransformCxxOpCalls;
+};
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERPREINCREMENTCHECK_H

clang-tools-extra/clang-tidy/llvm/PreferPreIncrementCheck.cpp

+//===--- PreferPreIncrementCheck.cpp - clang-tidy -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PreferPreIncrementCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+namespace {
+AST_MATCHER_P(UnaryOperator, isOperator, UnaryOperatorKind, Opcode) {
+  return Node.getOpcode() == Opcode;
+}
+} // namespace
+
+PreferPreIncrementCheck::PreferPreIncrementCheck(StringRef Name,
+                                                 ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      TransformCxxOpCalls(Options.get("TransformCxxOpCalls", 1) != 0) {}
+
+void PreferPreIncrementCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "TransformCxxOpCalls", TransformCxxOpCalls);
+}
+
+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.
+  auto BadParents =
+      unless(anyOf(hasParent(decl()), hasParent(expr()),
+                   hasParent(returnStmt()), hasParent(cxxThrowExpr())));
+  auto BoundExpr = expr().bind("ignore");
+  auto BoundChecks =
+      anyOf(hasParent(ifStmt(hasCondition(BoundExpr))),
+            hasParent(forStmt(hasCondition(BoundExpr))),
+            hasParent(switchStmt(hasCondition(BoundExpr))),
+            hasParent(whileStmt(hasCondition(BoundExpr))),
+            hasParent(doStmt(hasCondition(BoundExpr))),
+            hasParent(conditionalOperator(hasCondition(BoundExpr))),
+            hasParent(binaryConditionalOperator(hasCondition(BoundExpr))));
+
+  auto UnusedInParent =
+      allOf(BadParents, unless(allOf(BoundChecks, equalsBoundNode("ignore"))));
+
+  Finder->addMatcher(
+      unaryOperator(isOperator(UnaryOperatorKind::UO_PostInc), UnusedInParent)
+          .bind("op++"),
+      this);
+  Finder->addMatcher(
+      unaryOperator(isOperator(UnaryOperatorKind::UO_PostDec), UnusedInParent)
+          .bind("op--"),
+      this);
+
+  if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls)
+    return;
+
+  auto FindCxxOpCalls = [&](llvm::StringRef PostfixName) {
+    Finder->addMatcher(
+        cxxOperatorCallExpr(
+            UnusedInParent,
+            callee(cxxMethodDecl(
+                hasName(PostfixName), parameterCountIs(1),
+                hasParent(cxxRecordDecl(hasMethod(cxxMethodDecl(
+                    hasName(PostfixName), parameterCountIs(0))))))))
+            .bind(PostfixName),
+        this);
+  };
+  FindCxxOpCalls("operator++");
+  FindCxxOpCalls("operator--");
+}
+
+void PreferPreIncrementCheck::check(const MatchFinder::MatchResult &Result) {
+  auto Apply = [&](SourceRange Range, SourceLocation OpLoc,
+                   const Expr &Expression, bool IsIncrement) {
+    // Warn for all occurances, but don't fix macro usage.
+    DiagnosticBuilder Diag =
+        diag(Range.getBegin(), "Use Pre-%0 instead of Post-%0")
+        << (IsIncrement ? "increment" : "decrement");
+    if (OpLoc.isMacroID() || Range.getBegin().isMacroID() ||
+        Range.getEnd().isMacroID())
+      return;
+    llvm::SmallVector<char, 256> Buffer;
+    Diag << FixItHint::CreateReplacement(
+        Range,
+        ((IsIncrement ? "++" : "--") +
+         tooling::fixit::getText(Expression, *Result.Context).rtrim(("-+")))
+            .toStringRef(Buffer));
+  };
+
+  if (const auto *UnaryOp = Result.Nodes.getNodeAs<UnaryOperator>("op++"))
+    return Apply(UnaryOp->getSourceRange(), UnaryOp->getOperatorLoc(),
+                 *UnaryOp->getSubExpr(), true);
+  if (const auto *UnaryOp = Result.Nodes.getNodeAs<UnaryOperator>("op--"))
+    return Apply(UnaryOp->getSourceRange(), UnaryOp->getOperatorLoc(),
+                 *UnaryOp->getSubExpr(), false);
+  if (const auto *OpCall =
+          Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator++"))
+    return Apply(OpCall->getSourceRange(), OpCall->getOperatorLoc(),
+                 *OpCall->getExprStmt(), true);
+  if (const auto *OpCall =
+          Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator--"))
+    return Apply(OpCall->getSourceRange(), OpCall->getOperatorLoc(),
+                 *OpCall->getExprStmt(), false);
+}
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

 //===-- PerformanceTidyModule.cpp - clang-tidy ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../llvm/PreferPreIncrementCheck.h"
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitConversionInLoopCheck.h"
 #include "InefficientAlgorithmCheck.h"
 #include "InefficientStringConcatenationCheck.h"
 #include "InefficientVectorOperationCheck.h"
 #include "MoveConstArgCheck.h"
 #include "MoveConstructorInitCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MoveConstArgCheck>(
         "performance-move-const-arg");
     CheckFactories.registerCheck<MoveConstructorInitCheck>(
         "performance-move-constructor-init");
     CheckFactories.registerCheck<NoAutomaticMoveCheck>(
         "performance-no-automatic-move");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
         "performance-noexcept-move-constructor");
+    CheckFactories.registerCheck<llvm_check::PreferPreIncrementCheck>(
+        "performance-prefer-pre-increment");
     CheckFactories.registerCheck<TriviallyDestructibleCheck>(
         "performance-trivially-destructible");
     CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
         "performance-type-promotion-in-math-fn");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
         "performance-unnecessary-copy-initialization");
     CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
         "performance-unnecessary-value-param");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`google-upgrade-googletest-case
   replaces them with equivalent APIs with ``suite``.
 
 - New :doc:`linuxkernel-must-use-errs
   <clang-tidy/checks/linuxkernel-must-use-errs>` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
   ``linux/err.h``.
 
+- New :doc:`llvm-prefer-pre-increment
+  <clang-tidy/checks/llvm-prefer-pre-increment>` check.
+
+  Flags usages of postfix increment and decrement which could be swapped for
+  their prefix alternatives.
+
 - New :doc:`llvm-prefer-register-over-unsigned
   <clang-tidy/checks/llvm-prefer-register-over-unsigned>` check.
 
   Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
   them to use ``Register``
 
 - New :doc:`objc-missing-hash
   <clang-tidy/checks/objc-missing-hash>` check.

 New aliases
 ^^^^^^^^^^^
 
 - New alias :doc:`cert-pos44-c
   <clang-tidy/checks/cert-pos44-c>` to
   :doc:`bugprone-bad-signal-to-kill-thread
   <clang-tidy/checks/bugprone-bad-signal-to-kill-thread>` was added.
 
+- New alias :doc:`performance-prefer-pre-increment
+  <clang-tidy/checks/performance-prefer-pre-increment>` to
+  :doc:`llvm-prefer-pre-increment
+  <clang-tidy/checks/llvm-prefer-pre-increment>` was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`bugprone-posix-return
   <clang-tidy/checks/bugprone-posix-return>` check.
 
   Now also checks if any calls to ``pthread_*`` functions expect negative
   return values.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `hicpp-multiway-paths-covered <hicpp-multiway-paths-covered.html>`_,
    `hicpp-no-assembler <hicpp-no-assembler.html>`_,
    `hicpp-signed-bitwise <hicpp-signed-bitwise.html>`_,
    `linuxkernel-must-use-errs <linuxkernel-must-use-errs.html>`_,
    `llvm-header-guard <llvm-header-guard.html>`_,
    `llvm-include-order <llvm-include-order.html>`_, "Yes"
    `llvm-namespace-comment <llvm-namespace-comment.html>`_,
    `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes"
+   `llvm-prefer-pre-increment <llvm-prefer-pre-increment.html>`_, "Yes"
    `llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes"
    `llvm-twine-local <llvm-twine-local.html>`_, "Yes"
    `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
    `misc-misplaced-const <misc-misplaced-const.html>`_,
    `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
.. csv-table:: Aliases..
    `hicpp-use-auto <hicpp-use-auto.html>`_, `modernize-use-auto <modernize-use-auto.html>`_, "Yes"
    `hicpp-use-emplace <hicpp-use-emplace.html>`_, `modernize-use-emplace <modernize-use-emplace.html>`_, "Yes"
    `hicpp-use-equals-default <hicpp-use-equals-default.html>`_, `modernize-use-equals-default <modernize-use-equals-default.html>`_, "Yes"
    `hicpp-use-equals-delete <hicpp-use-equals-delete.html>`_, `modernize-use-equals-delete <modernize-use-equals-delete.html>`_, "Yes"
    `hicpp-use-noexcept <hicpp-use-noexcept.html>`_, `modernize-use-noexcept <modernize-use-noexcept.html>`_, "Yes"
    `hicpp-use-nullptr <hicpp-use-nullptr.html>`_, `modernize-use-nullptr <modernize-use-nullptr.html>`_, "Yes"
    `hicpp-use-override <hicpp-use-override.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
    `hicpp-vararg <hicpp-vararg.html>`_, `cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_,
+   `performance-prefer-pre-increment <performance-prefer-pre-increment.html>`_, `llvm-prefer-pre-increment <llvm-prefer-pre-increment.html>`_, "Yes"
 
 .. toctree::
    :glob:
    :hidden:
 
    *

clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst

+.. title:: clang-tidy - llvm-prefer-pre-increment
+
+llvm-prefer-pre-increment
+=========================
+
+Flags usages of the unary postfix increment and decrement operators where the
+result isnt used which could be replaced with the more efficient prefix variety.
+
+.. code-block:: c++
+
+  // Finds these
+  for (int I = 0; I < 10; I++) {}
+  (*Ptr)++;
+  // Replaces with
+  for (int I = 0; I < 10; ++I) {}
+  ++(*Ptr);
+
+  // Doesn't replace these
+  *Ptr++ = 0;
+  call(I++);
+  Index = I++;
+
+Options
+-------
+
+.. option:: TransformCxxOpCalls
+
+   Enables checking postfix operations on classes and structures that overload
+   both the prefix and postfix operator ``++`` or ``--``.
+   Default value is `1`.
+
+This check helps to enforce this `LLVM Coding Standards recommendation
+<https://llvm.org/docs/CodingStandards.html#prefer-preincrement>`_.

clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst

+.. title:: clang-tidy - performance-prefer-pre-increment
+.. meta::
+   :http-equiv=refresh: 5;URL=llvm-prefer-pre-increment.html
+
+performance-prefer-pre-increment
+================================
+
+The cert-oop54-cpp check is an alias, please see
+`llvm-prefer-pre-increment <llvm-prefer-pre-increment.html>`_
+for more information.

clang-tools-extra/test/clang-tidy/checkers/llvm-prefer-pre-increment-disable-cpp-opcalls.cpp

+// RUN: %check_clang_tidy %s llvm-prefer-pre-increment %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: llvm-prefer-pre-increment.TransformCxxOpCalls, value: 0}]}'
+
+template <typename T>
+class Iterator {
+  T *Current;
+
+public:
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T &operator*() const { return *Current; }
+  Iterator &operator++() { return ++Current, *this; }
+  Iterator operator++(int) {
+    Iterator Copy = *this;
+    ++Current;
+    return Copy;
+  }
+  Iterator &operator--() { return --Current, *this; }
+  Iterator operator--(int) {
+    Iterator Copy = *this;
+    --Current;
+    return Copy;
+  }
+};
+
+template <typename T>
+class PostfixIterator {
+  T *Current;
+
+public:
+  PostfixIterator(T *Pointer) : Current(Pointer) {}
+  T &operator*() 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<int> It(&Array[0]);
+  It++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++It;
+  It--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-decrement instead of Post-decrement
+  // CHECK-FIXES-NOT: {{^}}  --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<int> PfIt(&Array[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]]:4: warning: Use Pre-increment instead of Post-increment
+  *PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:4: warning: Use Pre-decrement instead of Post-decrement
+}

clang-tools-extra/test/clang-tidy/checkers/llvm-prefer-pre-increment.c

+// RUN: %check_clang_tidy %s llvm-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)) {
+    // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+}
+
+void bar(int *Begin, int *End) {
+  for (int *I = Begin; I != End;) {
+    *I++ = 0;
+    // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  }
+  for (int *I = Begin; I != End; I++) {
+    // CHECK-MESSAGES: [[@LINE-1]]:34: warning: Use Pre-increment instead of Post-increment
+    // CHECK-FIXES: {{^}}  for (int *I = Begin; I != End; ++I) {
+    *I = 0;
+  }
+  for (int *I = Begin; I != End; ++I) {
+    // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+    *I = 0;
+  }
+  for (int *I = Begin; I != End; I++){
+    // CHECK-MESSAGES: [[@LINE-1]]:34: warning: Use Pre-increment instead of Post-increment
+    // CHECK-FIXES: {{^}}  for (int *I = Begin; I != End; ++I) {
+    (*I)++; // c1
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: Use Pre-increment instead of Post-increment
+    // CHECK-FIXES: {{^}}    ++(*I); // c1
+    (*I)++; // c2
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: Use Pre-increment instead of Post-increment
+    // CHECK-FIXES: {{^}}    ++(*I); // c2
+    (*I)++; // c3
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: Use Pre-increment instead of Post-increment
+    // CHECK-FIXES: {{^}}    ++(*I); // c3
+  }
+}
+
+int handle(int);
+
+void baz(){
+  int I = 0;
+  handle(I++);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  int J = I++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  if (J++) {
+  }
+  // CHECK-MESSAGES-NOT: [[@LINE-2]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  switch (J++) {}
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  while (J++) {
+  }
+  // CHECK-MESSAGES-NOT: [[@LINE-2]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  do {
+  } while (J++);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  for (; J++;) {
+  }
+  // CHECK-MESSAGES-NOT: [[@LINE-2]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  J++ ? J-- : J++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  J++ ?: J--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use Pre-increment instead of Post-increment
+  J++;
+  // CHECK-MESSAGES: [[@LINE-1]]:{{3}}: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++J;
+}
+
+struct Foo {
+  int Bar;
+};
+
+struct Baz {
+  struct Foo Bar;
+};
+
+int foobar(struct Baz Foobar, struct Foo *Bar) {
+  Foobar.Bar.Bar++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++Foobar.Bar.Bar;
+  Bar->Bar++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES: {{^}}  ++Bar->Bar;
+  Foobar.
+  Bar.
+  Bar++;
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: Use Pre-increment instead of Post-increment
+  // CHECK-FIXES:      {{^}}  ++Foobar.
+  // CHECK-FIXES-NEXT: {{^}}  Bar.
+  // CHECK-FIXES-NEXT: {{^}}  Bar;
+  return Bar->Bar++;
+}

clang-tools-extra/test/clang-tidy/checkers/llvm-prefer-pre-increment.cpp

+// RUN: %check_clang_tidy %s llvm-prefer-pre-increment %t
+
+template <typename T>
+class Iterator {
+  T *Current;
+
+public:
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T &operator*() const { return *Current; }
+  Iterator &operator++() { return ++Current, *this; }
+  Iterator operator++(int) {
+    Iterator Copy = *this;
+    ++Current;
+    return Copy;
+  }
+  Iterator &operator--() { return --Current, *this; }
+  Iterator operator--(int) {
+    Iterator Copy = *this;
+    --Current;
+    return Copy;
+  }
+};
+
+template <typename T>
+class PostfixIterator {
+  T *Current;
+
+public:
+  PostfixIterator(T *Pointer) : Current(Pointer) {}
+  T &operator*() 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<int> It(&Array[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<int> PfIt(&Array[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
+}