https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/186324
>From 213d4379603917e11bf83b9bd29471c9d1e12749 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Thu, 12 Mar 2026 15:54:49 +0100 Subject: [PATCH 1/3] [clang-tidy] Detect std::rot[lr] pattern within modernize.use-std-bit Basically turning `x << N | x >> (64 - N)` into `std::rotl(x, N)`. --- .../clang-tidy/modernize/UseStdBitCheck.cpp | 55 ++++++++++++++++++ .../checks/modernize/use-std-bit.rst | 2 + .../checkers/modernize/use-std-bit.cpp | 56 +++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp index e649c1c5aadda..7fbde04bc4525 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp @@ -8,6 +8,7 @@ #include "UseStdBitCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/Support/FormatVariadic.h" using namespace clang::ast_matchers; @@ -37,7 +38,10 @@ void UseStdBitCheck::registerMatchers(MatchFinder *Finder) { const auto LogicalAnd = MakeCommutativeBinaryOperatorMatcher("&&"); const auto Sub = MakeBinaryOperatorMatcher("-"); + const auto ShiftLeft = MakeBinaryOperatorMatcher("<<"); + const auto ShiftRight = MakeBinaryOperatorMatcher(">>"); const auto BitwiseAnd = MakeCommutativeBinaryOperatorMatcher("&"); + const auto BitwiseOr = MakeCommutativeBinaryOperatorMatcher("|"); const auto CmpNot = MakeCommutativeBinaryOperatorMatcher("!="); const auto CmpGt = MakeBinaryOperatorMatcher(">"); const auto CmpGte = MakeBinaryOperatorMatcher(">="); @@ -87,6 +91,15 @@ void UseStdBitCheck::registerMatchers(MatchFinder *Finder) { hasArgument(0, expr(hasType(isUnsignedInteger())).bind("v"))))) .bind("popcount_expr"), this); + + // Rotating an integer by a fixed amount + Finder->addMatcher( + BitwiseOr(ShiftLeft(BindDeclRef("v"), + integerLiteral().bind("shift_left_amount")), + ShiftRight(BoundDeclRef("v"), + integerLiteral().bind("shift_right_amount"))) + .bind("rotate_expr"), + this); } void UseStdBitCheck::registerPPCallbacks(const SourceManager &SM, @@ -141,6 +154,48 @@ void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) { << IncludeInserter.createIncludeInsertion( Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); } + } else if (const auto *MatchedExpr = + Result.Nodes.getNodeAs<BinaryOperator>("rotate_expr")) { + const auto *MatchedVarDecl = Result.Nodes.getNodeAs<VarDecl>("v"); + const auto ShiftLeftAmount = + Result.Nodes.getNodeAs<IntegerLiteral>("shift_left_amount")->getValue(); + const auto ShiftRightAmount = + Result.Nodes.getNodeAs<IntegerLiteral>("shift_right_amount") + ->getValue(); + const uint64_t MatchedVarSize = + Context.getTypeSize(MatchedVarDecl->getType()); + + // Overflowing shifts + if (ShiftLeftAmount.sge(MatchedVarSize)) + return; + if (ShiftRightAmount.sge(MatchedVarSize)) + return; + // Not a rotation. + if (MatchedVarSize != (ShiftLeftAmount + ShiftRightAmount)) + return; + + const bool NeedsIntCast = + MatchedExpr->getType() != MatchedVarDecl->getType(); + const bool IsRotl = ShiftRightAmount.sge(ShiftLeftAmount); + + const StringRef ReplacementFuncName = IsRotl ? "rotl" : "rotr"; + const uint64_t ReplacementShiftAmount = + (IsRotl ? ShiftLeftAmount : ShiftRightAmount).getZExtValue(); + auto Diag = diag(MatchedExpr->getBeginLoc(), "use 'std::%0' instead") + << ReplacementFuncName; + if (auto R = MatchedExpr->getSourceRange(); + !R.getBegin().isMacroID() && !R.getEnd().isMacroID()) { + Diag << FixItHint::CreateReplacement( + MatchedExpr->getSourceRange(), + llvm::formatv("{3}std::{0}({1}, {2})", ReplacementFuncName, + MatchedVarDecl->getName(), + ReplacementShiftAmount, + NeedsIntCast ? "(int)" : "") + .str()) + << IncludeInserter.createIncludeInsertion( + Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); + } + } else { llvm_unreachable("unexpected match"); } diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst index 26ef1b0841654..0428a48c653fe 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst @@ -15,4 +15,6 @@ Expression Replacement ``(x != 0) && !(x & (x - 1))`` ``std::has_one_bit(x)`` ``(x > 0) && !(x & (x - 1))`` ``std::has_one_bit(x)`` ``std::bitset<N>(x).count()`` ``std::popcount(x)`` +``x << 3 | x >> 61`` ``std::rotl(x, 3)`` +``x << 61 | x >> 3`` ``std::rotr(x, 3)`` ============================== ======================= diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp index 52c725282f33c..0392fb9b8ec63 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp @@ -195,3 +195,59 @@ unsigned invalid_popcount_bitset(unsigned x, signed y) { }; } + +/* + * rotate patterns + */ +unsigned char rotate_left_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotl(x, 3); + return x << 3 | x >> 5; +} +unsigned char rotate_left_pattern_perm(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotl(x, 3); + return x >> 5 | x << 3; +} +unsigned char rotate_swap_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotl(x, 4); + return x << 4 | x >> 4; +} +unsigned char rotate_right_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotr(x, 3); + return x << 5 | x >> 3; +} +unsigned char rotate_right_pattern_perm(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotr(x, 3); + return x >> 3 | x << 5; +} + +#define ROTR v >> 3 | v << 5 +unsigned char rotate_macro(unsigned char v) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // No fixes, it comes from macro expansion. + return ROTR; +} + +/* + * Invalid rotate patterns + */ +void invalid_rotate_patterns(unsigned char x, signed char y, unsigned char z) { + int patterns[] = { + // non-matching references + x >> 3 | z << 5, + // bad shift combination + x >> 3 | x << 6, + x >> 4 | x << 3, + // bad operator combination + x << 3 | x << 6, + x + 3 | x << 6, + x >> 3 & x << 5, + x >> 5 ^ x << 3, + // unsupported types + y >> 4 | y << 4, + }; +} >From 649950bd09e10e85e7773b08da98fc60cdc9e219 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Thu, 19 Mar 2026 00:11:42 +0100 Subject: [PATCH 2/3] fixup! [clang-tidy] Detect std::rot[lr] pattern within modernize.use-std-bit --- .../clang-tidy/modernize/UseStdBitCheck.cpp | 77 ++++++++++++----- .../clang-tidy/modernize/UseStdBitCheck.h | 1 + .../checkers/modernize/use-std-bit.cpp | 83 +++++++++++++++---- 3 files changed, 123 insertions(+), 38 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp index 7fbde04bc4525..57f8a4ddf430a 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp @@ -18,7 +18,8 @@ UseStdBitCheck::UseStdBitCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM), - areDiagsSelfContained()) {} + areDiagsSelfContained()), + StrictMode(Options.get("StrictMode", false)) {} void UseStdBitCheck::registerMatchers(MatchFinder *Finder) { const auto MakeBinaryOperatorMatcher = [](auto Op) { @@ -94,11 +95,13 @@ void UseStdBitCheck::registerMatchers(MatchFinder *Finder) { // Rotating an integer by a fixed amount Finder->addMatcher( - BitwiseOr(ShiftLeft(BindDeclRef("v"), - integerLiteral().bind("shift_left_amount")), - ShiftRight(BoundDeclRef("v"), - integerLiteral().bind("shift_right_amount"))) - .bind("rotate_expr"), + traverse( + TK_AsIs, + BitwiseOr(ShiftLeft(BindDeclRef("v"), + integerLiteral().bind("shift_left_amount")), + ShiftRight(BoundDeclRef("v"), + integerLiteral().bind("shift_right_amount"))) + .bind("rotate_expr")), this); } @@ -110,10 +113,21 @@ void UseStdBitCheck::registerPPCallbacks(const SourceManager &SM, void UseStdBitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); + Options.store(Opts, "StrictMode", StrictMode); +} + +static const Expr *GetParentExprOrSelf(const Expr *E, ASTContext &Context) { + ParentMapContext &PMap = Context.getParentMapContext(); + DynTypedNodeList P = PMap.getParents(*E); + if (P.size() != 1) + return nullptr; + + const Expr *ParentE = P[0].get<Expr>(); + return ParentE ? ParentE : E; } void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) { - const ASTContext &Context = *Result.Context; + ASTContext &Context = *Result.Context; const SourceManager &Source = Context.getSourceManager(); if (const auto *MatchedExpr = @@ -155,11 +169,20 @@ void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) { Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); } } else if (const auto *MatchedExpr = - Result.Nodes.getNodeAs<BinaryOperator>("rotate_expr")) { + Result.Nodes.getNodeAs<Expr>("rotate_expr")) { + // Detect if the expression is an explicit cast. If that's the case we don't + // need to insert a cast. + const Expr *ParentExprOrSelf = GetParentExprOrSelf(MatchedExpr, Context); + bool HasExplicitIntegerCast = false; + if (const auto *CE = dyn_cast<CastExpr>(ParentExprOrSelf)) { + HasExplicitIntegerCast = + CE->getType()->isIntegerType() && !isa<ImplicitCastExpr>(CE); + } + const auto *MatchedVarDecl = Result.Nodes.getNodeAs<VarDecl>("v"); - const auto ShiftLeftAmount = + const llvm::APInt ShiftLeftAmount = Result.Nodes.getNodeAs<IntegerLiteral>("shift_left_amount")->getValue(); - const auto ShiftRightAmount = + const llvm::APInt ShiftRightAmount = Result.Nodes.getNodeAs<IntegerLiteral>("shift_right_amount") ->getValue(); const uint64_t MatchedVarSize = @@ -174,8 +197,11 @@ void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) { if (MatchedVarSize != (ShiftLeftAmount + ShiftRightAmount)) return; + // Only insert cast if the operand is the result is not subject to cast and + // some implicit promotion happened. const bool NeedsIntCast = - MatchedExpr->getType() != MatchedVarDecl->getType(); + StrictMode && !HasExplicitIntegerCast && + Context.getTypeSize(MatchedExpr->getType()) > MatchedVarSize; const bool IsRotl = ShiftRightAmount.sge(ShiftLeftAmount); const StringRef ReplacementFuncName = IsRotl ? "rotl" : "rotr"; @@ -184,17 +210,24 @@ void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) { auto Diag = diag(MatchedExpr->getBeginLoc(), "use 'std::%0' instead") << ReplacementFuncName; if (auto R = MatchedExpr->getSourceRange(); - !R.getBegin().isMacroID() && !R.getEnd().isMacroID()) { - Diag << FixItHint::CreateReplacement( - MatchedExpr->getSourceRange(), - llvm::formatv("{3}std::{0}({1}, {2})", ReplacementFuncName, - MatchedVarDecl->getName(), - ReplacementShiftAmount, - NeedsIntCast ? "(int)" : "") - .str()) - << IncludeInserter.createIncludeInsertion( - Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); - } + R.getBegin().isMacroID() && !R.getEnd().isMacroID()) + return; + + const SourceLocation PreviousLocation = + MatchedExpr->getBeginLoc().getLocWithOffset(-1); + const bool NeedsSpace = + isAlphanumeric(*Source.getCharacterData(PreviousLocation)); + + Diag << FixItHint::CreateReplacement( + MatchedExpr->getSourceRange(), + llvm::formatv("{3}{4}std::{0}({1}, {2}){5}", + ReplacementFuncName, MatchedVarDecl->getName(), + ReplacementShiftAmount, NeedsSpace ? " " : "", + NeedsIntCast ? "static_cast<int>(" : "", + NeedsIntCast ? ")" : "") + .str()) + << IncludeInserter.createIncludeInsertion( + Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); } else { llvm_unreachable("unexpected match"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.h index 6dd455d4286c4..afdebb863a8fe 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.h @@ -37,6 +37,7 @@ class UseStdBitCheck : public ClangTidyCheck { private: utils::IncludeInserter IncludeInserter; + const bool StrictMode; }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp index 0392fb9b8ec63..b8680e8127302 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp @@ -1,4 +1,5 @@ -// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-std-bit %t +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-std-bit %t -check-suffixes=,NOSTRICT +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-std-bit %t -config="{CheckOptions: { modernize-use-std-bit.StrictMode: true }}" -check-suffixes=,STRICT // CHECK-FIXES: #include <bit> /* @@ -199,30 +200,80 @@ unsigned invalid_popcount_bitset(unsigned x, signed y) { /* * rotate patterns */ -unsigned char rotate_left_pattern(unsigned char x) { - // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] - // CHECK-FIXES: return (int)std::rotl(x, 3); + +using uint64_t = __UINT64_TYPE__; +using uint32_t = __UINT32_TYPE__; + +int rotate_left_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+3]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES-NOSTRICT: return std::rotl(x, 3); + // CHECK-FIXES-STRICT: return static_cast<int>(std::rotl(x, 3)); + return (x) << 3 | x >> 5; +} + +auto rotate_left_pattern_with_cast(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:29: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return static_cast<short>(std::rotl(x, 3)); + return static_cast<short>((x) << 3 | x >> 5); +} + +unsigned char rotate_left_pattern_with_implicit_cast(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+3]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES-NOSTRICT: return std::rotl(x, 3); + // CHECK-FIXES-STRICT: return static_cast<int>(std::rotl(x, 3)); + return (x) << 3 | x >> 5; +} + +auto rotate_left_pattern_without_cast(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+3]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES-NOSTRICT: return std::rotl(x, 3); + // CHECK-FIXES-STRICT: return static_cast<int>(std::rotl(x, 3)); return x << 3 | x >> 5; } -unsigned char rotate_left_pattern_perm(unsigned char x) { + +uint32_t rotate_left_pattern_with_surrounding_parenthesis(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+3]]:11: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES-NOSTRICT: return (std::rotl(x, 3)); + // CHECK-FIXES-STRICT: return (static_cast<int>(std::rotl(x, 3))); + return (x << 3 | x >> 5); +} + +uint64_t rotate_left_pattern_int64(uint64_t x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return std::rotl(x, 3); + return x << 3 | x >> 61; +} + +uint32_t rotate_left_pattern_int32(uint32_t x) { // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] - // CHECK-FIXES: return (int)std::rotl(x, 3); + // CHECK-FIXES: return std::rotl(x, 3); + return (x) << 3 | x >> 29; +} + +unsigned char rotate_left_pattern_perm(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+3]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES-NOSTRICT: return std::rotl(x, 3); + // CHECK-FIXES-STRICT: return static_cast<int>(std::rotl(x, 3)); return x >> 5 | x << 3; } -unsigned char rotate_swap_pattern(unsigned char x) { + +uint32_t rotate_swap_pattern(uint32_t x) { // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] - // CHECK-FIXES: return (int)std::rotl(x, 4); - return x << 4 | x >> 4; + // CHECK-FIXES: return std::rotl(x, 16); + return x << 16 | x >> 16; } -unsigned char rotate_right_pattern(unsigned char x) { + +uint64_t rotate_right_pattern(uint64_t x) { // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] - // CHECK-FIXES: return (int)std::rotr(x, 3); - return x << 5 | x >> 3; + // CHECK-FIXES: return std::rotr(x, 3); + return (x << 61) | ((x >> 3)); } -unsigned char rotate_right_pattern_perm(unsigned char x) { - // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] - // CHECK-FIXES: return (int)std::rotr(x, 3); - return x >> 3 | x << 5; + +unsigned char rotate_right_pattern_perm(unsigned char x0) { + // CHECK-MESSAGES: :[[@LINE+3]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // CHECK-FIXES-NOSTRICT: return std::rotr(x0, 3); + // CHECK-FIXES-STRICT: return static_cast<int>(std::rotr(x0, 3)); + return x0 >> 3 | x0 << 5; } #define ROTR v >> 3 | v << 5 >From d6e0f72539e854da409d863c0c58014cbbaa9cb6 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Thu, 19 Mar 2026 12:12:29 +0100 Subject: [PATCH 3/3] fixup! fixup! [clang-tidy] Detect std::rot[lr] pattern within modernize.use-std-bit --- .../checks/modernize/use-std-bit.rst | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst index 0428a48c653fe..faaccca6eed08 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst @@ -18,3 +18,27 @@ Expression Replacement ``x << 3 | x >> 61`` ``std::rotl(x, 3)`` ``x << 61 | x >> 3`` ``std::rotr(x, 3)`` ============================== ======================= + +Options +------- + +.. option:: StrictMode + + When set to ``true`` (default is ``false``), insert explicit cast to make sure the + type of the substituted expression is unchanged. Example: + + .. code:: c++ + + auto foo(unsigned char x) { + return x << 3 | x >> 5; + } + + Becomes: + + .. code:: c++ + + #include <bit> + + auto foo(unsigned char x) { + return static_cast<int>(std::rotl(x, 3)); + } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
