https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/70595
>From ff65fcca6047b0eebf73a87285dcff5f641a8ef0 Mon Sep 17 00:00:00 2001 From: Piotr Zegar <piotr.ze...@nokia.com> Date: Tue, 10 Oct 2023 20:37:05 +0000 Subject: [PATCH 1/5] [clang-tidy] Add readability-redundant-casting check Detects explicit type casting operations that involve the same source and destination types, and subsequently recommend their removal. Covers a range of explicit casting operations. Its primary objective is to enhance code readability and maintainability by eliminating unnecessary type casting. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../readability/RedundantCastingCheck.cpp | 242 ++++++++++++++++++ .../readability/RedundantCastingCheck.h | 38 +++ .../clang-tidy/utils/FixItHintUtils.h | 1 + clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/redundant-casting.rst | 37 +++ .../readability/redundant-casting.cpp | 200 +++++++++++++++ 9 files changed, 529 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index fa571d5dd7650d..636860f54c73cc 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -35,6 +35,7 @@ add_clang_library(clangTidyReadabilityModule QualifiedAutoCheck.cpp ReadabilityTidyModule.cpp RedundantAccessSpecifiersCheck.cpp + RedundantCastingCheck.cpp RedundantControlFlowCheck.cpp RedundantDeclarationCheck.cpp RedundantFunctionPtrDereferenceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index f769752c5de5fa..2f5d47546197f7 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -38,6 +38,7 @@ #include "OperatorsRepresentationCheck.h" #include "QualifiedAutoCheck.h" #include "RedundantAccessSpecifiersCheck.h" +#include "RedundantCastingCheck.h" #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" @@ -117,6 +118,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-qualified-auto"); CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>( "readability-redundant-access-specifiers"); + CheckFactories.registerCheck<RedundantCastingCheck>( + "readability-redundant-casting"); CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>( "readability-redundant-function-ptr-dereference"); CheckFactories.registerCheck<RedundantMemberInitCheck>( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp new file mode 100644 index 00000000000000..a2827d8e73e816 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp @@ -0,0 +1,242 @@ +//===--- RedundantCastingCheck.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 "RedundantCastingCheck.h" +#include "../utils/FixItHintUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool areTypesEquals(QualType S, QualType D) { + if (S == D) + return true; + + const auto *TS = S->getAs<TypedefType>(); + const auto *TD = D->getAs<TypedefType>(); + if (TS != TD) + return false; + + QualType PtrS = S->getPointeeType(); + QualType PtrD = D->getPointeeType(); + + if (!PtrS.isNull() && !PtrD.isNull()) + return areTypesEquals(PtrS, PtrD); + + const DeducedType *DT = S->getContainedDeducedType(); + if (DT && DT->isDeduced()) + return D == DT->getDeducedType(); + + return false; +} + +static bool areTypesEquals(QualType TypeS, QualType TypeD, + bool IgnoreTypeAliases) { + const QualType CTypeS = TypeS.getCanonicalType(); + const QualType CTypeD = TypeD.getCanonicalType(); + if (CTypeS != CTypeD) + return false; + + return IgnoreTypeAliases || areTypesEquals(TypeS.getLocalUnqualifiedType(), + TypeD.getLocalUnqualifiedType()); +} + +static bool areBinaryOperatorOperandsTypesEqual(const Expr *E, + bool IgnoreTypeAliases) { + if (!E) + return false; + const Expr *WithoutImplicitAndParen = E->IgnoreParenImpCasts(); + if (!WithoutImplicitAndParen) + return false; + if (const auto *B = dyn_cast<BinaryOperator>(WithoutImplicitAndParen)) { + const QualType NonReferenceType = + WithoutImplicitAndParen->getType().getNonReferenceType(); + if (!areTypesEquals( + B->getLHS()->IgnoreImplicit()->getType().getNonReferenceType(), + NonReferenceType, IgnoreTypeAliases)) + return true; + if (!areTypesEquals( + B->getRHS()->IgnoreImplicit()->getType().getNonReferenceType(), + NonReferenceType, IgnoreTypeAliases)) + return true; + } + return false; +} + +static const Decl *getSourceExprDecl(const Expr *SourceExpr) { + const Expr *CleanSourceExpr = SourceExpr->IgnoreParenImpCasts(); + if (const auto *E = dyn_cast<DeclRefExpr>(CleanSourceExpr)) { + return E->getDecl(); + } + + if (const auto *E = dyn_cast<CallExpr>(CleanSourceExpr)) { + return E->getCalleeDecl(); + } + + if (const auto *E = dyn_cast<MemberExpr>(CleanSourceExpr)) { + return E->getMemberDecl(); + } + return nullptr; +} + +RedundantCastingCheck::RedundantCastingCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreTypeAliases(Options.getLocalOrGlobal("IgnoreTypeAliases", false)) {} + +void RedundantCastingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); + Options.store(Opts, "IgnoreTypeAliases", IgnoreTypeAliases); +} + +void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { + + auto SimpleType = qualType(hasCanonicalType( + qualType(anyOf(builtinType(), references(builtinType()), + references(pointsTo(qualType())), pointsTo(qualType()))))); + + auto BitfieldMemberExpr = memberExpr(member(fieldDecl(isBitField()))); + + Finder->addMatcher( + explicitCastExpr( + unless(hasCastKind(CK_ConstructorConversion)), + unless(hasCastKind(CK_UserDefinedConversion)), + unless(cxxFunctionalCastExpr(hasDestinationType(unless(SimpleType)))), + + hasDestinationType(qualType().bind("type2")), + hasSourceExpression(anyOf( + expr(unless(initListExpr()), unless(BitfieldMemberExpr), + hasType(qualType().bind("type1"))) + .bind("source"), + initListExpr(unless(hasInit(1, expr())), + hasInit(0, expr(unless(BitfieldMemberExpr), + hasType(qualType().bind("type1"))) + .bind("source")))))) + .bind("cast"), + this); +} + +void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { + const auto *SourceExpr = Result.Nodes.getNodeAs<Expr>("source"); + auto TypeD = *Result.Nodes.getNodeAs<QualType>("type2"); + + if (SourceExpr->getValueKind() == VK_LValue && + TypeD.getCanonicalType()->isRValueReferenceType()) + return; + + const auto TypeS = + Result.Nodes.getNodeAs<QualType>("type1")->getNonReferenceType(); + TypeD = TypeD.getNonReferenceType(); + + if (!areTypesEquals(TypeS, TypeD, IgnoreTypeAliases)) + return; + + if (areBinaryOperatorOperandsTypesEqual(SourceExpr, IgnoreTypeAliases)) + return; + + const auto *CastExpr = Result.Nodes.getNodeAs<ExplicitCastExpr>("cast"); + if (IgnoreMacros && + (CastExpr->getBeginLoc().isMacroID() || + CastExpr->getEndLoc().isMacroID() || CastExpr->getExprLoc().isMacroID())) + return; + + { + auto Diag = diag(CastExpr->getExprLoc(), + "redundant explicit casting to the same type %0 as the " + "sub-expression, remove this casting"); + Diag << TypeD; + + const SourceManager &SM = *Result.SourceManager; + const SourceLocation SourceExprBegin = + SM.getExpansionLoc(SourceExpr->getBeginLoc()); + const SourceLocation SourceExprEnd = + SM.getExpansionLoc(SourceExpr->getEndLoc()); + + if (SourceExprBegin != CastExpr->getBeginLoc()) + Diag << FixItHint::CreateRemoval(SourceRange( + CastExpr->getBeginLoc(), SourceExprBegin.getLocWithOffset(-1))); + + const SourceLocation NextToken = Lexer::getLocForEndOfToken( + SourceExprEnd, 0U, SM, Result.Context->getLangOpts()); + + if (SourceExprEnd != CastExpr->getEndLoc()) { + Diag << FixItHint::CreateRemoval( + SourceRange(NextToken, CastExpr->getEndLoc())); + } + + if (utils::fixit::areParensNeededForStatement(*SourceExpr)) { + + Diag << FixItHint::CreateInsertion(SourceExprBegin, "(") + << FixItHint::CreateInsertion(NextToken, ")"); + } + } + + const auto *SourceExprDecl = getSourceExprDecl(SourceExpr); + if (!SourceExprDecl) + return; + + if (const auto *D = dyn_cast<CXXConstructorDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from the invocation of this constructor", + DiagnosticIDs::Note); + return; + } + + if (const auto *D = dyn_cast<FunctionDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from the invocation of this " + "%select{function|method}0", + DiagnosticIDs::Note) + << isa<CXXMethodDecl>(D) << D->getReturnTypeSourceRange(); + return; + } + + if (const auto *D = dyn_cast<FieldDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from referencing this member", + DiagnosticIDs::Note) + << SourceRange(D->getTypeSpecStartLoc(), D->getTypeSpecEndLoc()); + return; + } + + if (const auto *D = dyn_cast<ParmVarDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from referencing this parameter", + DiagnosticIDs::Note) + << SourceRange(D->getTypeSpecStartLoc(), D->getTypeSpecEndLoc()); + return; + } + + if (const auto *D = dyn_cast<VarDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from referencing this variable", + DiagnosticIDs::Note) + << SourceRange(D->getTypeSpecStartLoc(), D->getTypeSpecEndLoc()); + return; + } + + if (const auto *D = dyn_cast<EnumConstantDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from referencing this enum constant", + DiagnosticIDs::Note); + return; + } + + if (const auto *D = dyn_cast<BindingDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from referencing this bound variable", + DiagnosticIDs::Note); + return; + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h new file mode 100644 index 00000000000000..fdcfede05d4369 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.h @@ -0,0 +1,38 @@ +//===--- RedundantCastingCheck.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_READABILITY_REDUNDANTCASTINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTCASTINGCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Detects explicit type casting operations that involve the same source and +/// destination types, and subsequently recommend their removal. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-casting.html +class RedundantCastingCheck : public ClangTidyCheck { +public: + RedundantCastingCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + const bool IgnoreMacros; + const bool IgnoreTypeAliases; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTCASTINGCHECK_H diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h index d25bea2c6d2973..2b96b2b2ce600c 100644 --- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h +++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h @@ -51,6 +51,7 @@ std::string formatDereference(const Expr &ExprNode, const ASTContext &Context); // \brief Checks whatever a expression require extra () to be always used in // safe way in any other expression. bool areParensNeededForStatement(const Stmt &Node); + } // namespace clang::tidy::utils::fixit #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d77267588db915..36687dcf0b4ed4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -235,6 +235,12 @@ New checks Finds return statements with ``void`` values used within functions with ``void`` result types. +- New :doc:`readability-redundant-casting + <clang-tidy/checks/readability/redundant-casting>` check. + + Detects explicit type casting operations that involve the same source and + destination types, and subsequently recommend their removal. + - New :doc:`readability-reference-to-constructed-temporary <clang-tidy/checks/readability/reference-to-constructed-temporary>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5f21449cfc3df4..503b9fac827320 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -365,6 +365,7 @@ Clang-Tidy Checks :doc:`readability-operators-representation <readability/operators-representation>`, "Yes" :doc:`readability-qualified-auto <readability/qualified-auto>`, "Yes" :doc:`readability-redundant-access-specifiers <readability/redundant-access-specifiers>`, "Yes" + :doc:`readability-redundant-casting <readability/redundant-casting>`, "Yes" :doc:`readability-redundant-control-flow <readability/redundant-control-flow>`, "Yes" :doc:`readability-redundant-declaration <readability/redundant-declaration>`, "Yes" :doc:`readability-redundant-function-ptr-dereference <readability/redundant-function-ptr-dereference>`, "Yes" 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 new file mode 100644 index 00000000000000..23eaa225f03a3c --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-casting.rst @@ -0,0 +1,37 @@ +.. title:: clang-tidy - readability-redundant-casting + +readability-redundant-casting +============================= + +Detects explicit type casting operations that involve the same source and +destination types, and subsequently recommend their removal. Covers a range of +explicit casting operations, including ``static_cast``, ``const_cast``, C-style +casts, and ``reinterpret_cast``. Its primary objective is to enhance code +readability and maintainability by eliminating unnecessary type casting. + +.. code-block:: c++ + + int value = 42; + int result = static_cast<int>(value); + +In this example, the ``static_cast<int>(value)`` is redundant, as it performs +a cast from an ``int`` to another ``int``. + +Casting operations involving constructor conversions, user-defined conversions, +functional casts, type-dependent casts, casts between distinct type aliases that +refer to the same underlying type, as well as bitfield-related casts and casts +directly from lvalue to rvalue, are all disregarded by the check. + +Options +------- + +.. option:: IgnoreMacros + + 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`. 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 new file mode 100644 index 00000000000000..7dca6921298d38 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp @@ -0,0 +1,200 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s readability-redundant-casting %t +// RUN: %check_clang_tidy -std=c++11-or-later -check-suffix=,MACROS %s readability-redundant-casting %t -- \ +// RUN: -config='{CheckOptions: { readability-redundant-casting.IgnoreMacros: false }}' +// RUN: %check_clang_tidy -std=c++11-or-later -check-suffix=,ALIASES %s readability-redundant-casting %t -- \ +// RUN: -config='{CheckOptions: { readability-redundant-casting.IgnoreTypeAliases: true }}' + +struct A {}; +struct B : A {}; +A getA(); + +void testRedundantStaticCasting(A& value) { + A& a1 = static_cast<A&>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:36: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} A& a1 = value; +} + +void testRedundantConstCasting1(A& value) { + A& a2 = const_cast<A&>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:36: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} A& a2 = value; +} + +void testRedundantConstCasting2(const A& value) { + const A& a3 = const_cast<const A&>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant explicit casting to the same type 'const A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:42: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} const A& a3 = value; +} + +void testRedundantReinterpretCasting(A& value) { + A& a4 = reinterpret_cast<A&>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:41: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} A& a4 = value; +} + +void testRedundantCCasting(A& value) { + A& a5 = (A&)(value); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:31: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} A& a5 = value; +} + +void testDoubleCasting(A& value) { + A& a6 = static_cast<A&>(reinterpret_cast<A&>(value)); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-4]]:27: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} A& a6 = value; +} + +void testDiffrentTypesCast(B& value) { + A& a7 = static_cast<A&>(value); +} + +void testCastingWithAuto() { + auto a = getA(); + A& a8 = static_cast<A&>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:8: note: source type originates from referencing this variable + // CHECK-FIXES: {{^}} A& a8 = a; +} + +void testCastingWithConstAuto() { + const auto a = getA(); + const A& a9 = static_cast<const A&>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant explicit casting to the same type 'const A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:14: note: source type originates from referencing this variable + // CHECK-FIXES: {{^}} const A& a9 = a; +} + +void testCastingWithAutoPtr(A& ptr) { + auto* a = &ptr; + A* a10 = static_cast<A*>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant explicit casting to the same type 'A *' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: source type originates from referencing this variable + // CHECK-FIXES: {{^}} A* a10 = a; +} + +template<typename T> +void testRedundantTemplateCasting(T& value) { + A& a = static_cast<A&>(value); + T& t = static_cast<T&>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: redundant explicit casting to the same type 'T' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-4]]:38: note: source type originates from referencing this parameter + // CHECK-FIXES: {{^}} T& t = value; +} + +void testTemplate() { + A value; + testRedundantTemplateCasting(value); +} + +void testValidRefConstCast() { + const auto a = getA(); + A& a11 = const_cast<A&>(a); +} + +void testValidPtrConstCast(const A* ptr) { + A* a12 = const_cast<A*>(ptr); +} + +#define CAST(X) static_cast<int>(X) + +void testMacroCasting(int value) { + int a = CAST(value); + // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] +} + +#define PTR_NAME name + +void testMacroCasting(A* PTR_NAME) { + A* a13 = static_cast<A*>(PTR_NAME); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant explicit casting to the same type 'A *' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: {{^}} A* a13 = PTR_NAME; +} + +struct CastBool { + operator bool() const { + return true; + } +}; + +void testUserOperatorCast(const CastBool& value) { + bool b = static_cast<bool>(value); +} + +using TypeA = A; + +void testTypedefCast(A& value) { + TypeA& a = static_cast<TypeA&>(value); + // CHECK-MESSAGES-ALIASES: :[[@LINE-1]]:14: warning: redundant explicit casting to the same type 'TypeA' (aka 'A') as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES-ALIASES: {{^}} TypeA& a = value; +} + +void testTypedefCast2(TypeA& value) { + A& a = static_cast<A&>(value); + // CHECK-MESSAGES-ALIASES: :[[@LINE-1]]:10: warning: redundant explicit casting to the same type 'A' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES-ALIASES: {{^}} A& a = value; +} + +void testFunctionalCastWithPrimitive(int a) { + int b = int(a); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: {{^}} int b = a; +} + +void testFunctionalCastWithInitExpr(unsigned a) { + unsigned b = ~unsigned{!a}; + unsigned c = unsigned{0}; +} + +void testBinaryOperator(char c) { + int a = int(c - 'C'); +} + +struct BIT { + bool b:1; +}; + +template<typename ...Args> +void make(Args&& ...); + +void testBinaryOperator(BIT b) { + make((bool)b.b); +} + +struct Class { + using Iterator = const char*; + + Iterator begin() { + return static_cast<Iterator>(first()); +// CHECK-MESSAGES-ALIASES: :[[@LINE-1]]:12: warning: redundant explicit casting to the same type 'Iterator' (aka 'const char *') as the sub-expression, remove this casting [readability-redundant-casting] +// CHECK-MESSAGES-ALIASES: :[[@LINE+4]]:15: note: source type originates from the invocation of this method +// CHECK-FIXES-ALIASES: {{^}} return first(); + } + + const char* first(); +}; + +void testAddOperation(int aa, int bb) { + int c = static_cast<int>(aa + bb) * aa; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: {{^}} int c = (aa + bb) * aa; +} + +void testAddOperationWithParen(int a, int b) { + int c = static_cast<int>((a+b))*a; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: {{^}} int c = (a+b)*a; +} + +void testRValueCast(int&& a) { + int&& b = static_cast<int&&>(a); + int&& c = static_cast<int&&>(10); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-FIXES: {{^}} int&& c = 10; +} >From 4e28bb44d0c4b7daa1785afe614f5d9b821b810f Mon Sep 17 00:00:00 2001 From: Piotr Zegar <piotr.ze...@nokia.com> Date: Tue, 16 Jan 2024 17:37:36 +0000 Subject: [PATCH 2/5] Change type1, type2 to srcType, dstType --- .../clang-tidy/readability/RedundantCastingCheck.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp index a2827d8e73e816..ce75e6c3a0a6b2 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp @@ -112,14 +112,14 @@ void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { unless(hasCastKind(CK_UserDefinedConversion)), unless(cxxFunctionalCastExpr(hasDestinationType(unless(SimpleType)))), - hasDestinationType(qualType().bind("type2")), + hasDestinationType(qualType().bind("dstType")), hasSourceExpression(anyOf( expr(unless(initListExpr()), unless(BitfieldMemberExpr), - hasType(qualType().bind("type1"))) + hasType(qualType().bind("srcType"))) .bind("source"), initListExpr(unless(hasInit(1, expr())), hasInit(0, expr(unless(BitfieldMemberExpr), - hasType(qualType().bind("type1"))) + hasType(qualType().bind("srcType"))) .bind("source")))))) .bind("cast"), this); @@ -127,14 +127,14 @@ void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { const auto *SourceExpr = Result.Nodes.getNodeAs<Expr>("source"); - auto TypeD = *Result.Nodes.getNodeAs<QualType>("type2"); + auto TypeD = *Result.Nodes.getNodeAs<QualType>("dstType"); if (SourceExpr->getValueKind() == VK_LValue && TypeD.getCanonicalType()->isRValueReferenceType()) return; const auto TypeS = - Result.Nodes.getNodeAs<QualType>("type1")->getNonReferenceType(); + Result.Nodes.getNodeAs<QualType>("srcType")->getNonReferenceType(); TypeD = TypeD.getNonReferenceType(); if (!areTypesEquals(TypeS, TypeD, IgnoreTypeAliases)) >From d3c974a9eeb095866d786fddbed9660434c05ce7 Mon Sep 17 00:00:00 2001 From: Piotr Zegar <piotr.ze...@nokia.com> Date: Tue, 16 Jan 2024 17:53:55 +0000 Subject: [PATCH 3/5] Add checks for isNull --- .../readability/RedundantCastingCheck.cpp | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp index ce75e6c3a0a6b2..2a77fbd509dd34 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp @@ -57,15 +57,20 @@ static bool areBinaryOperatorOperandsTypesEqual(const Expr *E, if (!WithoutImplicitAndParen) return false; if (const auto *B = dyn_cast<BinaryOperator>(WithoutImplicitAndParen)) { - const QualType NonReferenceType = - WithoutImplicitAndParen->getType().getNonReferenceType(); - if (!areTypesEquals( - B->getLHS()->IgnoreImplicit()->getType().getNonReferenceType(), - NonReferenceType, IgnoreTypeAliases)) + const QualType Type = WithoutImplicitAndParen->getType(); + if (Type.isNull()) + return false; + + const QualType NonReferenceType = Type.getNonReferenceType(); + const QualType LHSType = B->getLHS()->IgnoreImplicit()->getType(); + if (!LHSType.isNull() && + !areTypesEquals(LHSType.getNonReferenceType(), NonReferenceType, + IgnoreTypeAliases)) return true; - if (!areTypesEquals( - B->getRHS()->IgnoreImplicit()->getType().getNonReferenceType(), - NonReferenceType, IgnoreTypeAliases)) + const QualType RHSType = B->getRHS()->IgnoreImplicit()->getType(); + if (!RHSType.isNull() && + !areTypesEquals(RHSType.getNonReferenceType(), NonReferenceType, + IgnoreTypeAliases)) return true; } return false; >From 03b3c936d979eae4532c963688320ef3a6670bd8 Mon Sep 17 00:00:00 2001 From: Piotr Zegar <piotr.ze...@nokia.com> Date: Tue, 16 Jan 2024 18:06:39 +0000 Subject: [PATCH 4/5] Add tests and notes for NonTypeTemplateParmDecl --- .../readability/RedundantCastingCheck.cpp | 8 +++++++ .../readability/redundant-casting.cpp | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp index 2a77fbd509dd34..4023563f1a8aa4 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp @@ -242,6 +242,14 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { DiagnosticIDs::Note); return; } + + if (const auto *D = dyn_cast<NonTypeTemplateParmDecl>(SourceExprDecl)) { + diag(D->getLocation(), + "source type originates from referencing this non-type template " + "parameter", + DiagnosticIDs::Note); + return; + } } } // namespace clang::tidy::readability 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 7dca6921298d38..9aed05bdf9b948 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 @@ -198,3 +198,24 @@ void testRValueCast(int&& a) { // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] // CHECK-FIXES: {{^}} int&& c = 10; } + +template <int V> +void testRedundantNTTPCasting() { + int a = static_cast<int>(V); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant explicit casting to the same type 'int' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-4]]:15: note: source type originates from referencing this non-type template parameter + // CHECK-FIXES: {{^}} int a = V; +} + +template <typename T, T V> +void testValidNTTPCasting() { + int a = static_cast<int>(V); +} + +template <typename T, T V> +void testRedundantDependentNTTPCasting() { + T a = static_cast<T>(V); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant explicit casting to the same type 'T' as the sub-expression, remove this casting [readability-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-4]]:25: note: source type originates from referencing this non-type template parameter + // CHECK-FIXES: {{^}} T a = V; +} >From fab960f125882b949dfe9b16907bd8efdcf81682 Mon Sep 17 00:00:00 2001 From: Piotr Zegar <piotr.ze...@nokia.com> Date: Tue, 16 Jan 2024 18:10:20 +0000 Subject: [PATCH 5/5] Micro fixes --- .../readability/RedundantCastingCheck.cpp | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp index 4023563f1a8aa4..00e0c8160493d1 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantCastingCheck.cpp @@ -16,7 +16,7 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { -static bool areTypesEquals(QualType S, QualType D) { +static bool areTypesEqual(QualType S, QualType D) { if (S == D) return true; @@ -29,7 +29,7 @@ static bool areTypesEquals(QualType S, QualType D) { QualType PtrD = D->getPointeeType(); if (!PtrS.isNull() && !PtrD.isNull()) - return areTypesEquals(PtrS, PtrD); + return areTypesEqual(PtrS, PtrD); const DeducedType *DT = S->getContainedDeducedType(); if (DT && DT->isDeduced()) @@ -38,15 +38,15 @@ static bool areTypesEquals(QualType S, QualType D) { return false; } -static bool areTypesEquals(QualType TypeS, QualType TypeD, - bool IgnoreTypeAliases) { +static bool areTypesEqual(QualType TypeS, QualType TypeD, + bool IgnoreTypeAliases) { const QualType CTypeS = TypeS.getCanonicalType(); const QualType CTypeD = TypeD.getCanonicalType(); if (CTypeS != CTypeD) return false; - return IgnoreTypeAliases || areTypesEquals(TypeS.getLocalUnqualifiedType(), - TypeD.getLocalUnqualifiedType()); + return IgnoreTypeAliases || areTypesEqual(TypeS.getLocalUnqualifiedType(), + TypeD.getLocalUnqualifiedType()); } static bool areBinaryOperatorOperandsTypesEqual(const Expr *E, @@ -64,13 +64,13 @@ static bool areBinaryOperatorOperandsTypesEqual(const Expr *E, const QualType NonReferenceType = Type.getNonReferenceType(); const QualType LHSType = B->getLHS()->IgnoreImplicit()->getType(); if (!LHSType.isNull() && - !areTypesEquals(LHSType.getNonReferenceType(), NonReferenceType, - IgnoreTypeAliases)) + !areTypesEqual(LHSType.getNonReferenceType(), NonReferenceType, + IgnoreTypeAliases)) return true; const QualType RHSType = B->getRHS()->IgnoreImplicit()->getType(); if (!RHSType.isNull() && - !areTypesEquals(RHSType.getNonReferenceType(), NonReferenceType, - IgnoreTypeAliases)) + !areTypesEqual(RHSType.getNonReferenceType(), NonReferenceType, + IgnoreTypeAliases)) return true; } return false; @@ -142,7 +142,7 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { Result.Nodes.getNodeAs<QualType>("srcType")->getNonReferenceType(); TypeD = TypeD.getNonReferenceType(); - if (!areTypesEquals(TypeS, TypeD, IgnoreTypeAliases)) + if (!areTypesEqual(TypeS, TypeD, IgnoreTypeAliases)) return; if (areBinaryOperatorOperandsTypesEqual(SourceExpr, IgnoreTypeAliases)) @@ -179,7 +179,6 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { } if (utils::fixit::areParensNeededForStatement(*SourceExpr)) { - Diag << FixItHint::CreateInsertion(SourceExprBegin, "(") << FixItHint::CreateInsertion(NextToken, ")"); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits