[clang-tools-extra] 08932dd - [clang-tidy] Detect std::rot[lr] pattern within modernize.use-std-bit (#186324)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 14 13:59:14 PDT 2026
Author: serge-sans-paille
Date: 2026-04-14T20:59:09Z
New Revision: 08932ddde177a7f9dacf5d34e209c48f3d52800e
URL: https://github.com/llvm/llvm-project/commit/08932ddde177a7f9dacf5d34e209c48f3d52800e
DIFF: https://github.com/llvm/llvm-project/commit/08932ddde177a7f9dacf5d34e209c48f3d52800e.diff
LOG: [clang-tidy] Detect std::rot[lr] pattern within modernize.use-std-bit (#186324)
Basically turning `x << N | x >> (64 - N)` into `std::rotl(x, N)`.
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.h
clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp
index e649c1c5aadda..0abe7d6323bdf 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;
@@ -17,7 +18,8 @@ UseStdBitCheck::UseStdBitCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
utils::IncludeSorter::IS_LLVM),
- areDiagsSelfContained()) {}
+ areDiagsSelfContained()),
+ HonorIntPromotion(Options.get("HonorIntPromotion", false)) {}
void UseStdBitCheck::registerMatchers(MatchFinder *Finder) {
const auto MakeBinaryOperatorMatcher = [](auto Op) {
@@ -37,7 +39,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 +92,16 @@ 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(
+ expr(BitwiseOr(ShiftLeft(BindDeclRef("v"),
+ integerLiteral().bind("shift_left_amount")),
+ ShiftRight(BoundDeclRef("v"),
+ integerLiteral().bind("shift_right_amount"))),
+ optionally(hasParent(castExpr(hasType(isInteger())).bind("cast"))))
+ .bind("rotate_expr"),
+ this);
}
void UseStdBitCheck::registerPPCallbacks(const SourceManager &SM,
@@ -97,10 +112,11 @@ void UseStdBitCheck::registerPPCallbacks(const SourceManager &SM,
void UseStdBitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+ Options.store(Opts, "HonorIntPromotion", HonorIntPromotion);
}
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 =
@@ -141,6 +157,59 @@ void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) {
<< IncludeInserter.createIncludeInsertion(
Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>");
}
+ } else if (const auto *MatchedExpr =
+ 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.
+
+ bool HasExplicitIntegerCast = false;
+ if (const Expr *CE = Result.Nodes.getNodeAs<CastExpr>("cast"))
+ HasExplicitIntegerCast = !isa<ImplicitCastExpr>(CE);
+
+ const auto *MatchedVarDecl = Result.Nodes.getNodeAs<VarDecl>("v");
+ const llvm::APInt ShiftLeftAmount =
+ Result.Nodes.getNodeAs<IntegerLiteral>("shift_left_amount")->getValue();
+ const llvm::APInt 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;
+
+ // Only insert cast if the operand is not subject to cast and
+ // some implicit promotion happened.
+ const bool NeedsIntCast =
+ HonorIntPromotion && !HasExplicitIntegerCast &&
+ Context.getTypeSize(MatchedExpr->getType()) > MatchedVarSize;
+ 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())
+ return;
+
+ Diag << FixItHint::CreateReplacement(
+ MatchedExpr->getSourceRange(),
+ llvm::formatv("{3}std::{0}({1}, {2}){4}", ReplacementFuncName,
+ MatchedVarDecl->getName(), ReplacementShiftAmount,
+ 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..abc76dc8c0d5b 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 HonorIntPromotion;
};
} // namespace clang::tidy::modernize
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..36848b7e3d228 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,31 @@ 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)``
============================== =======================
+
+Options
+-------
+
+.. option:: HonorIntPromotion
+
+ When set to ``true`` (default is ``false``), insert explicit cast to make sure the
+ type of the substituted expression is unchanged. Example:
+
+ .. code:: c++
+
+ // Return type is deduced as 'int' (not 'unsigned char') due to implicit conversions.
+ 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));
+ }
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..615d1202e8092 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=,NOPROMOTION
+// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-std-bit %t -config="{CheckOptions: { modernize-use-std-bit.HonorIntPromotion: true }}" -check-suffixes=,PROMOTION
// CHECK-FIXES: #include <bit>
/*
@@ -195,3 +196,109 @@ unsigned invalid_popcount_bitset(unsigned x, signed y) {
};
}
+
+/*
+ * rotate patterns
+ */
+
+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-NOPROMOTION: return std::rotl(x, 3);
+ // CHECK-FIXES-PROMOTION: 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-NOPROMOTION: return std::rotl(x, 3);
+ // CHECK-FIXES-PROMOTION: 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-NOPROMOTION: return std::rotl(x, 3);
+ // CHECK-FIXES-PROMOTION: return static_cast<int>(std::rotl(x, 3));
+ return x << 3 | x >> 5;
+}
+
+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-NOPROMOTION: return (std::rotl(x, 3));
+ // CHECK-FIXES-PROMOTION: 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 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-NOPROMOTION: return std::rotl(x, 3);
+ // CHECK-FIXES-PROMOTION: return static_cast<int>(std::rotl(x, 3));
+ return x >> 5 | x << 3;
+}
+
+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 std::rotl(x, 16);
+ return x << 16 | x >> 16;
+}
+
+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 std::rotr(x, 3);
+ return (x << 61) | ((x >> 3));
+}
+
+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-NOPROMOTION: return std::rotr(x0, 3);
+ // CHECK-FIXES-PROMOTION: return static_cast<int>(std::rotr(x0, 3));
+ return x0 >> 3 | x0 << 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,
+ };
+}
More information about the cfe-commits
mailing list