Skip to content

Commit 430e1be

Browse files
hnrklssnlocalspook
hnrklssn
and
localspook
authored
[clang-tidy] detect redundant uses of LLVM's cast, dyn_cast (llvm#189274)
Warns when casting to the same pointee type, or when the target pointee type is a super type of the argument's pointee type. Supported functions: - cast - cast_if_present - cast_or_null - dyn_cast - dyn_cast_if_present - dyn_cast_or_null --------- Co-authored-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
1 parent 815edc3 commit 430e1be

File tree

10 files changed

+625
-7
lines changed

10 files changed

+625
-7
lines changed

‎clang-tools-extra/clang-tidy/llvm/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_clang_library(clangTidyLLVMModule STATIC
1010
PreferIsaOrDynCastInConditionalsCheck.cpp
1111
PreferRegisterOverUnsignedCheck.cpp
1212
PreferStaticOverAnonymousNamespaceCheck.cpp
13+
RedundantCastingCheck.cpp
1314
TwineLocalCheck.cpp
1415
TypeSwitchCaseTypesCheck.cpp
1516
UseNewMLIROpBuilderCheck.cpp

‎clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "PreferIsaOrDynCastInConditionalsCheck.h"
1717
#include "PreferRegisterOverUnsignedCheck.h"
1818
#include "PreferStaticOverAnonymousNamespaceCheck.h"
19+
#include "RedundantCastingCheck.h"
1920
#include "TwineLocalCheck.h"
2021
#include "TypeSwitchCaseTypesCheck.h"
2122
#include "UseNewMLIROpBuilderCheck.h"
@@ -43,6 +44,8 @@ class LLVMModule : public ClangTidyModule {
4344
"llvm-prefer-static-over-anonymous-namespace");
4445
CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
4546
"llvm-qualified-auto");
47+
CheckFactories.registerCheck<RedundantCastingCheck>(
48+
"llvm-redundant-casting");
4649
CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
4750
CheckFactories.registerCheck<TypeSwitchCaseTypesCheck>(
4851
"llvm-type-switch-case-types");

