[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

120–123

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

120–123

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

120–123

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

64

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.

87

i think pre and post instead of capitalized.

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

12

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.

45

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

64

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

45

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

45

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

22

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

38–39

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

43

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?

89

Typo: occurances -> occurrences

91

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

120–123

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/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 "../performance/PreferPreIncrementCheck.h"
 #include "../readability/NamespaceCommentCheck.h"
 #include "../readability/QualifiedAutoCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.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<performance::PreferPreIncrementCheck>(
+        "llvm-prefer-pre-increment");
     CheckFactories.registerCheck<PreferRegisterOverUnsignedCheck>(
         "llvm-prefer-register-over-unsigned");
     CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
         "llvm-qualified-auto");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
   }
 
   ClangTidyOptions getModuleOptions() override {

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

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyPerformanceModule
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitConversionInLoopCheck.cpp
   InefficientAlgorithmCheck.cpp
   InefficientStringConcatenationCheck.cpp
   InefficientVectorOperationCheck.cpp
   MoveConstArgCheck.cpp
   MoveConstructorInitCheck.cpp
   NoAutomaticMoveCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   PerformanceTidyModule.cpp
+  PreferPreIncrementCheck.cpp
   TriviallyDestructibleCheck.cpp
   TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangAnalysis
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   )

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

 #include "ImplicitConversionInLoopCheck.h"
 #include "InefficientAlgorithmCheck.h"
 #include "InefficientStringConcatenationCheck.h"
 #include "InefficientVectorOperationCheck.h"
 #include "MoveConstArgCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "NoAutomaticMoveCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
+#include "PreferPreIncrementCheck.h"
 #include "TriviallyDestructibleCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
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<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/clang-tidy/performance/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_PERFORMANCE_PREFERPREINCREMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_PREFERPREINCREMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+///  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/performance-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:
+  void processMatch(SourceRange Range, SourceLocation OperatorLoc,
+                    bool IsIncrement, bool WarnOnly);
+
+  const bool TransformCxxOpCalls;
+  const bool TransformDependentType;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_PREFERPREINCREMENTCHECK_H

clang-tools-extra/clang-tidy/performance/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/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+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", true)),
+      TransformDependentType(Options.get("TransformDependentType", false)) {}
+
+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,
+                                   unless(isInTemplateInstantiation()))
+                         .bind("op++"),
+                     this);
+  Finder->addMatcher(unaryOperator(isOperator(UnaryOperatorKind::UO_PostDec),
+                                   UnusedInParent,
+                                   unless(isInTemplateInstantiation()))
+                         .bind("op--"),
+                     this);
+
+  if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls)
+    return;
+
+  auto FindCxxOpCalls = [&](llvm::StringRef PostfixName) {
+    Finder->addMatcher(
+        cxxOperatorCallExpr(
+            UnusedInParent, unless(isInTemplateInstantiation()),
+            callee(cxxMethodDecl(
+                hasName(PostfixName), parameterCountIs(1),
+                hasParent(cxxRecordDecl(hasMethod(cxxMethodDecl(
+                    hasName(PostfixName), parameterCountIs(0))))))))
+            .bind(PostfixName),
+        this);
+  };
+  FindCxxOpCalls("operator++");
+  FindCxxOpCalls("operator--");
+}
+
+void PreferPreIncrementCheck::processMatch(SourceRange Range,
+                                           SourceLocation OperatorLoc,
+                                           bool IsIncrement, bool WarnOnly) {
+  // 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 (WarnOnly)
+    return;
+  if (OperatorLoc.isMacroID() || Range.getBegin().isMacroID() ||
+      Range.getEnd().isMacroID())
+    return;
+  Diag << tooling::fixit::createRemoval(OperatorLoc);
+  Diag << FixItHint::CreateInsertion(Range.getBegin(),
+                                     IsIncrement ? "++" : "--");
+}
+
+void PreferPreIncrementCheck::check(const MatchFinder::MatchResult &Result) {
+  auto ShouldWarnOnly = [&](const Expr *E) -> bool {
+    return !TransformDependentType && E->getType()->isDependentType();
+  };
+  if (const auto *UnaryOp = Result.Nodes.getNodeAs<UnaryOperator>("op++"))
+    return processMatch(UnaryOp->getSourceRange(), UnaryOp->getOperatorLoc(),
+                        true, ShouldWarnOnly(UnaryOp));
+  if (const auto *UnaryOp = Result.Nodes.getNodeAs<UnaryOperator>("op--"))
+    return processMatch(UnaryOp->getSourceRange(), UnaryOp->getOperatorLoc(),
+                        false, ShouldWarnOnly(UnaryOp));
+  if (const auto *OpCall =
+          Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator++"))
+    return processMatch(OpCall->getSourceRange(), OpCall->getOperatorLoc(),
+                        true, ShouldWarnOnly(OpCall));
+  if (const auto *OpCall =
+          Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator--"))
+    return processMatch(OpCall->getSourceRange(), OpCall->getOperatorLoc(),
+                        false, ShouldWarnOnly(OpCall));
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-reserved-identifier
 
   Checks for usages of identifiers reserved for use by the implementation.
 
 - New :doc:`cert-oop57-cpp
   <clang-tidy/checks/cert-oop57-cpp>` check.
 
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
   
+- New :doc:`performance-prefer-pre-increment
+  <clang-tidy/checks/performance-prefer-pre-increment>` check.
+
+  Flags usages of postfix increment and decrement which could be swapped for
+  their prefix alternatives.
+
 - New :doc:`objc-dealloc-in-category
   <clang-tidy/checks/objc-dealloc-in-category>` check.
 
   Finds implementations of -dealloc in Objective-C categories.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-dcl37-c
   <clang-tidy/checks/cert-dcl37-c>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.
 
 - New alias :doc:`cert-dcl51-cpp
   <clang-tidy/checks/cert-dcl51-cpp>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.
   
+- New alias :doc:`llvm-prefer-pre-increment
+  <clang-tidy/checks/llvm-prefer-pre-increment>` to
+  :doc:`performance-prefer-pre-increment
+  <clang-tidy/checks/performance-prefer-pre-increment>` was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`readability-qualified-auto
   <clang-tidy/checks/readability-qualified-auto>` check now supports a 
   `AddConstToQualified` to enable adding ``const`` qualifiers to variables
   typed with ``auto *`` and ``auto &``.
 
 - Improved :doc:`readability-redundant-string-init
   <clang-tidy/checks/readability-redundant-string-init>` check now supports a
   `StringNames` option enabling its application to custom string classes. The
   check now detects in class initializers and constructor initializers which

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

.. csv-table::
    `performance-implicit-conversion-in-loop <performance-implicit-conversion-in-loop.html>`_,
    `performance-inefficient-algorithm <performance-inefficient-algorithm.html>`_, "Yes"
    `performance-inefficient-string-concatenation <performance-inefficient-string-concatenation.html>`_,
    `performance-inefficient-vector-operation <performance-inefficient-vector-operation.html>`_, "Yes"
    `performance-move-const-arg <performance-move-const-arg.html>`_, "Yes"
    `performance-move-constructor-init <performance-move-constructor-init.html>`_, "Yes"
    `performance-no-automatic-move <performance-no-automatic-move.html>`_,
    `performance-noexcept-move-constructor <performance-noexcept-move-constructor.html>`_, "Yes"
+   `performance-prefer-pre-increment <performance-prefer-pre-increment.html>`_, "Yes"
    `performance-trivially-destructible <performance-trivially-destructible.html>`_, "Yes"
    `performance-type-promotion-in-math-fn <performance-type-promotion-in-math-fn.html>`_, "Yes"
    `performance-unnecessary-copy-initialization <performance-unnecessary-copy-initialization.html>`_,
    `performance-unnecessary-value-param <performance-unnecessary-value-param.html>`_, "Yes"
    `portability-simd-intrinsics <portability-simd-intrinsics.html>`_,
    `readability-avoid-const-params-in-decls <readability-avoid-const-params-in-decls.html>`_,
    `readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
    `readability-const-return-type <readability-const-return-type.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>`_,
+   `llvm-prefer-pre-increment <llvm-prefer-pre-increment.html>`_, `performance-prefer-pre-increment <performance-prefer-pre-increment.html>`_, "Yes"
    `llvm-qualified-auto <llvm-qualified-auto.html>`_, `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes"

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

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

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

+.. title:: clang-tidy - performance-prefer-pre-increment
+
+performance-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
+
+   When set to `1`, this enables checking postfix operations on classes and
+   structures that overload both the prefix and postfix operator ``++`` or 
+   ``--``.
+   Default value is `1`.
+
+.. option:: TransformDependentType
+   
+   When set to `1`, this will change all postfix operators that are template
+   dependent to their prefix variation. This can cause errors if instantiations
+   don't overload the prefix ``++`` or ``--`` operator.
+   Default value is `0`.
+
+.. code-block:: c++
+
+   template <typename T>
+   void Inc(T& val) {
+     val++;
+   }
+
+When `TransformDependentType` is set to `1` becomes:
+
+.. code-block:: c++
+
+  template <typename T>
+  void Inc(T& val) {
+    ++val;
+  }
+
+This check helps to enforce this `LLVM Coding Standards recommendation
+<https://llvm.org/docs/CodingStandards.html#prefer-preincrement>`_.

clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h

+#ifndef PERFORMANCE_PREFER_PRE_INCREMENT_ITERATOR_H
+#define PERFORMANCE_PREFER_PRE_INCREMENT_ITERATOR_H
+
+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;
+  }
+};
+
+#endif // PERFORMANCE_PREFER_PRE_INCREMENT_ITERATOR_H

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

+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: performance-prefer-pre-increment.TransformCxxOpCalls, value: 0}]}' \
+// RUN: -- -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+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/performance-prefer-pre-increment.c

+// 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)) {
+    // 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
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wgnu-conditional-omitted-operand"
+  J++ ?: J--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+#pragma clang diagnostic pop
+  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;
+  // clang-format off
+  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;
+  // clang-format on
+  return Bar->Bar++;
+}

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

+// 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<int> {
+  using Iterator<int>::Iterator;
+};
+
+void foo() {
+  int Array[32];
+  Iterator<int> It(&Array[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<int> PfIt(&Array[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(&Array[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 <typename T>
+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