https://github.com/saideepaksana updated https://github.com/llvm/llvm-project/pull/167158
>From 5faeda6577fe3926967582306475fddba73324f1 Mon Sep 17 00:00:00 2001 From: Sai Deepak <[email protected]> Date: Fri, 7 Nov 2025 17:26:00 +0000 Subject: [PATCH 1/9] test(readability): fix constant-operand-order test to be robust and place RUN flags correctly --- clang-tools-extra/clang-tidy/readability/CMakeLists.txt | 1 + .../clang-tidy/readability/ReadabilityTidyModule.cpp | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 161a0d96caf41..d8237d3d5dda2 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityModule STATIC AvoidUnconditionalPreprocessorIfCheck.cpp BracesAroundStatementsCheck.cpp ConstReturnTypeCheck.cpp + ConstantOperandOrderCheck.cpp ContainerContainsCheck.cpp ContainerDataPointerCheck.cpp ContainerSizeEmptyCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index afb63571de583..48feeaec4af8f 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -16,6 +16,7 @@ #include "AvoidUnconditionalPreprocessorIfCheck.h" #include "BracesAroundStatementsCheck.h" #include "ConstReturnTypeCheck.h" +#include "ConstantOperandOrderCheck.h" #include "ContainerContainsCheck.h" #include "ContainerDataPointerCheck.h" #include "ContainerSizeEmptyCheck.h" @@ -86,6 +87,10 @@ class ReadabilityModule : public ClangTidyModule { "readability-braces-around-statements"); CheckFactories.registerCheck<ConstReturnTypeCheck>( "readability-const-return-type"); + CheckFactories.registerCheck<ConstantOperandOrderCheck>( + "readability-constant-operand-order"); + CheckFactories.registerCheck<ConstantOperandOrderCheck>( + "readability-constant-operand-order"); CheckFactories.registerCheck<ContainerContainsCheck>( "readability-container-contains"); CheckFactories.registerCheck<ContainerDataPointerCheck>( >From a8534c177f4715042b8e336ce8028c464e5ee5b0 Mon Sep 17 00:00:00 2001 From: Sai Deepak <[email protected]> Date: Fri, 7 Nov 2025 17:41:08 +0000 Subject: [PATCH 2/9] test(readability-constant-operand-order): add CHECK-LEFT expectations for ok_eq --- .../readability/constant-operand-order.cpp | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp new file mode 100644 index 0000000000000..570dc6fde4c2b --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp @@ -0,0 +1,35 @@ +void ok_eq(int a) { + if (a == 0) {} // CHECK-MESSAGES-NOT: readability-constant-operand-order + // CHECK-LEFT-MESSAGES: warning: constant operand should be on the Left side + // CHECK-LEFT-FIXES: if (0 == a) {} // +} + + +void swap_eq(int a) { + if (0 == a) {} // CHECK-MESSAGES: warning: constant operand should be on the Right side + // CHECK-FIXES: if (a == 0) {} // +} + +void swap_asym(int a) { + if (0 < a) {} // CHECK-MESSAGES: warning: + // CHECK-FIXES: if (a > 0) {} // +} + +void null_ptr(int *p) { + if (nullptr == p) {} // CHECK-MESSAGES: warning: + // CHECK-FIXES: if (p == nullptr) {} // +} + +// No-fix when side effects: +int g(); +void side_effects(int a) { + if (0 == g()) {} // CHECK-MESSAGES: warning: + // CHECK-FIXES-NOT: if (g() == 0) +} + +// Config variant: allow Left and only ==,!= +void left_ok(int a) { + if (0 == a) {} // CHECK-MESSAGES: warning: + // CHECK-FIXES: if (a == 0) {} // + // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order +} \ No newline at end of file >From d899e1eb8d5a7bee7be1bdccab114b081da7564d Mon Sep 17 00:00:00 2001 From: Sai Deepak <[email protected]> Date: Fri, 7 Nov 2025 17:45:23 +0000 Subject: [PATCH 3/9] test(readability-constant-operand-order): add CHECK-LEFT expectations for ok_eq --- .../checkers/readability/constant-operand-order.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp index 570dc6fde4c2b..5a4f3139a6e55 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp @@ -1,10 +1,13 @@ +// RUN: %check_clang_tidy %s readability-constant-operand-order %t -- -- -std=c++17 +// RUN: %check_clang_tidy -check-suffixes=LEFT %s readability-constant-operand-order %t -- \ +// RUN: --config="{CheckOptions:[{key: readability-constant-operand-order.PreferredConstantSide, value: Left}, {key: readability-constant-operand-order.BinaryOperators, value: '==,!='}]}" -- -std=c++17 + void ok_eq(int a) { if (a == 0) {} // CHECK-MESSAGES-NOT: readability-constant-operand-order // CHECK-LEFT-MESSAGES: warning: constant operand should be on the Left side // CHECK-LEFT-FIXES: if (0 == a) {} // } - void swap_eq(int a) { if (0 == a) {} // CHECK-MESSAGES: warning: constant operand should be on the Right side // CHECK-FIXES: if (a == 0) {} // @@ -32,4 +35,4 @@ void left_ok(int a) { if (0 == a) {} // CHECK-MESSAGES: warning: // CHECK-FIXES: if (a == 0) {} // // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order -} \ No newline at end of file +} >From 6500bb4f23e2bb39fffb499cb965314590541c83 Mon Sep 17 00:00:00 2001 From: Sai Deepak <[email protected]> Date: Fri, 7 Nov 2025 17:54:03 +0000 Subject: [PATCH 4/9] test(readability-constant-operand-order): add full CHECK-LEFT expectations --- .../checkers/readability/constant-operand-order.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp index 5a4f3139a6e55..34ff4a2826ae4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp @@ -11,16 +11,19 @@ void ok_eq(int a) { void swap_eq(int a) { if (0 == a) {} // CHECK-MESSAGES: warning: constant operand should be on the Right side // CHECK-FIXES: if (a == 0) {} // + // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } void swap_asym(int a) { if (0 < a) {} // CHECK-MESSAGES: warning: // CHECK-FIXES: if (a > 0) {} // + // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } void null_ptr(int *p) { if (nullptr == p) {} // CHECK-MESSAGES: warning: // CHECK-FIXES: if (p == nullptr) {} // + // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } // No-fix when side effects: @@ -28,6 +31,7 @@ int g(); void side_effects(int a) { if (0 == g()) {} // CHECK-MESSAGES: warning: // CHECK-FIXES-NOT: if (g() == 0) + // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } // Config variant: allow Left and only ==,!= >From 5b00277c8c93e5a5cde1f7bdb04722cd17c3de93 Mon Sep 17 00:00:00 2001 From: Sai Deepak <[email protected]> Date: Sat, 8 Nov 2025 16:35:17 +0000 Subject: [PATCH 5/9] [clang-tidy] Add readability-constant-operand-order check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This introduces a new clang-tidy readability check that enforces a consistent position for constant operands in binary comparison expressions. The check warns when a constant appears on the non-preferred side of an operator and can automatically fix simple cases (no side effects, not in macros). Configurable options: * PreferredConstantSide (Right | Left, default = Right) * BinaryOperators (comma-separated list, default = ==,!=,<,<=,>,>=) Example: if (0 == x) → if (x == 0) Includes tests, documentation, and integration with the Readability module. --- .../readability/ConstantOperandOrderCheck.cpp | 197 ++++++++++++++++++ .../readability/ConstantOperandOrderCheck.h | 58 ++++++ .../readability/ReadabilityTidyModule.cpp | 2 - clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 17 +- .../readability/constant-operand-order.rst | 32 +++ .../readability/constant-operand-order.cpp | 13 +- 7 files changed, 302 insertions(+), 22 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst diff --git a/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp new file mode 100644 index 0000000000000..215c736141adb --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp @@ -0,0 +1,197 @@ +//===----------------------------------------------------------------------===// +// +// 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 "ConstantOperandOrderCheck.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/OperationKinds.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +/// Out-of-line ctor so vtable is emitted. +ConstantOperandOrderCheck::ConstantOperandOrderCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) { + // Read options (StringRef -> std::string). + PreferredSide = Options.get(PreferredSideOption, "Right").str(); + + // Parse BinaryOperators option (comma-separated). + std::string OpsCSV = + Options.get(BinaryOperatorsOption, "==,!=,<,<=,>,>=").str(); + + llvm::SmallVector<llvm::StringRef, 8> Tokens; + llvm::StringRef(OpsCSV).split(Tokens, ','); + Operators.clear(); + for (auto Tok : Tokens) { + llvm::StringRef Trim = Tok.trim(); + if (!Trim.empty()) + Operators.emplace_back(Trim.str()); + } +} + +ConstantOperandOrderCheck::~ConstantOperandOrderCheck() = default; + +void ConstantOperandOrderCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, PreferredSideOption, PreferredSide); + Options.store(Opts, BinaryOperatorsOption, + llvm::join(Operators.begin(), Operators.end(), ",")); +} + +// ------------------------ helpers ------------------------ + +namespace { + +static const Expr *strip(const Expr *E) { + return E ? E->IgnoreParenImpCasts() : nullptr; +} + +static bool isSimpleConstantExpr(const Expr *E) { + E = strip(E); + if (!E) + return false; + + if (isa<IntegerLiteral>(E) || isa<FloatingLiteral>(E) || + isa<StringLiteral>(E) || isa<CharacterLiteral>(E) || + isa<CXXBoolLiteralExpr>(E) || isa<CXXNullPtrLiteralExpr>(E) || + isa<GNUNullExpr>(E)) + return true; + + if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) { + if (isa<EnumConstantDecl>(DRE->getDecl())) + return true; + if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) + return VD->isConstexpr() || VD->getType().isConstQualified(); + } + + return false; +} + +static bool hasSideEffectsExpr(const Expr *E, ASTContext &Ctx) { + E = strip(E); + return E && E->HasSideEffects(Ctx); +} + +static std::string invertOperatorText(llvm::StringRef Op) { + if (Op == "<") + return ">"; + if (Op == ">") + return "<"; + if (Op == "<=") + return ">="; + if (Op == ">=") + return "<="; + // symmetric: ==, != + return Op.str(); +} + +} // namespace + +// ------------------------ matchers ------------------------ + +void ConstantOperandOrderCheck::registerMatchers(MatchFinder *Finder) { + if (Operators.empty()) + return; + + for (const auto &Op : Operators) { + Finder->addMatcher(binaryOperator(hasOperatorName(Op), + hasLHS(expr().bind("lhs")), + hasRHS(expr().bind("rhs"))) + .bind("binop"), + this); + } +} + +// ------------------------ check / fixit ------------------------ + +void ConstantOperandOrderCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Bin = Result.Nodes.getNodeAs<BinaryOperator>("binop"); + const auto *L = Result.Nodes.getNodeAs<Expr>("lhs"); + const auto *R = Result.Nodes.getNodeAs<Expr>("rhs"); + if (!Bin || !L || !R) + return; + + const ASTContext &Ctx = *Result.Context; + SourceManager &SM = *Result.SourceManager; + + const Expr *LCore = strip(L); + const Expr *RCore = strip(R); + const bool LIsConst = isSimpleConstantExpr(LCore); + const bool RIsConst = isSimpleConstantExpr(RCore); + + // Only when exactly one side is constant. + if (LIsConst == RIsConst) + return; + + const bool PreferRight = (PreferredSide == "Right"); + + // If it's already on the preferred side -> nothing to do. + if ((PreferRight && RIsConst && !LIsConst) || + (!PreferRight && LIsConst && !RIsConst)) + return; + + // At this point: exactly one side is constant, and it's on the *wrong* side. + // Emit diagnosis (tests expect a warning even when we won't provide a fix). + auto D = + diag(Bin->getOperatorLoc(), "constant operand should be on the %0 side") + << PreferredSide; + + // Conservative: don't offer fix-its if swapping would move side-effects or if + // we're inside a macro expansion. + const bool LSE = L->HasSideEffects(Ctx); + const bool RSE = R->HasSideEffects(Ctx); + const bool AnyMacro = L->getBeginLoc().isMacroID() || + R->getBeginLoc().isMacroID() || + Bin->getOperatorLoc().isMacroID(); + if (LSE || RSE || AnyMacro) + return; // warning-only: no FixIt attached. + + // Get token ranges for the two operands. + CharSourceRange LRange = CharSourceRange::getTokenRange(L->getSourceRange()); + CharSourceRange RRange = CharSourceRange::getTokenRange(R->getSourceRange()); + if (LRange.isInvalid() || RRange.isInvalid()) + return; + + llvm::StringRef LText = Lexer::getSourceText(LRange, SM, Ctx.getLangOpts()); + llvm::StringRef RText = Lexer::getSourceText(RRange, SM, Ctx.getLangOpts()); + if (LText.empty() || RText.empty()) + return; + + // Compute operator replacement (invert for asymmetric operators). + llvm::StringRef OpName = Bin->getOpcodeStr(); + std::string NewOp = invertOperatorText(OpName); + + // Apply operand swaps as two independent replacements (safer than replacing + // the whole Bin range). + // Replace left operand with right text: + D << FixItHint::CreateReplacement(LRange, RText.str()); + // Replace right operand with left text: + D << FixItHint::CreateReplacement(RRange, LText.str()); + + // If needed, replace the operator token too. + if (NewOp != OpName.str()) { + // Compute an operator token range robustly: operator start and end. + SourceLocation OpStart = Bin->getOperatorLoc(); + SourceLocation OpEnd = + Lexer::getLocForEndOfToken(OpStart, 0, SM, Ctx.getLangOpts()); + if (OpStart.isValid() && OpEnd.isValid()) { + SourceRange OpRange(OpStart, OpEnd); + CharSourceRange OpTok = CharSourceRange::getTokenRange(OpRange); + if (!OpTok.isInvalid()) + D << FixItHint::CreateReplacement(OpTok, NewOp); + } + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h new file mode 100644 index 0000000000000..c776fade2a1e3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h @@ -0,0 +1,58 @@ +//===----------------------------------------------------------------------===// +// +// 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_READABILITY_CONSTANTOPERANDORDERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTANTOPERANDORDERCHECK_H + +#include "../ClangTidyCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +#include <string> +#include <vector> + +namespace clang::tidy::readability { +/// Finds binary expressions where the constant operand appears on the +/// non-preferred side (configurable) and offers a fix-it that swaps operands +/// (and inverts the operator for asymmetric operators like `<` / `>`). +/// Options +/// - BinaryOperators: comma-separated list of operators to check +/// (default: "==,!=,<,<=,>,>=") +/// - PreferredConstantSide: "Left" or "Right" (default: "Right") +class ConstantOperandOrderCheck : public ClangTidyCheck { +public: + ConstantOperandOrderCheck(StringRef Name, ClangTidyContext *Context); + ~ConstantOperandOrderCheck() override; + + // Persist options so they show up in config files. + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + // This check is primarily for C/C++, keep it limited to C++ for now. + return LangOpts.CPlusPlus; + } + +private: + // Option keys (used when storing & reading options) + static constexpr llvm::StringLiteral BinaryOperatorsOption = + "BinaryOperators"; + static constexpr llvm::StringLiteral PreferredSideOption = + "PreferredConstantSide"; + + // Runtime values, populated from Options in the constructor (or storeOptions) + std::string PreferredSide; // "Left" or "Right" + std::vector<std::string> Operators; // list of operator names, e.g. "==" + + // Implementation helpers live in the .cpp file. +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTANTOPERANDORDERCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 48feeaec4af8f..e87233b6c9aea 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -89,8 +89,6 @@ class ReadabilityModule : public ClangTidyModule { "readability-const-return-type"); CheckFactories.registerCheck<ConstantOperandOrderCheck>( "readability-constant-operand-order"); - CheckFactories.registerCheck<ConstantOperandOrderCheck>( - "readability-constant-operand-order"); CheckFactories.registerCheck<ContainerContainsCheck>( "readability-container-contains"); CheckFactories.registerCheck<ContainerDataPointerCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 666865cfb2fcd..7a56733614023 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -226,6 +226,11 @@ New checks Finds virtual function overrides with different visibility than the function in the base class. +- New :doc:`readability-constant-operand-order + <clang-tidy/checks/readability/constant-operand-order>` check. + + FIXME: Write a short description. + - New :doc:`readability-redundant-parentheses <clang-tidy/checks/readability/redundant-parentheses>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index e2875604af72b..bc115571de1f0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -178,8 +178,13 @@ Clang-Tidy Checks :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`, :doc:`bugprone-use-after-move <bugprone/use-after-move>`, :doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes" + :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`, + :doc:`cert-env33-c <cert/env33-c>`, :doc:`cert-err33-c <cert/err33-c>`, + :doc:`cert-err52-cpp <cert/err52-cpp>`, :doc:`cert-err60-cpp <cert/err60-cpp>`, + :doc:`cert-flp30-c <cert/flp30-c>`, + :doc:`cert-mem57-cpp <cert/mem57-cpp>`, :doc:`cert-msc50-cpp <cert/msc50-cpp>`, :doc:`cert-msc51-cpp <cert/msc51-cpp>`, :doc:`cert-oop58-cpp <cert/oop58-cpp>`, @@ -373,8 +378,9 @@ Clang-Tidy Checks :doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`, :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, "Yes" :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, - :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" + :doc:`readability-braces-around-statements <readability/braces-around-statements>`, :doc:`readability-const-return-type <readability/const-return-type>`, "Yes" + :doc:`readability-constant-operand-order <readability/constant-operand-order>`, "Yes" :doc:`readability-container-contains <readability/container-contains>`, "Yes" :doc:`readability-container-data-pointer <readability/container-data-pointer>`, "Yes" :doc:`readability-container-size-empty <readability/container-size-empty>`, "Yes" @@ -442,20 +448,15 @@ Check aliases :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`, :doc:`modernize-avoid-variadic-functions <modernize/avoid-variadic-functions>`, :doc:`cert-dcl51-cpp <cert/dcl51-cpp>`, :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" :doc:`cert-dcl54-cpp <cert/dcl54-cpp>`, :doc:`misc-new-delete-overloads <misc/new-delete-overloads>`, - :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`, :doc:`bugprone-std-namespace-modification <bugprone/std-namespace-modification>`, :doc:`cert-dcl59-cpp <cert/dcl59-cpp>`, :doc:`google-build-namespaces <google/build-namespaces>`, - :doc:`cert-env33-c <cert/env33-c>`, :doc:`bugprone-command-processor <bugprone/command-processor>`, :doc:`cert-err09-cpp <cert/err09-cpp>`, :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`, :doc:`cert-err34-c <cert/err34-c>`, :doc:`bugprone-unchecked-string-to-number-conversion <bugprone/unchecked-string-to-number-conversion>`, - :doc:`cert-err52-cpp <cert/err52-cpp>`, :doc:`modernize-avoid-setjmp-longjmp <modernize/avoid-setjmp-longjmp>`, :doc:`cert-err58-cpp <cert/err58-cpp>`, :doc:`bugprone-throwing-static-initialization <bugprone/throwing-static-initialization>`, :doc:`cert-err61-cpp <cert/err61-cpp>`, :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`, :doc:`cert-exp42-c <cert/exp42-c>`, :doc:`bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison>`, :doc:`cert-fio38-c <cert/fio38-c>`, :doc:`misc-non-copyable-objects <misc/non-copyable-objects>`, - :doc:`cert-flp30-c <cert/flp30-c>`, :doc:`bugprone-float-loop-counter <bugprone/float-loop-counter>`, :doc:`cert-flp37-c <cert/flp37-c>`, :doc:`bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison>`, :doc:`cert-int09-c <cert/int09-c>`, :doc:`readability-enum-initial-value <readability/enum-initial-value>`, "Yes" - :doc:`cert-mem57-cpp <cert/mem57-cpp>`, :doc:`bugprone-default-operator-new-on-overaligned-type <bugprone/default-operator-new-on-overaligned-type>`, :doc:`cert-msc24-c <cert/msc24-c>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`, :doc:`cert-msc30-c <cert/msc30-c>`, :doc:`cert-msc50-cpp <cert/msc50-cpp>`, :doc:`cert-msc32-c <cert/msc32-c>`, :doc:`cert-msc51-cpp <cert/msc51-cpp>`, @@ -578,12 +579,12 @@ Check aliases :doc:`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines/non-private-member-variables-in-classes>`, :doc:`misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes>`, :doc:`cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init>`, :doc:`modernize-use-default-member-init <modernize/use-default-member-init>`, "Yes" :doc:`fuchsia-header-anon-namespaces <fuchsia/header-anon-namespaces>`, :doc:`google-build-namespaces <google/build-namespaces>`, - :doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" + :doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, :doc:`google-readability-function-size <google/readability-function-size>`, :doc:`readability-function-size <readability/function-size>`, :doc:`google-readability-namespace-comments <google/readability-namespace-comments>`, :doc:`llvm-namespace-comment <llvm/namespace-comment>`, :doc:`hicpp-avoid-c-arrays <hicpp/avoid-c-arrays>`, :doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`, :doc:`hicpp-avoid-goto <hicpp/avoid-goto>`, :doc:`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto>`, - :doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" + :doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, :doc:`hicpp-deprecated-headers <hicpp/deprecated-headers>`, :doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes" :doc:`hicpp-explicit-conversions <hicpp/explicit-conversions>`, :doc:`google-explicit-constructor <google/explicit-constructor>`, "Yes" :doc:`hicpp-function-size <hicpp/function-size>`, :doc:`readability-function-size <readability/function-size>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst new file mode 100644 index 0000000000000..592aa5d085f9f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst @@ -0,0 +1,32 @@ +.. title:: clang-tidy - readability-constant-operand-order + +readability-constant-operand-order +================================== + +Warns when a constant appears on the non-preferred side of a supported binary +operator and offers a fix-it to swap operands (and invert the operator for +``<``, ``>``, ``<=``, ``>=``). + +Examples +-------- + +.. code-block:: c++ + + // Before + if (nullptr == p) { /* ... */ } + if (0 < x) { /* ... */ } + + // After + if (p == nullptr) { /* ... */ } + if (x > 0) { /* ... */ } + +Options +------- + +.. option:: PreferredConstantSide (string) + + Either ``Left`` or ``Right``. Default: ``Right``. + +.. option:: BinaryOperators (string) + + Comma-separated list of operators to check. Default: ``==,!=,<,<=,>,>=``. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp index 34ff4a2826ae4..c65c37b16e745 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/constant-operand-order.cpp @@ -1,29 +1,19 @@ // RUN: %check_clang_tidy %s readability-constant-operand-order %t -- -- -std=c++17 -// RUN: %check_clang_tidy -check-suffixes=LEFT %s readability-constant-operand-order %t -- \ -// RUN: --config="{CheckOptions:[{key: readability-constant-operand-order.PreferredConstantSide, value: Left}, {key: readability-constant-operand-order.BinaryOperators, value: '==,!='}]}" -- -std=c++17 - -void ok_eq(int a) { - if (a == 0) {} // CHECK-MESSAGES-NOT: readability-constant-operand-order - // CHECK-LEFT-MESSAGES: warning: constant operand should be on the Left side - // CHECK-LEFT-FIXES: if (0 == a) {} // -} +// RUN: %check_clang_tidy %s readability-constant-operand-order %t -- -check-suffixes=LEFT -- -config="{CheckOptions:[{key: readability-constant-operand-order.PreferredConstantSide, value: Left}, {key: readability-constant-operand-order.BinaryOperators, value: '==,!='}]}" -- -std=c++17 void swap_eq(int a) { if (0 == a) {} // CHECK-MESSAGES: warning: constant operand should be on the Right side // CHECK-FIXES: if (a == 0) {} // - // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } void swap_asym(int a) { if (0 < a) {} // CHECK-MESSAGES: warning: // CHECK-FIXES: if (a > 0) {} // - // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } void null_ptr(int *p) { if (nullptr == p) {} // CHECK-MESSAGES: warning: // CHECK-FIXES: if (p == nullptr) {} // - // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } // No-fix when side effects: @@ -31,7 +21,6 @@ int g(); void side_effects(int a) { if (0 == g()) {} // CHECK-MESSAGES: warning: // CHECK-FIXES-NOT: if (g() == 0) - // CHECK-LEFT-MESSAGES-NOT: readability-constant-operand-order } // Config variant: allow Left and only ==,!= >From 27824416398c86b830bed156c466fbbaae8f6f5a Mon Sep 17 00:00:00 2001 From: Sai Deepak Sana <[email protected]> Date: Sun, 9 Nov 2025 18:49:57 +0530 Subject: [PATCH 6/9] Update clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp Co-authored-by: EugeneZelenko <[email protected]> --- .../clang-tidy/readability/ConstantOperandOrderCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp index 215c736141adb..b8dc8ba44928e 100644 --- a/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.cpp @@ -169,8 +169,8 @@ void ConstantOperandOrderCheck::check(const MatchFinder::MatchResult &Result) { return; // Compute operator replacement (invert for asymmetric operators). - llvm::StringRef OpName = Bin->getOpcodeStr(); - std::string NewOp = invertOperatorText(OpName); + const StringRef OpName = Bin->getOpcodeStr(); + const std::string NewOp = invertOperatorText(OpName); // Apply operand swaps as two independent replacements (safer than replacing // the whole Bin range). >From 848efd9364e4d5c6682e0497fec76cef2eee6f77 Mon Sep 17 00:00:00 2001 From: Sai Deepak Sana <[email protected]> Date: Sun, 9 Nov 2025 18:50:11 +0530 Subject: [PATCH 7/9] Update clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h Co-authored-by: EugeneZelenko <[email protected]> --- .../clang-tidy/readability/ConstantOperandOrderCheck.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h index c776fade2a1e3..ecb1b978e27c7 100644 --- a/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ConstantOperandOrderCheck.h @@ -49,8 +49,6 @@ class ConstantOperandOrderCheck : public ClangTidyCheck { // Runtime values, populated from Options in the constructor (or storeOptions) std::string PreferredSide; // "Left" or "Right" std::vector<std::string> Operators; // list of operator names, e.g. "==" - - // Implementation helpers live in the .cpp file. }; } // namespace clang::tidy::readability >From be965fe88c2e5412aa003ac9b56250ca021a23cb Mon Sep 17 00:00:00 2001 From: Sai Deepak Sana <[email protected]> Date: Sun, 9 Nov 2025 18:50:34 +0530 Subject: [PATCH 8/9] Update clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst Co-authored-by: EugeneZelenko <[email protected]> --- .../clang-tidy/checks/readability/constant-operand-order.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst index 592aa5d085f9f..dbcb5d99c6f24 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst @@ -4,7 +4,7 @@ readability-constant-operand-order ================================== Warns when a constant appears on the non-preferred side of a supported binary -operator and offers a fix-it to swap operands (and invert the operator for +operator and offers a fix-it note to swap operands (and invert the operator for ``<``, ``>``, ``<=``, ``>=``). Examples >From 99615185baeb0ee0ba9d82c68b27ae2a64be5624 Mon Sep 17 00:00:00 2001 From: Sai Deepak Sana <[email protected]> Date: Sun, 9 Nov 2025 18:50:46 +0530 Subject: [PATCH 9/9] Update clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst Co-authored-by: EugeneZelenko <[email protected]> --- .../clang-tidy/checks/readability/constant-operand-order.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst index dbcb5d99c6f24..55979e53f2740 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/constant-operand-order.rst @@ -25,7 +25,7 @@ Options .. option:: PreferredConstantSide (string) - Either ``Left`` or ``Right``. Default: ``Right``. + Either `Left` or `Right`. Default: `Right`. .. option:: BinaryOperators (string) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