‎clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.cpp‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "RedundantCastingCheck.h"
10+
#include "clang/AST/ASTTypeTraits.h"
11+
#include "clang/AST/Decl.h"
12+
#include "clang/AST/DeclCXX.h"
13+
#include "clang/AST/Expr.h"
14+
#include "clang/AST/ExprCXX.h"
15+
#include "clang/AST/NestedNameSpecifierBase.h"
16+
#include "clang/AST/ParentMapContext.h"
17+
#include "clang/AST/TemplateBase.h"
18+
#include "clang/AST/TypeBase.h"
19+
#include "clang/ASTMatchers/ASTMatchFinder.h"
20+
#include "clang/ASTMatchers/ASTMatchers.h"
21+
#include "clang/Lex/Lexer.h"
22+
#include "llvm/ADT/STLExtras.h"
23+
24+
using namespace clang::ast_matchers;
25+
26+
namespace clang::tidy::llvm_check {
27+
28+
namespace {
29+
AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
30+
AST_MATCHER_P(OverloadExpr, hasAnyUnresolvedName, ArrayRef<StringRef>, Names) {
31+
auto DeclName = Node.getName();
32+
if (!DeclName.isIdentifier())
33+
return false;
34+
const IdentifierInfo *II = DeclName.getAsIdentifierInfo();
35+
return llvm::any_of(Names, [II](StringRef Name) { return II->isStr(Name); });
36+
}
37+
} // namespace
38+
39+
static constexpr StringRef FunctionNames[] = {
40+
"cast", "cast_or_null", "cast_if_present",
41+
"dyn_cast", "dyn_cast_or_null", "dyn_cast_if_present"};
42+
43+
void RedundantCastingCheck::registerMatchers(MatchFinder *Finder) {
44+
auto IsInLLVMNamespace = hasDeclContext(
45+
namespaceDecl(hasName("llvm"), hasDeclContext(translationUnitDecl())));
46+
auto AnyCalleeName =
47+
allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
48+
callee(expr(ignoringImpCasts(
49+
declRefExpr(
50+
to(namedDecl(hasAnyName(FunctionNames), IsInLLVMNamespace)),
51+
templateArgumentLocCountIs(1))
52+
.bind("callee")))));
53+
auto AnyCalleeNameInUninstantiatedTemplate =
54+
allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
55+
callee(expr(ignoringImpCasts(
56+
unresolvedLookupExpr(hasAnyUnresolvedName(FunctionNames),
57+
templateArgumentLocCountIs(1))
58+
.bind("callee")))));
59+
Finder->addMatcher(callExpr(AnyCalleeName, argumentCountIs(1),
60+
optionally(hasParent(
61+
callExpr(AnyCalleeName).bind("parent_cast"))))
62+
.bind("call"),
63+
this);
64+
Finder->addMatcher(
65+
callExpr(
66+
AnyCalleeNameInUninstantiatedTemplate, argumentCountIs(1),
67+
optionally(hasAncestor(
68+
namespaceDecl(hasName("llvm"), hasParent(translationUnitDecl()))
69+
.bind("llvm_ns"))))
70+
.bind("call"),
71+
this);
72+
}
73+
74+
static QualType stripPointerOrReference(QualType Ty) {
75+
QualType Pointee = Ty->getPointeeType();
76+
if (Pointee.isNull())
77+
return Ty;
78+
return Pointee;
79+
}
80+
81+
static bool isLLVMNamespace(NestedNameSpecifier NNS) {
82+
if (NNS.getKind() != NestedNameSpecifier::Kind::Namespace)
83+
return false;
84+
auto Pair = NNS.getAsNamespaceAndPrefix();
85+
if (Pair.Namespace->getNamespace()->getName() != "llvm")
86+
return false;
87+
const NestedNameSpecifier::Kind Kind = Pair.Prefix.getKind();
88+
return Kind == NestedNameSpecifier::Kind::Null ||
89+
Kind == NestedNameSpecifier::Kind::Global;
90+
}
91+
92+
void RedundantCastingCheck::check(const MatchFinder::MatchResult &Result) {
93+
const auto &Nodes = Result.Nodes;
94+
const auto *Call = Nodes.getNodeAs<CallExpr>("call");
95+
96+
CanQualType RetTy;
97+
std::string FuncName;
98+
if (const auto *ResolvedCallee = Nodes.getNodeAs<DeclRefExpr>("callee")) {
99+
const auto *F = cast<FunctionDecl>(ResolvedCallee->getDecl());
100+
RetTy = stripPointerOrReference(F->getReturnType())
101+
->getCanonicalTypeUnqualified();
102+
FuncName = F->getName();
103+
} else if (const auto *UnresolvedCallee =
104+
Nodes.getNodeAs<UnresolvedLookupExpr>("callee")) {
105+
const bool IsExplicitlyLLVM =
106+
isLLVMNamespace(UnresolvedCallee->getQualifier());
107+
const auto *CallerNS = Nodes.getNodeAs<NamedDecl>("llvm_ns");
108+
if (!IsExplicitlyLLVM && !CallerNS)
109+
return;
110+
auto TArg = UnresolvedCallee->template_arguments()[0].getArgument();
111+
if (TArg.getKind() != TemplateArgument::Type)
112+
return;
113+
114+
RetTy = TArg.getAsType()->getCanonicalTypeUnqualified();
115+
FuncName = UnresolvedCallee->getName().getAsString();
116+
} else {
117+
llvm_unreachable("");
118+
}
119+
120+
const auto *Arg = Call->getArg(0);
121+
const QualType ArgTy = Arg->getType();
122+
const QualType ArgPointeeTy = stripPointerOrReference(ArgTy);
123+
const CanQualType FromTy = ArgPointeeTy->getCanonicalTypeUnqualified();
124+
const auto *FromDecl = FromTy->getAsCXXRecordDecl();
125+
const auto *RetDecl = RetTy->getAsCXXRecordDecl();
126+
const bool IsDerived =
127+
FromDecl && RetDecl && FromDecl->isDerivedFrom(RetDecl);
128+
if (FromTy != RetTy && !IsDerived)
129+
return;
130+
131+
QualType ParentTy;
132+
if (const auto *ParentCast = Nodes.getNodeAs<Expr>("parent_cast")) {
133+
ParentTy = ParentCast->getType();
134+
} else {
135+
// IgnoreUnlessSpelledInSource prevents matching implicit casts
136+
const TraversalKindScope TmpTraversalKind(*Result.Context, TK_AsIs);
137+
for (const DynTypedNode Parent : Result.Context->getParents(*Call)) {
138+
if (const auto *ParentCastExpr = Parent.get<CastExpr>()) {
139+
ParentTy = ParentCastExpr->getType();
140+
break;
141+
}
142+
}
143+
}
144+
if (!ParentTy.isNull()) {
145+
const CXXRecordDecl *ParentDecl = ParentTy->getAsCXXRecordDecl();
146+
if (FromDecl && ParentDecl) {
147+
CXXBasePaths Paths(/*FindAmbiguities=*/true,
148+
/*RecordPaths=*/false,
149+
/*DetectVirtual=*/false);
150+
const bool IsDerivedFromParent =
151+
FromDecl && ParentDecl && FromDecl->isDerivedFrom(ParentDecl, Paths);
152+
// For the following case a direct `cast<A>(d)` would be ambiguous:
153+
// struct A {};
154+
// struct B : A {};
155+
// struct C : A {};
156+
// struct D : B, C {};
157+
// So we should not warn for `A *a = cast<C>(d)`.
158+
if (IsDerivedFromParent &&
159+
Paths.isAmbiguous(ParentTy->getCanonicalTypeUnqualified()))
160+
return;
161+
}
162+
}
163+
164+
auto GetText = [&](SourceRange R) {
165+
return Lexer::getSourceText(CharSourceRange::getTokenRange(R),
166+
*Result.SourceManager, getLangOpts());
167+
};
168+
StringRef ArgText = GetText(Arg->getSourceRange());
169+
diag(Call->getExprLoc(), "redundant use of '%0'")
170+
<< FuncName
171+
<< FixItHint::CreateReplacement(Call->getSourceRange(), ArgText);
172+
// printing the canonical type for a template parameter prints as e.g.
173+
// 'type-parameter-0-0'
174+
const QualType DiagFromTy(ArgPointeeTy->getUnqualifiedDesugaredType(), 0);
175+
diag(Arg->getExprLoc(),
176+
"source expression has%select{| pointee}0 type %1%select{|, which is a "
177+
"subtype of %3}2",
178+
DiagnosticIDs::Note)
179+
<< Arg->getSourceRange() << ArgTy->isPointerType() << DiagFromTy
180+
<< (FromTy != RetTy) << RetTy;
181+
}
182+
183+
} // namespace clang::tidy::llvm_check

