https://github.com/hnrklssn created https://github.com/llvm/llvm-project/pull/191081
Warns when performing a dynamic type check that is always true, either because the dynamic type is the same as the static type, or because the static type derives the dynamic type. Supported functions: - isa - isa_and_present - isa_and_nonnull Related PR: https://github.com/llvm/llvm-project/pull/189274 >From e14bec55272e815a36662d2354c21dd4985d854a Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <[email protected]> Date: Sun, 29 Mar 2026 12:48:15 -0700 Subject: [PATCH 1/4] [clang-tidy] detect uses of llvm::isa that are always true Warns when performing a dynamic type check that is always true, either because the dynamic type is the same as the static type, or because the static type derives the dynamic type. Supported functions: - isa - isa_and_present - isa_and_nonnull --- .../clang-tidy/llvm/RedundantCastingCheck.cpp | 101 +++++++------- clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../checks/llvm/redundant-casting.rst | 12 +- .../checkers/llvm/redundant-casting.cpp | 2 +- .../checkers/llvm/redundant-isa.cpp | 126 ++++++++++++++++++ 5 files changed, 194 insertions(+), 50 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp index 8f8c152b5de86..59808c539419d 100644 --- a/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp @@ -38,7 +38,8 @@ AST_MATCHER_P(OverloadExpr, hasAnyUnresolvedName, ArrayRef<StringRef>, Names) { static constexpr StringRef FunctionNames[] = { "cast", "cast_or_null", "cast_if_present", - "dyn_cast", "dyn_cast_or_null", "dyn_cast_if_present"}; + "dyn_cast", "dyn_cast_or_null", "dyn_cast_if_present", + "isa", "isa_and_nonnull", "isa_and_present"}; void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { auto IsInLLVMNamespace = hasDeclContext( @@ -93,12 +94,15 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { const auto &Nodes = Result.Nodes; const auto *Call = Nodes.getNodeAs<CallExpr>("call"); - CanQualType RetTy; + TemplateArgument TArg; std::string FuncName; if (const auto *ResolvedCallee = Nodes.getNodeAs<DeclRefExpr>("callee")) { const auto *F = cast<FunctionDecl>(ResolvedCallee->getDecl()); - RetTy = stripPointerOrReference(F->getReturnType()) - ->getCanonicalTypeUnqualified(); + const auto *TArgs = F->getTemplateSpecializationArgs(); + if (TArgs->size() != 2) + return; + + TArg = TArgs->get(0); FuncName = F->getName(); } else if (const auto *UnresolvedCallee = Nodes.getNodeAs<UnresolvedLookupExpr>("callee")) { @@ -107,16 +111,17 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { const auto *CallerNS = Nodes.getNodeAs<NamedDecl>("llvm_ns"); if (!IsExplicitlyLLVM && !CallerNS) return; - auto TArg = UnresolvedCallee->template_arguments()[0].getArgument(); - if (TArg.getKind() != TemplateArgument::Type) - return; - RetTy = TArg.getAsType()->getCanonicalTypeUnqualified(); + TArg = UnresolvedCallee->template_arguments()[0].getArgument(); FuncName = UnresolvedCallee->getName().getAsString(); } else { - llvm_unreachable(""); + llvm_unreachable("unexpected callee kind"); } + if (TArg.getKind() != TemplateArgument::Type) + return; + + CanQualType RetTy = TArg.getAsType()->getCanonicalTypeUnqualified(); const auto *Arg = Call->getArg(0); const QualType ArgTy = Arg->getType(); const QualType ArgPointeeTy = stripPointerOrReference(ArgTy); @@ -128,47 +133,53 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { if (FromTy != RetTy && !IsDerived) return; - QualType ParentTy; - if (const auto *ParentCast = Nodes.getNodeAs<Expr>("parent_cast")) { - ParentTy = ParentCast->getType(); + const bool IsIsa = StringRef(FuncName).starts_with("isa"); + if (IsIsa) { + diag(Call->getExprLoc(), "call to '%0' always succeeds") + << FuncName; } else { - // IgnoreUnlessSpelledInSource prevents matching implicit casts - const TraversalKindScope TmpTraversalKind(*Result.Context, TK_AsIs); - for (const DynTypedNode Parent : Result.Context->getParents(*Call)) { - if (const auto *ParentCastExpr = Parent.get<CastExpr>()) { - ParentTy = ParentCastExpr->getType(); - break; + QualType ParentTy; + if (const auto *ParentCast = Nodes.getNodeAs<Expr>("parent_cast")) { + ParentTy = ParentCast->getType(); + } else { + // IgnoreUnlessSpelledInSource prevents matching implicit casts + const TraversalKindScope TmpTraversalKind(*Result.Context, TK_AsIs); + for (const DynTypedNode Parent : Result.Context->getParents(*Call)) { + if (const auto *ParentCastExpr = Parent.get<CastExpr>()) { + ParentTy = ParentCastExpr->getType(); + break; + } } } - } - if (!ParentTy.isNull()) { - const CXXRecordDecl *ParentDecl = ParentTy->getAsCXXRecordDecl(); - if (FromDecl && ParentDecl) { - CXXBasePaths Paths(/*FindAmbiguities=*/true, - /*RecordPaths=*/false, - /*DetectVirtual=*/false); - const bool IsDerivedFromParent = - FromDecl && ParentDecl && FromDecl->isDerivedFrom(ParentDecl, Paths); - // For the following case a direct `cast<A>(d)` would be ambiguous: - // struct A {}; - // struct B : A {}; - // struct C : A {}; - // struct D : B, C {}; - // So we should not warn for `A *a = cast<C>(d)`. - if (IsDerivedFromParent && - Paths.isAmbiguous(ParentTy->getCanonicalTypeUnqualified())) - return; + if (!ParentTy.isNull()) { + const CXXRecordDecl *ParentDecl = ParentTy->getAsCXXRecordDecl(); + if (FromDecl && ParentDecl) { + CXXBasePaths Paths(/*FindAmbiguities=*/true, + /*RecordPaths=*/false, + /*DetectVirtual=*/false); + const bool IsDerivedFromParent = + FromDecl && ParentDecl && FromDecl->isDerivedFrom(ParentDecl, Paths); + // For the following case a direct `cast<A>(d)` would be ambiguous: + // struct A {}; + // struct B : A {}; + // struct C : A {}; + // struct D : B, C {}; + // So we should not warn for `A *a = cast<C>(d)`. + if (IsDerivedFromParent && + Paths.isAmbiguous(ParentTy->getCanonicalTypeUnqualified())) + return; + } } - } - auto GetText = [&](SourceRange R) { - return Lexer::getSourceText(CharSourceRange::getTokenRange(R), - *Result.SourceManager, getLangOpts()); - }; - StringRef ArgText = GetText(Arg->getSourceRange()); - diag(Call->getExprLoc(), "redundant use of '%0'") - << FuncName - << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText); + auto GetText = [&](SourceRange R) { + return Lexer::getSourceText(CharSourceRange::getTokenRange(R), + *Result.SourceManager, getLangOpts()); + }; + StringRef ArgText = GetText(Arg->getSourceRange()); + diag(Call->getExprLoc(), "redundant use of '%0'") + << FuncName + << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText); + } // printing the canonical type for a template parameter prints as e.g. // 'type-parameter-0-0' const QualType DiagFromTy(ArgPointeeTy->getUnqualifiedDesugaredType(), 0); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 78a7ff6ff7561..dcdede6229e8e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -130,7 +130,8 @@ New checks Points out uses of ``cast<>``, ``dyn_cast<>`` and their ``or_null`` variants that are unnecessary because the argument already is of the target type, or a - derived type thereof. + derived type thereof. Also does similar analysis for calls to ``isa<>`` that + always return ``true``. - New :doc:`llvm-type-switch-case-types <clang-tidy/checks/llvm/type-switch-case-types>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst index 84596b26ee382..cc2717f527847 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst @@ -3,9 +3,9 @@ llvm-redundant-casting ====================== -Points out uses of ``cast<>``, ``dyn_cast<>`` and their ``or_null`` variants -that are unnecessary because the argument already is of the target type, or a -derived type thereof. +Points out uses of ``cast<>``, ``dyn_cast<>``, ``isa<>`` and their ``or_null`` +variants that are unnecessary because the argument already is of the target +type, or a derived type thereof. .. code-block:: c++ @@ -16,6 +16,9 @@ derived type thereof. // replaced by: A x = a; + // Finds: + bool b = isa<A>(a) // always true + struct B : public A {}; B b; // Finds: @@ -30,3 +33,6 @@ Supported functions: - ``llvm::dyn_cast`` - ``llvm::dyn_cast_or_null`` - ``llvm::dyn_cast_if_present`` + - ``llvm::isa`` + - ``llvm::isa_and_nonnull`` + - ``llvm::isa_and_present`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-casting.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-casting.cpp index 933cbadc51701..5955c8a6870a6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-casting.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-casting.cpp @@ -24,7 +24,7 @@ CAST_FUNCTION(cast_or_null) CAST_FUNCTION(cast_if_present) CAST_FUNCTION(dyn_cast_or_null) CAST_FUNCTION(dyn_cast_if_present) -} +} // namespace llvm struct A {}; struct B : A {}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp new file mode 100644 index 0000000000000..9275f659b51a4 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp @@ -0,0 +1,126 @@ +// RUN: %check_clang_tidy -std=c++17 %s llvm-redundant-casting %t -- -- -fno-delayed-template-parsing + +namespace llvm { +#define ISA_FUNCTION(name) \ +template <typename To, typename From> \ +[[nodiscard]] inline bool name(const From &Val) { \ + return true; \ +} \ +template <typename To, typename From> \ +[[nodiscard]] inline bool name(const From *Val) { \ + return true; \ +} + +ISA_FUNCTION(isa) +ISA_FUNCTION(isa_and_present) +ISA_FUNCTION(isa_and_nonnull) +} // namespace llvm + +struct A {}; +struct B : A {}; +A &getA(); + +void testIsa(A& value) { + bool b1 = llvm::isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:26: note: source expression has type 'A' + (void)b1; +} + +void testIsaAndPresent(A& value) { + bool b2 = llvm::isa_and_present<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa_and_present' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:38: note: source expression has type 'A' + (void)b2; +} + +void testIsaAndNonnull(A& value) { + bool b3 = llvm::isa_and_nonnull<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa_and_nonnull' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:38: note: source expression has type 'A' + (void)b3; +} + +void testIsaNonDeclRef() { + bool b4 = llvm::isa<A>((getA())); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:26: note: source expression has type 'A' + (void)b4; +} + +void testUpcast(B& value) { + bool b5 = llvm::isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:26: note: source expression has type 'B', which is a subtype of 'A' + (void)b5; +} + +void testDowncast(A& value) { + bool b6 = llvm::isa<B>(value); + (void)b6; +} + +namespace llvm { +void testIsaInLLVM(A& value) { + bool b7 = isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:20: note: source expression has type 'A' + (void)b7; +} +} // namespace llvm + +void testIsaPointer(A* value) { + bool b8 = llvm::isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:26: note: source expression has pointee type 'A' + (void)b8; +} + +template <typename T> +void testIsaTemplateIgnore(T* value) { + bool b9 = llvm::isa<A>(value); + (void)b9; +} +template void testIsaTemplateIgnore<A>(A *value); + +template <typename T> +struct testIsaTemplateIgnoreStruct { + void method(T* value) { + bool b10 = llvm::isa<A>(value); + (void)b10; + } +}; + +void call(testIsaTemplateIgnoreStruct<A> s, A *a) { + s.method(a); +} + +template <typename T> +void testIsaTemplateTrigger1(T* value) { + bool b11 = llvm::isa<T>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:27: note: source expression has pointee type 'T' + (void)b11; +} + +template <typename T> +void testIsaTemplateTrigger2(A* value, T other) { + bool b12 = llvm::isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:27: note: source expression has pointee type 'A' + (void)b12; (void)other; +} + +void testIsaConst(const A& value) { + bool b13 = llvm::isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:27: note: source expression has type 'A' + (void)b13; +} + +void testIsaConstPointer(const A* value) { + bool b14 = llvm::isa<A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:27: note: source expression has pointee type 'A' + (void)b14; +} >From e3d5cf0cc145e5a8135daecc2bea8e77df187d0c Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <[email protected]> Date: Sun, 29 Mar 2026 16:20:48 -0700 Subject: [PATCH 2/4] [clang-tidy] support isa<> with template pack parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real `isa<>` implementation allows for a variable number of target types. This means that calls with even a single template parameter will still have a parameter pack, which would fail to be recognise with the naïve implemenation which would abort for any template argument that wasn't a plain type parameter. This generalises the recognised pattern to detect cases involving multiple target types. --- .../clang-tidy/llvm/RedundantCastingCheck.cpp | 192 +++++++++++------- .../checks/llvm/redundant-casting.rst | 10 +- .../checkers/llvm/redundant-isa.cpp | 26 ++- 3 files changed, 149 insertions(+), 79 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp index 59808c539419d..f5f05ba1f496b 100644 --- a/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp @@ -20,6 +20,8 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Lex/Lexer.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/Support/ErrorHandling.h" using namespace clang::ast_matchers; @@ -36,35 +38,58 @@ AST_MATCHER_P(OverloadExpr, hasAnyUnresolvedName, ArrayRef<StringRef>, Names) { } } // namespace -static constexpr StringRef FunctionNames[] = { +static constexpr StringRef CastFunctionNames[] = { "cast", "cast_or_null", "cast_if_present", - "dyn_cast", "dyn_cast_or_null", "dyn_cast_if_present", - "isa", "isa_and_nonnull", "isa_and_present"}; + "dyn_cast", "dyn_cast_or_null", "dyn_cast_if_present"}; + +static constexpr StringRef IsaFunctionNames[] = {"isa", "isa_and_nonnull", + "isa_and_present"}; void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { auto IsInLLVMNamespace = hasDeclContext( namespaceDecl(hasName("llvm"), hasDeclContext(translationUnitDecl()))); - auto AnyCalleeName = + auto AnyCastCalleeName = allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), callee(expr(ignoringImpCasts( - declRefExpr( - to(namedDecl(hasAnyName(FunctionNames), IsInLLVMNamespace)), - templateArgumentLocCountIs(1)) + declRefExpr(to(namedDecl(hasAnyName(CastFunctionNames), + IsInLLVMNamespace)), + templateArgumentLocCountIs(1)) .bind("callee"))))); - auto AnyCalleeNameInUninstantiatedTemplate = + auto AnyCastCalleeNameInUninstantiatedTemplate = allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), callee(expr(ignoringImpCasts( - unresolvedLookupExpr(hasAnyUnresolvedName(FunctionNames), + unresolvedLookupExpr(hasAnyUnresolvedName(CastFunctionNames), templateArgumentLocCountIs(1)) .bind("callee"))))); - Finder->addMatcher(callExpr(AnyCalleeName, argumentCountIs(1), - optionally(hasParent( - callExpr(AnyCalleeName).bind("parent_cast")))) + Finder->addMatcher( + callExpr(AnyCastCalleeName, argumentCountIs(1), + optionally( + hasParent(callExpr(AnyCastCalleeName).bind("parent_cast")))) + .bind("call"), + this); + + auto AnyIsaCalleeName = + allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), + callee(expr(ignoringImpCasts( + declRefExpr( + to(namedDecl(hasAnyName(IsaFunctionNames), IsInLLVMNamespace)), + hasAnyTemplateArgumentLoc(anything())) + .bind("callee"))))); + Finder->addMatcher(callExpr(AnyIsaCalleeName, argumentCountIs(1)) .bind("call"), this); + + auto AnyIsaCalleeNameInUninstantiatedTemplate = + allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), + callee(expr(ignoringImpCasts( + unresolvedLookupExpr(hasAnyUnresolvedName(IsaFunctionNames), + hasAnyTemplateArgumentLoc(anything())) + .bind("callee"))))); Finder->addMatcher( callExpr( - AnyCalleeNameInUninstantiatedTemplate, argumentCountIs(1), + anyOf(AnyCastCalleeNameInUninstantiatedTemplate, + AnyIsaCalleeNameInUninstantiatedTemplate), + argumentCountIs(1), optionally(hasAncestor( namespaceDecl(hasName("llvm"), hasParent(translationUnitDecl())) .bind("llvm_ns")))) @@ -94,7 +119,7 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { const auto &Nodes = Result.Nodes; const auto *Call = Nodes.getNodeAs<CallExpr>("call"); - TemplateArgument TArg; + llvm::SmallVector<CanQualType, 4> TargetTypes; std::string FuncName; if (const auto *ResolvedCallee = Nodes.getNodeAs<DeclRefExpr>("callee")) { const auto *F = cast<FunctionDecl>(ResolvedCallee->getDecl()); @@ -102,7 +127,20 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { if (TArgs->size() != 2) return; - TArg = TArgs->get(0); + TemplateArgument TArg = TArgs->get(0); + if (TArg.getKind() == TemplateArgument::Type) { + CanQualType TargetTy = TArg.getAsType()->getCanonicalTypeUnqualified(); + TargetTypes.emplace_back(TargetTy); + } else if (TArg.getKind() == TemplateArgument::Pack) { + for (const auto &E : TArg.pack_elements()) { + if (E.getKind() != TemplateArgument::Type) + return; + TargetTypes.emplace_back(E.getAsType()->getCanonicalTypeUnqualified()); + } + } else { + llvm_unreachable("unexpected template argument"); + } + FuncName = F->getName(); } else if (const auto *UnresolvedCallee = Nodes.getNodeAs<UnresolvedLookupExpr>("callee")) { @@ -112,83 +150,89 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { if (!IsExplicitlyLLVM && !CallerNS) return; - TArg = UnresolvedCallee->template_arguments()[0].getArgument(); + for (const auto &TArgLoc : UnresolvedCallee->template_arguments()) { + const TemplateArgument TArg = TArgLoc.getArgument(); + if (TArg.getKind() != TemplateArgument::Type) + return; + TargetTypes.emplace_back(TArg.getAsType()->getCanonicalTypeUnqualified()); + } FuncName = UnresolvedCallee->getName().getAsString(); } else { llvm_unreachable("unexpected callee kind"); } - if (TArg.getKind() != TemplateArgument::Type) - return; - - CanQualType RetTy = TArg.getAsType()->getCanonicalTypeUnqualified(); const auto *Arg = Call->getArg(0); const QualType ArgTy = Arg->getType(); const QualType ArgPointeeTy = stripPointerOrReference(ArgTy); const CanQualType FromTy = ArgPointeeTy->getCanonicalTypeUnqualified(); const auto *FromDecl = FromTy->getAsCXXRecordDecl(); - const auto *RetDecl = RetTy->getAsCXXRecordDecl(); - const bool IsDerived = - FromDecl && RetDecl && FromDecl->isDerivedFrom(RetDecl); - if (FromTy != RetTy && !IsDerived) + const bool IsIsa = StringRef(FuncName).starts_with("isa"); + if (!IsIsa && TargetTypes.size() != 1) return; - const bool IsIsa = StringRef(FuncName).starts_with("isa"); - if (IsIsa) { - diag(Call->getExprLoc(), "call to '%0' always succeeds") - << FuncName; - } else { - QualType ParentTy; - if (const auto *ParentCast = Nodes.getNodeAs<Expr>("parent_cast")) { - ParentTy = ParentCast->getType(); + for (CanQualType TargetTy : TargetTypes) { + const auto *RetDecl = TargetTy->getAsCXXRecordDecl(); + const bool IsDerived = FromDecl && RetDecl && FromDecl->isDerivedFrom(RetDecl); + if (FromTy != TargetTy && !IsDerived) + continue; + + if (IsIsa) { + diag(Call->getExprLoc(), "call to '%0' always succeeds") + << FuncName; } else { - // IgnoreUnlessSpelledInSource prevents matching implicit casts - const TraversalKindScope TmpTraversalKind(*Result.Context, TK_AsIs); - for (const DynTypedNode Parent : Result.Context->getParents(*Call)) { - if (const auto *ParentCastExpr = Parent.get<CastExpr>()) { - ParentTy = ParentCastExpr->getType(); - break; + QualType ParentTy; + if (const auto *ParentCast = Nodes.getNodeAs<Expr>("parent_cast")) { + ParentTy = ParentCast->getType(); + } else { + // IgnoreUnlessSpelledInSource prevents matching implicit casts + const TraversalKindScope TmpTraversalKind(*Result.Context, TK_AsIs); + for (const DynTypedNode Parent : Result.Context->getParents(*Call)) { + if (const auto *ParentCastExpr = Parent.get<CastExpr>()) { + ParentTy = ParentCastExpr->getType(); + break; + } } } - } - if (!ParentTy.isNull()) { - const CXXRecordDecl *ParentDecl = ParentTy->getAsCXXRecordDecl(); - if (FromDecl && ParentDecl) { - CXXBasePaths Paths(/*FindAmbiguities=*/true, - /*RecordPaths=*/false, - /*DetectVirtual=*/false); - const bool IsDerivedFromParent = - FromDecl && ParentDecl && FromDecl->isDerivedFrom(ParentDecl, Paths); - // For the following case a direct `cast<A>(d)` would be ambiguous: - // struct A {}; - // struct B : A {}; - // struct C : A {}; - // struct D : B, C {}; - // So we should not warn for `A *a = cast<C>(d)`. - if (IsDerivedFromParent && - Paths.isAmbiguous(ParentTy->getCanonicalTypeUnqualified())) - return; + if (!ParentTy.isNull()) { + const CXXRecordDecl *ParentDecl = ParentTy->getAsCXXRecordDecl(); + if (FromDecl && ParentDecl) { + CXXBasePaths Paths(/*FindAmbiguities=*/true, + /*RecordPaths=*/false, + /*DetectVirtual=*/false); + const bool IsDerivedFromParent = + FromDecl && ParentDecl && FromDecl->isDerivedFrom(ParentDecl, Paths); + // For the following case a direct `cast<A>(d)` would be ambiguous: + // struct A {}; + // struct B : A {}; + // struct C : A {}; + // struct D : B, C {}; + // So we should not warn for `A *a = cast<C>(d)`. + if (IsDerivedFromParent && + Paths.isAmbiguous(ParentTy->getCanonicalTypeUnqualified())) + return; + } } - } - auto GetText = [&](SourceRange R) { - return Lexer::getSourceText(CharSourceRange::getTokenRange(R), - *Result.SourceManager, getLangOpts()); - }; - StringRef ArgText = GetText(Arg->getSourceRange()); - diag(Call->getExprLoc(), "redundant use of '%0'") - << FuncName - << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText); + auto GetText = [&](SourceRange R) { + return Lexer::getSourceText(CharSourceRange::getTokenRange(R), + *Result.SourceManager, getLangOpts()); + }; + StringRef ArgText = GetText(Arg->getSourceRange()); + diag(Call->getExprLoc(), "redundant use of '%0'") + << FuncName + << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText); + } + // printing the canonical type for a template parameter prints as e.g. + // 'type-parameter-0-0' + const QualType DiagFromTy(ArgPointeeTy->getUnqualifiedDesugaredType(), 0); + diag(Arg->getExprLoc(), + "source expression has%select{| pointee}0 type %1%select{|, which is a " + "subtype of %3}2", + DiagnosticIDs::Note) + << Arg->getSourceRange() << ArgTy->isPointerType() << DiagFromTy + << (FromTy != TargetTy) << TargetTy; + return; } - // printing the canonical type for a template parameter prints as e.g. - // 'type-parameter-0-0' - const QualType DiagFromTy(ArgPointeeTy->getUnqualifiedDesugaredType(), 0); - diag(Arg->getExprLoc(), - "source expression has%select{| pointee}0 type %1%select{|, which is a " - "subtype of %3}2", - DiagnosticIDs::Note) - << Arg->getSourceRange() << ArgTy->isPointerType() << DiagFromTy - << (FromTy != RetTy) << RetTy; } } // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst index cc2717f527847..10aaf67892bd7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst @@ -16,9 +16,6 @@ type, or a derived type thereof. // replaced by: A x = a; - // Finds: - bool b = isa<A>(a) // always true - struct B : public A {}; B b; // Finds: @@ -26,6 +23,13 @@ type, or a derived type thereof. // replaced by: A y = b; + struct C : public A {}; + C c; + // Finds: + bool r1 = isa<A>(a) // always true + bool r2 = isa<A>(b) // always true + bool r3 = isa<B, C>(c) // always true + Supported functions: - ``llvm::cast`` - ``llvm::cast_or_null`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp index 9275f659b51a4..715826eee37c2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp @@ -2,11 +2,11 @@ namespace llvm { #define ISA_FUNCTION(name) \ -template <typename To, typename From> \ +template <typename ...To, typename From> \ [[nodiscard]] inline bool name(const From &Val) { \ return true; \ } \ -template <typename To, typename From> \ +template <typename ...To, typename From> \ [[nodiscard]] inline bool name(const From *Val) { \ return true; \ } @@ -124,3 +124,25 @@ void testIsaConstPointer(const A* value) { // CHECK-MESSAGES: :[[@LINE-2]]:27: note: source expression has pointee type 'A' (void)b14; } + +void testIsaMulti(A& value) { + bool b15 = llvm::isa<int, A>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:32: note: source expression has type 'A' + (void)b15; +} + +void testIsaMultiUpcast(B& value) { + bool b16 = llvm::isa<int, A, B>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:35: note: source expression has type 'B', which is a subtype of 'A' + (void)b16; +} + +template <typename T> +void testIsaTemplateMulti(const T* value) { + bool b17 = llvm::isa<A, T>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:30: note: source expression has pointee type 'T' + (void)b17; +} >From e9595d1ac93006c953c66547955f6142e750fa73 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <[email protected]> Date: Wed, 8 Apr 2026 16:38:01 -0700 Subject: [PATCH 3/4] [clang-tidy] add additional namespace isa tests (NFC) Bring `isa` test coverage up to that of `cast`. --- .../checkers/llvm/redundant-isa.cpp | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp index 715826eee37c2..bb6941b81a712 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/redundant-isa.cpp @@ -146,3 +146,24 @@ void testIsaTemplateMulti(const T* value) { // CHECK-MESSAGES: :[[@LINE-2]]:30: note: source expression has pointee type 'T' (void)b17; } + +namespace llvm { +namespace magic { +template <typename T> +void testIsaImplicitlyLLVMUnresolve(T* value) { + bool b18 = isa<T>(value); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: call to 'isa' always succeeds [llvm-redundant-casting] + // CHECK-MESSAGES: :[[@LINE-2]]:21: note: source expression has pointee type 'T' + (void)b18; +} +} // namespace magic +} // namespace llvm + + +ISA_FUNCTION(isa) + +void testIsaNonLLVM(A& value) { + bool b19 = isa<A>(value); + (void)b19; +} + >From 4aebea6d6a5997928b2ba957102e302957761376 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <[email protected]> Date: Wed, 8 Apr 2026 16:46:25 -0700 Subject: [PATCH 4/4] clang format --- .../clang-tidy/llvm/RedundantCastingCheck.cpp | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp b/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp index f5f05ba1f496b..e1162b6a5d59f 100644 --- a/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp @@ -71,13 +71,12 @@ void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) { auto AnyIsaCalleeName = allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), callee(expr(ignoringImpCasts( - declRefExpr( - to(namedDecl(hasAnyName(IsaFunctionNames), IsInLLVMNamespace)), - hasAnyTemplateArgumentLoc(anything())) + declRefExpr(to(namedDecl(hasAnyName(IsaFunctionNames), + IsInLLVMNamespace)), + hasAnyTemplateArgumentLoc(anything())) .bind("callee"))))); - Finder->addMatcher(callExpr(AnyIsaCalleeName, argumentCountIs(1)) - .bind("call"), - this); + Finder->addMatcher( + callExpr(AnyIsaCalleeName, argumentCountIs(1)).bind("call"), this); auto AnyIsaCalleeNameInUninstantiatedTemplate = allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), @@ -172,13 +171,13 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { for (CanQualType TargetTy : TargetTypes) { const auto *RetDecl = TargetTy->getAsCXXRecordDecl(); - const bool IsDerived = FromDecl && RetDecl && FromDecl->isDerivedFrom(RetDecl); + const bool IsDerived = + FromDecl && RetDecl && FromDecl->isDerivedFrom(RetDecl); if (FromTy != TargetTy && !IsDerived) continue; if (IsIsa) { - diag(Call->getExprLoc(), "call to '%0' always succeeds") - << FuncName; + diag(Call->getExprLoc(), "call to '%0' always succeeds") << FuncName; } else { QualType ParentTy; if (const auto *ParentCast = Nodes.getNodeAs<Expr>("parent_cast")) { @@ -200,7 +199,8 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { /*RecordPaths=*/false, /*DetectVirtual=*/false); const bool IsDerivedFromParent = - FromDecl && ParentDecl && FromDecl->isDerivedFrom(ParentDecl, Paths); + FromDecl && ParentDecl && + FromDecl->isDerivedFrom(ParentDecl, Paths); // For the following case a direct `cast<A>(d)` would be ambiguous: // struct A {}; // struct B : A {}; @@ -225,10 +225,11 @@ void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) { // printing the canonical type for a template parameter prints as e.g. // 'type-parameter-0-0' const QualType DiagFromTy(ArgPointeeTy->getUnqualifiedDesugaredType(), 0); - diag(Arg->getExprLoc(), - "source expression has%select{| pointee}0 type %1%select{|, which is a " - "subtype of %3}2", - DiagnosticIDs::Note) + diag( + Arg->getExprLoc(), + "source expression has%select{| pointee}0 type %1%select{|, which is a " + "subtype of %3}2", + DiagnosticIDs::Note) << Arg->getSourceRange() << ArgTy->isPointerType() << DiagFromTy << (FromTy != TargetTy) << TargetTy; return; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
