[clang-tidy] Add check to perfer pre-increment over post-increment

[njames93]

See D72553.

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

add_clang_library(clangTidyPerformanceModule
   NoAutomaticMoveCheck.cpp
   NoIntToPtrCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   PerformanceTidyModule.cpp
   TriviallyDestructibleCheck.cpp
   TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp
+  UsePreincrementCheck.cpp
 
   LINK_LIBS
   clangTidy
   clangTidyUtils
 
   DEPENDS
   omp_gen
   )

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

 #include "MoveConstructorInitCheck.h"
 #include "NoAutomaticMoveCheck.h"
 #include "NoIntToPtrCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
 #include "TriviallyDestructibleCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
+#include "UsePreincrementCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 class PerformanceModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     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");
+    CheckFactories.registerCheck<UsePreincrementCheck>(
+        "performance-use-preincrement");
   }
 };
 
 // Register the PerformanceModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<PerformanceModule>
     X("performance-module", "Adds performance checks.");
 
 } // namespace performance
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the PerformanceModule.
 volatile int PerformanceModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clang-tidy/performance/UsePreincrementCheck.h

+//===--- UsePreincrementCheck.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_USEPREINCREMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_USEPREINCREMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// Checks that preincrement is used when there is no need for postincrement
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-use-preincrement.html
+class UsePreincrementCheck : public ClangTidyCheck {
+public:
+  UsePreincrementCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_USEPREINCREMENTCHECK_H

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

+//===--- UsePreincrementCheck.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 "UsePreincrementCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+void UsePreincrementCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      unaryOperator(hasOperatorName("++"), hasParent(compoundStmt()))
+          .bind("unaryOperator"),
+      this);
+}
+
+void UsePreincrementCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Operator = Result.Nodes.getNodeAs<UnaryOperator>("unaryOperator");
+  if (Operator->isPrefix())
+    return;
+
+  auto PostIncRange =
+      CharSourceRange::getTokenRange(Operator->getSourceRange());
+  auto ValRange =
+      CharSourceRange::getTokenRange(Operator->getSubExpr()->getSourceRange());
+
+  auto Diag =
+      diag(Operator->getOperatorLoc(),
+           "Preincrement could be used instead of postincrement.")
+      << FixItHint::CreateInsertion(ValRange.getBegin().getLocWithOffset(-2),
+                                    "++")
+      << FixItHint::CreateRemoval(PostIncRange.getEnd().getLocWithOffset(1));
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 
 New checks
 ^^^^^^^^^^
 
 - New :doc:`bugprone-shared-ptr-array-mismatch <clang-tidy/checks/bugprone-shared-ptr-array-mismatch>` check.
 
   Finds initializations of C++ shared pointers to non-array type that are initialized with an array.
 
+- New :doc:`performance-use-preincrement
+  <clang-tidy/checks/performance-use-preincrement>` check.
+
+  FIXME: add release notes.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Fixed a false positive in :doc:`readability-non-const-parameter
   <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue

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

.. csv-table::
    `bugprone-move-forwarding-reference <bugprone-move-forwarding-reference.html>`_, "Yes"
    `bugprone-multiple-statement-macro <bugprone-multiple-statement-macro.html>`_,
    `bugprone-no-escape <bugprone-no-escape.html>`_,
    `bugprone-not-null-terminated-result <bugprone-not-null-terminated-result.html>`_, "Yes"
    `bugprone-parent-virtual-call <bugprone-parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone-posix-return.html>`_, "Yes"
    `bugprone-redundant-branch-condition <bugprone-redundant-branch-condition.html>`_, "Yes"
    `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
-   `bugprone-shared-ptr-array-mismatch <bugprone-shared-ptr-array-mismatch.html>`_, "Yes"
+   `bugprone-shared-ptr-array-mismatch <bugprone-shared-ptr-array-mismatch.html>`_,
    `bugprone-signal-handler <bugprone-signal-handler.html>`_,
    `bugprone-signed-char-misuse <bugprone-signed-char-misuse.html>`_,
    `bugprone-sizeof-container <bugprone-sizeof-container.html>`_,
    `bugprone-sizeof-expression <bugprone-sizeof-expression.html>`_,
    `bugprone-spuriously-wake-up-functions <bugprone-spuriously-wake-up-functions.html>`_,
    `bugprone-string-constructor <bugprone-string-constructor.html>`_, "Yes"
    `bugprone-string-integer-assignment <bugprone-string-integer-assignment.html>`_, "Yes"
    `bugprone-string-literal-with-embedded-nul <bugprone-string-literal-with-embedded-nul.html>`_,
.. csv-table::
    `performance-move-constructor-init <performance-move-constructor-init.html>`_,
    `performance-no-automatic-move <performance-no-automatic-move.html>`_,
    `performance-no-int-to-ptr <performance-no-int-to-ptr.html>`_,
    `performance-noexcept-move-constructor <performance-noexcept-move-constructor.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>`_, "Yes"
    `performance-unnecessary-value-param <performance-unnecessary-value-param.html>`_, "Yes"
+   `performance-use-preincrement <performance-use-preincrement.html>`_, "Yes"
    `portability-restrict-system-includes <portability-restrict-system-includes.html>`_, "Yes"
    `portability-simd-intrinsics <portability-simd-intrinsics.html>`_,
    `readability-avoid-const-params-in-decls <readability-avoid-const-params-in-decls.html>`_, "Yes"
    `readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
    `readability-const-return-type <readability-const-return-type.html>`_, "Yes"
    `readability-container-contains <readability-container-contains.html>`_, "Yes"
    `readability-container-data-pointer <readability-container-data-pointer.html>`_, "Yes"
    `readability-container-size-empty <readability-container-size-empty.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/performance-use-preincrement.rst

+.. title:: clang-tidy - performance-use-preincrement
+
+performance-use-preincrement
+============================
+
+Checks that preincrement is used when there is no need for postincrement.
+This can lead to improved performance with iterators that are not trivially copyable.

clang-tools-extra/test/clang-tidy/checkers/performance-use-preincrement.cpp

+// RUN: %check_clang_tidy %s performance-use-preincrement %t
+
+void positivePostincrement() {
+  int I = 0;
+  I++;
+  // CHECK-MESSAGES: [[@LINE-1]]:4: warning: Preincrement could be used instead of postincrement. [performance-use-preincrement]
+}
+
+void negativePreincrement() {
+  int I = 0;
+  ++I;
+}
+
+void negativeUsedPostincrement() {
+  int I = 0;
+  int J = I++;
+  ++J;
+  J = I++;
+}
+
+int negativeReturn(int &I) {
+  return I++;
+}