‎clang-tools-extra/clang-tidy/llvm/RedundantCastingCheck.h‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_REDUNDANTCASTINGCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_REDUNDANTCASTINGCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::llvm_check {
15+
16+
/// Detect redundant uses of LLVM's cast and dyn_cast functions.
17+
///
18+
/// For the user-facing documentation see:
19+
/// https://clang.llvm.org/extra/clang-tidy/checks/llvm/redundant-casting.html
20+
class RedundantCastingCheck : public ClangTidyCheck {
21+
public:
22+
RedundantCastingCheck(StringRef Name, ClangTidyContext *Context)
23+
: ClangTidyCheck(Name, Context) {}
24+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
25+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
26+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
27+
return LangOpts.CPlusPlus;
28+
}
29+
30+
std::optional<TraversalKind> getCheckTraversalKind() const override {
31+
// Casts can be redundant for some instantiations but not others.
32+
// Only emit warnings in templates in the uninstantated versions.
33+
return TK_IgnoreUnlessSpelledInSource;
34+
}
35+
};
36+
37+
} // namespace clang::tidy::llvm_check
38+
39+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_REDUNDANTCASTINGCHECK_H

‎clang-tools-extra/docs/ReleaseNotes.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,13 @@ New checks
125125
Finds functions where throwing exceptions is unsafe but the function is still
126126
marked as potentially throwing.
127127

128+
- New :doc:`llvm-redundant-casting
129+
<clang-tidy/checks/llvm/redundant-casting>` check.
130+
131+
Points out uses of ``cast<>``, ``dyn_cast<>`` and their ``or_null`` variants
132+
that are unnecessary because the argument already is of the target type, or a
133+
derived type thereof.
134+
128135
- New :doc:`llvm-type-switch-case-types
129136
<clang-tidy/checks/llvm/type-switch-case-types>` check.
130137

‎clang-tools-extra/docs/clang-tidy/checks/list.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ Clang-Tidy Checks
252252
:doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes"
253253
:doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes"
254254
:doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`,
255+
:doc:`llvm-redundant-casting <llvm/redundant-casting>`, "Yes"
255256
:doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
256257
:doc:`llvm-type-switch-case-types <llvm/type-switch-case-types>`, "Yes"
257258
:doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"

‎clang-tools-extra/docs/clang-tidy/checks/llvm/redundant-casting.rst‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
.. title:: clang-tidy - llvm-redundant-casting
2+
3+
llvm-redundant-casting
4+
======================
5+
6+
Points out uses of ``cast<>``, ``dyn_cast<>`` and their ``or_null`` variants
7+
that are unnecessary because the argument already is of the target type, or a
8+
derived type thereof.
9+
10+
.. code-block:: c++
11+
12+
struct A {};
13+
A a;
14+
// Finds:
15+
A x = cast<A>(a);
16+
// replaced by:
17+
A x = a;
18+
19+
struct B : public A {};
20+
B b;
21+
// Finds:
22+
A y = cast<A>(b);
23+
// replaced by:
24+
A y = b;
25+
26+
Supported functions:
27+
- ``llvm::cast``
28+
- ``llvm::cast_or_null``
29+
- ``llvm::cast_if_present``
30+
- ``llvm::dyn_cast``
31+
- ``llvm::dyn_cast_or_null``
32+
- ``llvm::dyn_cast_if_present``

0 commit comments

Comments
 (0)