[clang-tools-extra] [Clang-Tidy] Handle nested-name-specifier in "llvm-prefer-isa-or-dyn-cast-in-conditionals" (PR #155982)
Yanzuo Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 3 08:01:37 PDT 2025
https://github.com/zwuis updated https://github.com/llvm/llvm-project/pull/155982
>From 426caa9f66cddd1deac23b397baf75f6809f6f52 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Fri, 29 Aug 2025 15:15:24 +0800
Subject: [PATCH 1/5] Handle nested-name-specifier in
"llvm-prefer-isa-or-dyn-cast-in-conditionals"
---
.../PreferIsaOrDynCastInConditionalsCheck.cpp | 119 ++++++++----------
clang-tools-extra/docs/ReleaseNotes.rst | 13 ++
...prefer-isa-or-dyn-cast-in-conditionals.cpp | 75 +++++++++++
3 files changed, 143 insertions(+), 64 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
index 4c138bcc564d8..5e9777248896a 100644
--- a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/Support/FormatVariadic.h"
using namespace clang::ast_matchers;
@@ -22,105 +23,95 @@ AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
void PreferIsaOrDynCastInConditionalsCheck::registerMatchers(
MatchFinder *Finder) {
- auto Condition = hasCondition(implicitCastExpr(has(
- callExpr(unless(isMacroID()), unless(cxxMemberCallExpr()),
- anyOf(callee(namedDecl(hasName("cast"))),
- callee(namedDecl(hasName("dyn_cast")).bind("dyn_cast"))))
- .bind("call"))));
-
- auto Any = anyOf(
- has(declStmt(containsDeclaration(
- 0, varDecl(hasInitializer(callExpr(unless(isMacroID()),
- unless(cxxMemberCallExpr()),
- callee(namedDecl(hasName("cast"))))
- .bind("assign")))))),
- Condition);
-
- auto CallExpression =
+ auto AnyCalleeName = [](ArrayRef<StringRef> CalleeName) {
+ return allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
+ callee(expr(ignoringImpCasts(
+ declRefExpr(to(namedDecl(hasAnyName(CalleeName))),
+ hasAnyTemplateArgumentLoc(anything()))
+ .bind("callee")))));
+ };
+
+ auto Condition = hasCondition(implicitCastExpr(
+ has(callExpr(AnyCalleeName({"cast", "dyn_cast"})).bind("cond"))));
+
+ auto Any =
+ anyOf(hasConditionVariableStatement(containsDeclaration(
+ 0, varDecl(hasInitializer(callExpr(AnyCalleeName({"cast"}))))
+ .bind("var"))),
+ Condition);
+
+ auto CallWithBindedArg =
callExpr(
-
- unless(isMacroID()), unless(cxxMemberCallExpr()),
- callee(namedDecl(hasAnyName("isa", "cast", "cast_or_null", "dyn_cast",
- "dyn_cast_or_null"))
- .bind("func")),
+ AnyCalleeName({"isa", "cast", "cast_or_null", "cast_if_present",
+ "dyn_cast", "dyn_cast_or_null",
+ "dyn_cast_if_present"}),
hasArgument(0, mapAnyOf(declRefExpr, cxxMemberCallExpr).bind("arg")))
.bind("rhs");
Finder->addMatcher(
- traverse(
- TK_AsIs,
- stmt(anyOf(
- ifStmt(Any), whileStmt(Any), doStmt(Condition),
- binaryOperator(unless(isExpansionInFileMatching(
- "llvm/include/llvm/Support/Casting.h")),
- hasOperatorName("&&"),
- hasLHS(implicitCastExpr().bind("lhs")),
- hasRHS(anyOf(implicitCastExpr(has(CallExpression)),
- CallExpression)))
- .bind("and")))),
+ stmt(anyOf(ifStmt(Any), forStmt(Any), whileStmt(Any), doStmt(Condition),
+ binaryOperator(unless(isExpansionInFileMatching(
+ "llvm/include/llvm/Support/Casting.h")),
+ hasOperatorName("&&"),
+ hasLHS(implicitCastExpr().bind("lhs")),
+ hasRHS(ignoringImpCasts(CallWithBindedArg)))
+ .bind("and"))),
this);
}
void PreferIsaOrDynCastInConditionalsCheck::check(
const MatchFinder::MatchResult &Result) {
- if (const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("assign")) {
- SourceLocation StartLoc = MatchedDecl->getCallee()->getExprLoc();
- SourceLocation EndLoc =
- StartLoc.getLocWithOffset(StringRef("cast").size() - 1);
+ const auto *Callee = Result.Nodes.getNodeAs<DeclRefExpr>("callee");
+ if (!Callee)
+ return;
+
+ SourceLocation StartLoc = Callee->getLocation();
+ SourceLocation EndLoc = Callee->getNameInfo().getEndLoc();
- diag(MatchedDecl->getBeginLoc(),
+ if (Result.Nodes.getNodeAs<VarDecl>("var")) {
+ diag(StartLoc,
"cast<> in conditional will assert rather than return a null pointer")
<< FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc),
"dyn_cast");
- } else if (const auto *MatchedDecl =
- Result.Nodes.getNodeAs<CallExpr>("call")) {
- SourceLocation StartLoc = MatchedDecl->getCallee()->getExprLoc();
- SourceLocation EndLoc =
- StartLoc.getLocWithOffset(StringRef("cast").size() - 1);
-
+ } else if (Result.Nodes.getNodeAs<CallExpr>("cond")) {
StringRef Message =
"cast<> in conditional will assert rather than return a null pointer";
- if (Result.Nodes.getNodeAs<NamedDecl>("dyn_cast"))
+ if (Callee->getDecl()->getName() == "dyn_cast")
Message = "return value from dyn_cast<> not used";
- diag(MatchedDecl->getBeginLoc(), Message)
+ diag(StartLoc, Message)
<< FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc), "isa");
- } else if (const auto *MatchedDecl =
- Result.Nodes.getNodeAs<BinaryOperator>("and")) {
+ } else if (Result.Nodes.getNodeAs<BinaryOperator>("and")) {
const auto *LHS = Result.Nodes.getNodeAs<ImplicitCastExpr>("lhs");
const auto *RHS = Result.Nodes.getNodeAs<CallExpr>("rhs");
const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
- const auto *Func = Result.Nodes.getNodeAs<NamedDecl>("func");
assert(LHS && "LHS is null");
assert(RHS && "RHS is null");
assert(Arg && "Arg is null");
- assert(Func && "Func is null");
- StringRef LHSString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(LHS->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
+ auto GetText = [&](SourceRange R) {
+ return Lexer::getSourceText(CharSourceRange::getTokenRange(R),
+ *Result.SourceManager, getLangOpts());
+ };
- StringRef ArgString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(Arg->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
+ StringRef LHSString = GetText(LHS->getSourceRange());
+
+ StringRef ArgString = GetText(Arg->getSourceRange());
if (ArgString != LHSString)
return;
- StringRef RHSString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(RHS->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
-
- std::string Replacement("isa_and_nonnull");
- Replacement += RHSString.substr(Func->getName().size());
+ std::string Replacement = llvm::formatv(
+ "{}{}{}", GetText(Callee->getQualifierLoc().getSourceRange()),
+ "isa_and_nonnull",
+ GetText(SourceRange(Callee->getLAngleLoc(), RHS->getEndLoc())));
- diag(MatchedDecl->getBeginLoc(),
+ diag(LHS->getBeginLoc(),
"isa_and_nonnull<> is preferred over an explicit test for null "
"followed by calling isa<>")
- << FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),
- MatchedDecl->getEndLoc()),
- Replacement);
+ << FixItHint::CreateReplacement(
+ SourceRange(LHS->getBeginLoc(), RHS->getEndLoc()), Replacement);
}
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 06641a602e28f..0829d336932c6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -244,6 +244,19 @@ Changes in existing checks
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
`IgnoreAliasing`, that allows not looking at underlying types of type aliases.
+- Improved :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals
+ <clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals>` check:
+
+ - Fix-it handles Callees with nested-name-specifier correctly.
+
+ - ``if`` statements with init-statement (``if (auto X = ...; ...)``) are
+ handled correctly.
+
+ - ``for`` loops are supported.
+
+ - ``cast_if_present`` and ``dyn_cast_if_present`` are supported in the case
+ that ``isa_and_nonnull`` is preferred.
+
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp
index 88e4b643004fc..8683926bace11 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp
@@ -9,14 +9,24 @@ struct Z {
bool baz(Y*);
};
+namespace llvm {
template <class X, class Y>
bool isa(Y *);
template <class X, class Y>
X *cast(Y *);
template <class X, class Y>
+X *cast_or_null(Y *);
+template <class X, class Y>
+X *cast_if_present(Y *);
+template <class X, class Y>
X *dyn_cast(Y *);
template <class X, class Y>
X *dyn_cast_or_null(Y *);
+template <class X, class Y>
+X *dyn_cast_if_present(Y *);
+} // namespace llvm
+
+using namespace llvm;
bool foo(Y *y, Z *z) {
if (auto x = cast<X>(y))
@@ -24,6 +34,26 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:16: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
// CHECK-FIXES: if (auto x = dyn_cast<X>(y))
+ if (auto x = ::cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:18: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (auto x = ::dyn_cast<X>(y))
+
+ if (auto x = llvm::cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (auto x = llvm::dyn_cast<X>(y))
+
+ if (auto x = ::llvm::cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (auto x = ::llvm::dyn_cast<X>(y))
+
+ for (; auto x = cast<X>(y); )
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional
+ // CHECK-FIXES: for (; auto x = dyn_cast<X>(y); )
+
while (auto x = cast<X>(y))
break;
// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional
@@ -34,6 +64,16 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: cast<> in conditional
// CHECK-FIXES: if (isa<X>(y))
+ if (auto x = cast<X>(y); cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (auto x = cast<X>(y); isa<X>(y))
+
+ for (; cast<X>(y); )
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional
+ // CHECK-FIXES: for (; isa<X>(y); )
+
while (cast<X>(y))
break;
// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional
@@ -50,6 +90,11 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: return value from dyn_cast<> not used [llvm-prefer-isa-or-dyn-cast-in-conditionals]
// CHECK-FIXES: if (isa<X>(y))
+ for (; dyn_cast<X>(y); )
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used
+ // CHECK-FIXES: for (; isa<X>(y); )
+
while (dyn_cast<X>(y))
break;
// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used
@@ -66,6 +111,21 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]
// CHECK-FIXES: if (isa_and_nonnull<X>(y))
+ if (y && ::isa<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (::isa_and_nonnull<X>(y))
+
+ if (y && llvm::isa<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (llvm::isa_and_nonnull<X>(y))
+
+ if (y && ::llvm::isa<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (::llvm::isa_and_nonnull<X>(y))
+
if (z->bar() && isa<Y>(z->bar()))
return true;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
@@ -76,6 +136,16 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
// CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
+ if (z->bar() && cast_or_null<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+ // CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
+
+ if (z->bar() && cast_if_present<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+ // CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
+
if (z->bar() && dyn_cast<Y>(z->bar()))
return true;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
@@ -86,6 +156,11 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
// CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
+ if (z->bar() && dyn_cast_if_present<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+ // CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
+
bool b = z->bar() && cast<Y>(z->bar());
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: isa_and_nonnull<> is preferred
// CHECK-FIXES: bool b = isa_and_nonnull<Y>(z->bar());
>From fe32d8a0ad42b7bc6ce4a0dfaee53bde950237d8 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Sat, 30 Aug 2025 22:11:25 +0800
Subject: [PATCH 2/5] Remove empty line, add `const`, and make known string
inline
---
.../llvm/PreferIsaOrDynCastInConditionalsCheck.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
index 5e9777248896a..45a808cafa941 100644
--- a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
@@ -96,15 +96,14 @@ void PreferIsaOrDynCastInConditionalsCheck::check(
};
StringRef LHSString = GetText(LHS->getSourceRange());
-
StringRef ArgString = GetText(Arg->getSourceRange());
if (ArgString != LHSString)
return;
- std::string Replacement = llvm::formatv(
- "{}{}{}", GetText(Callee->getQualifierLoc().getSourceRange()),
- "isa_and_nonnull",
+ const std::string Replacement = llvm::formatv(
+ "{}isa_and_nonnull{}",
+ GetText(Callee->getQualifierLoc().getSourceRange()),
GetText(SourceRange(Callee->getLAngleLoc(), RHS->getEndLoc())));
diag(LHS->getBeginLoc(),
>From 52265dbc2641938ca955f279d351a9c9f1d42114 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Sat, 30 Aug 2025 23:55:07 +0800
Subject: [PATCH 3/5] Use `assert` instead of branch
---
.../llvm/PreferIsaOrDynCastInConditionalsCheck.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
index 45a808cafa941..dae5f2bb4a379 100644
--- a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
@@ -62,8 +62,9 @@ void PreferIsaOrDynCastInConditionalsCheck::registerMatchers(
void PreferIsaOrDynCastInConditionalsCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *Callee = Result.Nodes.getNodeAs<DeclRefExpr>("callee");
- if (!Callee)
- return;
+
+ // Callee should be matched if anything is matched.
+ assert(Callee && "Callee is null");
SourceLocation StartLoc = Callee->getLocation();
SourceLocation EndLoc = Callee->getNameInfo().getEndLoc();
>From 220bdf1e7236aa7fb42c529abd1efadb28f97193 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Wed, 3 Sep 2025 22:18:00 +0800
Subject: [PATCH 4/5] Use more meaningful variable names, and revert supporting
`cast_if_present` and `dyn_cast_if_present` Let's wait until reaching a
consense on preferring `isa_and_nonnull` or `isa_and_present`
---
.../PreferIsaOrDynCastInConditionalsCheck.cpp | 27 ++++++++++++-------
clang-tools-extra/docs/ReleaseNotes.rst | 3 ---
...prefer-isa-or-dyn-cast-in-conditionals.cpp | 14 ----------
3 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
index dae5f2bb4a379..7810356d6b86d 100644
--- a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
@@ -31,25 +31,25 @@ void PreferIsaOrDynCastInConditionalsCheck::registerMatchers(
.bind("callee")))));
};
- auto Condition = hasCondition(implicitCastExpr(
+ auto CondExpr = hasCondition(implicitCastExpr(
has(callExpr(AnyCalleeName({"cast", "dyn_cast"})).bind("cond"))));
- auto Any =
+ auto CondExprOrCondVar =
anyOf(hasConditionVariableStatement(containsDeclaration(
0, varDecl(hasInitializer(callExpr(AnyCalleeName({"cast"}))))
.bind("var"))),
- Condition);
+ CondExpr);
auto CallWithBindedArg =
callExpr(
- AnyCalleeName({"isa", "cast", "cast_or_null", "cast_if_present",
- "dyn_cast", "dyn_cast_or_null",
- "dyn_cast_if_present"}),
+ AnyCalleeName(
+ {"isa", "cast", "cast_or_null", "dyn_cast", "dyn_cast_or_null"}),
hasArgument(0, mapAnyOf(declRefExpr, cxxMemberCallExpr).bind("arg")))
.bind("rhs");
Finder->addMatcher(
- stmt(anyOf(ifStmt(Any), forStmt(Any), whileStmt(Any), doStmt(Condition),
+ stmt(anyOf(ifStmt(CondExprOrCondVar), forStmt(CondExprOrCondVar),
+ whileStmt(CondExprOrCondVar), doStmt(CondExpr),
binaryOperator(unless(isExpansionInFileMatching(
"llvm/include/llvm/Support/Casting.h")),
hasOperatorName("&&"),
@@ -63,9 +63,12 @@ void PreferIsaOrDynCastInConditionalsCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *Callee = Result.Nodes.getNodeAs<DeclRefExpr>("callee");
- // Callee should be matched if anything is matched.
- assert(Callee && "Callee is null");
+ assert(Callee && "Callee should be binded if anything is matched");
+ // The first and last letter of the identifier
+ // llvm::cast<T>(x)
+ // ^ ^
+ // StartLoc EndLoc
SourceLocation StartLoc = Callee->getLocation();
SourceLocation EndLoc = Callee->getNameInfo().getEndLoc();
@@ -102,6 +105,9 @@ void PreferIsaOrDynCastInConditionalsCheck::check(
if (ArgString != LHSString)
return;
+ // It is not clear which is preferred between `isa_and_nonnull` and
+ // `isa_and_present`. See
+ // https://discourse.llvm.org/t/psa-swapping-out-or-null-with-if-present/65018
const std::string Replacement = llvm::formatv(
"{}isa_and_nonnull{}",
GetText(Callee->getQualifierLoc().getSourceRange()),
@@ -112,6 +118,9 @@ void PreferIsaOrDynCastInConditionalsCheck::check(
"followed by calling isa<>")
<< FixItHint::CreateReplacement(
SourceRange(LHS->getBeginLoc(), RHS->getEndLoc()), Replacement);
+ } else {
+ llvm_unreachable(
+ R"(One of "var", "cond" and "and" should be binded if anything is matched)");
}
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 0829d336932c6..ef611b4f7bf98 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -254,9 +254,6 @@ Changes in existing checks
- ``for`` loops are supported.
- - ``cast_if_present`` and ``dyn_cast_if_present`` are supported in the case
- that ``isa_and_nonnull`` is preferred.
-
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp
index 8683926bace11..6b4c917f6c599 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp
@@ -17,13 +17,9 @@ X *cast(Y *);
template <class X, class Y>
X *cast_or_null(Y *);
template <class X, class Y>
-X *cast_if_present(Y *);
-template <class X, class Y>
X *dyn_cast(Y *);
template <class X, class Y>
X *dyn_cast_or_null(Y *);
-template <class X, class Y>
-X *dyn_cast_if_present(Y *);
} // namespace llvm
using namespace llvm;
@@ -141,11 +137,6 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
// CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
- if (z->bar() && cast_if_present<Y>(z->bar()))
- return true;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
- // CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
-
if (z->bar() && dyn_cast<Y>(z->bar()))
return true;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
@@ -156,11 +147,6 @@ bool foo(Y *y, Z *z) {
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
// CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
- if (z->bar() && dyn_cast_if_present<Y>(z->bar()))
- return true;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
- // CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar()))
-
bool b = z->bar() && cast<Y>(z->bar());
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: isa_and_nonnull<> is preferred
// CHECK-FIXES: bool b = isa_and_nonnull<Y>(z->bar());
>From f2015e44cf9f1d22821ea87aaaaf99b7af0c6c66 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Wed, 3 Sep 2025 23:01:18 +0800
Subject: [PATCH 5/5] Keep alphabetical order
---
clang-tools-extra/docs/ReleaseNotes.rst | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5f1cd07b1a07b..c25291c8c7dc4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -196,6 +196,16 @@ Changes in existing checks
adding an option to allow pointer arithmetic via prefix/postfix increment or
decrement operators.
+- Improved :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals
+ <clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals>` check:
+
+ - Fix-it handles Callees with nested-name-specifier correctly.
+
+ - ``if`` statements with init-statement (``if (auto X = ...; ...)``) are
+ handled correctly.
+
+ - ``for`` loops are supported.
+
- Improved :doc:`misc-header-include-cycle
<clang-tidy/checks/misc/header-include-cycle>` check performance.
@@ -249,16 +259,6 @@ Changes in existing checks
<clang-tidy/checks/readability/uppercase-literal-suffix>` check to recognize
literal suffixes added in C++23 and C23.
-- Improved :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals
- <clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals>` check:
-
- - Fix-it handles Callees with nested-name-specifier correctly.
-
- - ``if`` statements with init-statement (``if (auto X = ...; ...)``) are
- handled correctly.
-
- - ``for`` loops are supported.
-
Removed checks
^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list