[clang-tools-extra] [Clang-Tidy] Handle nested-name-specifier in "llvm-prefer-isa-or-dyn-cast-in-conditionals" (PR #155982)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 7 15:01:49 PDT 2025
================
@@ -22,105 +23,104 @@ 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 CondExpr = hasCondition(implicitCastExpr(
+ has(callExpr(AnyCalleeName({"cast", "dyn_cast"})).bind("cond"))));
+
+ auto CondExprOrCondVar =
+ anyOf(hasConditionVariableStatement(containsDeclaration(
+ 0, varDecl(hasInitializer(callExpr(AnyCalleeName({"cast"}))))
+ .bind("var"))),
+ CondExpr);
+
+ 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", "dyn_cast", "dyn_cast_or_null"}),
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(CondExprOrCondVar), forStmt(CondExprOrCondVar),
+ whileStmt(CondExprOrCondVar), doStmt(CondExpr),
+ 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");
+
+ 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();
- 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());
----------------
EugeneZelenko wrote:
```suggestion
const StringRef LHSString = GetText(LHS->getSourceRange());
const StringRef ArgString = GetText(Arg->getSourceRange());
```
https://github.com/llvm/llvm-project/pull/155982
More information about the cfe-commits
mailing list