[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