Author: Gaurav Dhingra Date: 2026-05-07T13:39:56+08:00 New Revision: a63309dffd7da7f07d7f9fb5fb196ad5efe2044d
URL: https://github.com/llvm/llvm-project/commit/a63309dffd7da7f07d7f9fb5fb196ad5efe2044d DIFF: https://github.com/llvm/llvm-project/commit/a63309dffd7da7f07d7f9fb5fb196ad5efe2044d.diff LOG: [clang-tidy] Improve redundant-casting check for binary operation (#191386) Mark explicit casting as readabily redundant for a BinaryOperation with atleast one operand of the same type as the cast type in `RedundantCastingCheck` E.g.; `static<float>(1 + 2.0f)` // redundant cast Fixes #182132 Added: Modified: clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp index eae0b3f6d0e7d..e35466158f800 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp @@ -50,8 +50,8 @@ static bool areTypesEqual(QualType TypeS, QualType TypeD, TypeD.getLocalUnqualifiedType()); } -static bool areBinaryOperatorOperandsTypesEqualToOperatorResultType( - const Expr *E, bool IgnoreTypeAliases) { +static bool binaryOperatorOperandsTypesEqualToOperatorResultType( + const Expr *E, bool IgnoreTypeAliases, bool IgnoreImplicitCasts) { if (!E) return true; const Expr *WithoutImplicitAndParen = E->IgnoreParenImpCasts(); @@ -64,12 +64,21 @@ static bool areBinaryOperatorOperandsTypesEqualToOperatorResultType( const QualType NonReferenceType = Type.getNonReferenceType(); const QualType LHSType = B->getLHS()->IgnoreImplicit()->getType(); - if (LHSType.isNull() || !areTypesEqual(LHSType.getNonReferenceType(), - NonReferenceType, IgnoreTypeAliases)) - return false; const QualType RHSType = B->getRHS()->IgnoreImplicit()->getType(); - if (RHSType.isNull() || !areTypesEqual(RHSType.getNonReferenceType(), - NonReferenceType, IgnoreTypeAliases)) + const bool LHSMatches = + !LHSType.isNull() && areTypesEqual(LHSType.getNonReferenceType(), + NonReferenceType, IgnoreTypeAliases); + const bool RHSMatches = + !RHSType.isNull() && areTypesEqual(RHSType.getNonReferenceType(), + NonReferenceType, IgnoreTypeAliases); + // Explicit Cast is needed if: + // IgnoreImplicitCasts = false: neither of operands type matches cast type + // IgnoreImplicitCasts = true: at least one operand type doesn't match cast + // type + const bool CastIsNeeded = IgnoreImplicitCasts + ? (!LHSMatches || !RHSMatches) + : (!LHSMatches && !RHSMatches); + if (CastIsNeeded) return false; } return true; @@ -92,11 +101,13 @@ RedundantCastingCheck::RedundantCastingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IgnoreMacros(Options.get("IgnoreMacros", true)), - IgnoreTypeAliases(Options.get("IgnoreTypeAliases", false)) {} + IgnoreTypeAliases(Options.get("IgnoreTypeAliases", false)), + IgnoreImplicitCasts(Options.get("IgnoreImplicitCasts", false)) {} void RedundantCastingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreMacros", IgnoreMacros); Options.store(Opts, "IgnoreTypeAliases", IgnoreTypeAliases); + Options.store(Opts, "IgnoreImplicitCasts", IgnoreImplicitCasts); } void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { @@ -145,8 +156,8 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { if (!areTypesEqual(TypeS, TypeD, IgnoreTypeAliases)) return; - if (!areBinaryOperatorOperandsTypesEqualToOperatorResultType( - SourceExpr, IgnoreTypeAliases)) + if (!binaryOperatorOperandsTypesEqualToOperatorResultType( + SourceExpr, IgnoreTypeAliases, IgnoreImplicitCasts)) return; const auto *CastExpr = Result.Nodes.getNodeAs<ExplicitCastExpr>("cast"); diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h index c09767d0cda2b..25bfb4410a27f 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h @@ -31,6 +31,7 @@ class RedundantCastingCheck : public ClangTidyCheck { private: const bool IgnoreMacros; const bool IgnoreTypeAliases; + const bool IgnoreImplicitCasts; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 88ac25c127759..a15cdb25caecf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -579,6 +579,14 @@ Changes in existing checks - Fixed a false positive in array subscript expressions where the types are not yet resolved. +- Improved :doc:`readability-redundant-casting + <clang-tidy/checks/readability/redundant-casting>` check by adding the + `IgnoreImplicitCasts` option (default `false`) to flag casts as redundant + when at least one operand of a binary operation matches the cast type due to + implicit conversion. For example, ``static_cast<float>(1.0f + 1)`` is now + identified as redundant since ``1`` is implicitly converted to ``float``. + Setting this option to `true` restores the previous behavior. + - Improved :doc:`readability-redundant-member-init <clang-tidy/checks/readability/redundant-member-init>` check by adding an `IgnoreMacros` option to suppress warnings when the initializer involves diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst index 4b6c14ba1cf87..cd9ef09bcaf81 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst @@ -27,11 +27,24 @@ Options .. option:: IgnoreMacros - If set to `true`, the check will not give warnings inside macros. Default - is `true`. + If set to `true`, the check will not give warnings inside macros. Default + is `true`. .. option:: IgnoreTypeAliases - When set to `false`, the check will consider type aliases, and when set to - `true`, it will resolve all type aliases and operate on the underlying - types. Default is `false`. + When set to `false`, the check will consider type aliases, and when set to + `true`, it will resolve all type aliases and operate on the underlying types. + Default is `false`. + +.. option:: IgnoreImplicitCasts + + When set to `false`, the check will flag casts as redundant when atleast one + operand in an expression is implicitly cast to match the result type of the + explicit cast. When set to `true` the casts will not be flagged. Default is + `false`. + + For example, with `IgnoreImplicitCasts = false`: + + .. code-block:: c++ + + static_cast<float>(2.0f + 1); // redundant (1 implicitly converts to float) diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp index 001057aeaa495..13be4192f49d1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp @@ -5,6 +5,9 @@ // RUN: %check_clang_tidy -std=c++11,c++14,c++17 -check-suffix=,ALIASES %s readability-redundant-casting %t -- \ // RUN: -config='{CheckOptions: { readability-redundant-casting.IgnoreTypeAliases: true }}' \ // RUN: -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -std=c++11,c++14,c++17 -check-suffix=IMPLICIT %s readability-redundant-casting %t -- \ +// RUN: -config='{CheckOptions: { readability-redundant-casting.IgnoreImplicitCasts: true }}' \ +// RUN: -- -fno-delayed-template-parsing // RUN: %check_clang_tidy -std=c++20-or-later %s readability-redundant-casting %t -- \ // RUN: -- -fno-delayed-template-parsing -D CXX_20=1 // RUN: %check_clang_tidy -std=c++20-or-later -check-suffix=,MACROS %s readability-redundant-casting %t -- \ @@ -13,6 +16,12 @@ // RUN: %check_clang_tidy -std=c++20-or-later -check-suffix=,ALIASES %s readability-redundant-casting %t -- \ // RUN: -config='{CheckOptions: { readability-redundant-casting.IgnoreTypeAliases: true }}' \ // RUN: -- -fno-delayed-template-parsing -D CXX_20=1 +// RUN: %check_clang_tidy -std=c++20-or-later -check-suffix=IMPLICIT %s readability-redundant-casting %t -- \ +// RUN: -config='{CheckOptions: { readability-redundant-casting.IgnoreImplicitCasts: true }}' \ +// RUN: -- -fno-delayed-template-parsing -D CXX_20=1 + +#include <cstdint> +#include <cstddef> struct A {}; struct B : A {}; @@ -168,6 +177,38 @@ void testFunctionalCastWithInitExpr(unsigned a) { unsigned c = unsigned{0}; } +void testBinaryOperatorRedundantCasting() { + const auto diff _types_operands1 { static_cast<float>(1.0f + 1) }; + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: const auto diff _types_operands1 { (1.0f + 1) }; + // CHECK-FIXES-IMPLICIT: const auto diff _types_operands1 { static_cast<float>(1.0f + 1) }; + + const auto diff _types_operands2 { static_cast<float>(2 + 3.0f) }; + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: const auto diff _types_operands2 { (2 + 3.0f) }; + // CHECK-FIXES-IMPLICIT: const auto diff _types_operands2 { static_cast<float>(2 + 3.0f) }; + + const auto diff _types_operands3 { static_cast<int>(1 + static_cast<uint8_t>(1)) }; + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: const auto diff _types_operands3 { (1 + static_cast<uint8_t>(1)) }; + // CHECK-FIXES-IMPLICIT: const auto diff _types_operands3 { static_cast<int>(1 + static_cast<uint8_t>(1)) }; + + const auto diff _types_operands4 { + static_cast<size_t>(static_cast<size_t>(3) + 2) + }; + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: redundant explicit casting to the same type 'size_t' (aka 'unsigned long{{( long)?}}') as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: (static_cast<size_t>(3) + 2) + // CHECK-FIXES-IMPLICIT: static_cast<size_t>(static_cast<size_t>(3) + 2) + + const auto diff _types_operands5 { unsigned(7 + unsigned(4)) }; + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant explicit casting to the same type 'unsigned int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: const auto diff _types_operands5 { (7 + unsigned(4)) }; + // CHECK-FIXES-IMPLICIT: const auto diff _types_operands5 { unsigned(7 + unsigned(4)) }; + + // casting isn't redundant here + const auto diff _types_operands6 { (int)(-7 + unsigned(4)) }; +} + void testBinaryOperator(char c) { int a = int(c - 'C'); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
