diff --git a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h --- a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h @@ -56,6 +56,9 @@ } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + mutable std::optional HasIsPresent; }; } // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp --- a/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp @@ -9,6 +9,8 @@ #include "PreferIsaOrDynCastInConditionalsCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclarationName.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" @@ -41,14 +43,14 @@ auto CallExpression = callExpr( - allOf( - unless(isMacroID()), unless(cxxMemberCallExpr()), - allOf(callee(namedDecl(hasAnyName("isa", "cast", "cast_or_null", - "dyn_cast", "dyn_cast_or_null")) - .bind("func")), - hasArgument( - 0, - mapAnyOf(declRefExpr, cxxMemberCallExpr).bind("arg"))))) + allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), + allOf(callee(namedDecl(hasAnyName("isa", "cast", "cast_or_null", + "cast_if_present", "dyn_cast", + "dyn_cast_or_null", + "dyn_cast_if_present")) + .bind("func")), + hasArgument(0, mapAnyOf(declRefExpr, cxxMemberCallExpr) + .bind("arg"))))) .bind("rhs"); Finder->addMatcher( @@ -66,8 +68,31 @@ this); } +static bool FindIsaAndPresent(const ASTContext &ctx) { + auto LLVMIdent = ctx.Idents.find("llvm"); + if (LLVMIdent == ctx.Idents.end()) + return false; + auto IsaAndPresent = ctx.Idents.find("isa_and_present"); + if (IsaAndPresent == ctx.Idents.end()) + return false; + for (const NamedDecl *TopLevelLLVM : + ctx.getTranslationUnitDecl()->lookup(LLVMIdent->getValue())) { + const auto *NS = dyn_cast(TopLevelLLVM); + if (!NS) + continue; + for (const NamedDecl *IsaAndPresentDecl : + NS->lookup(IsaAndPresent->getValue())) { + if (isa(IsaAndPresentDecl)) + return true; + } + } + return false; +} + void PreferIsaOrDynCastInConditionalsCheck::check( const MatchFinder::MatchResult &Result) { + if (!HasIsPresent) + HasIsPresent = FindIsaAndPresent(*Result.Context); if (const auto *MatchedDecl = Result.Nodes.getNodeAs("assign")) { SourceLocation StartLoc = MatchedDecl->getCallee()->getExprLoc(); SourceLocation EndLoc = @@ -117,12 +142,16 @@ CharSourceRange::getTokenRange(RHS->getSourceRange()), *Result.SourceManager, getLangOpts())); - std::string Replacement("isa_and_nonnull"); - Replacement += RHSString.substr(Func->getName().size()); + StringRef ReplaceFunc = + *HasIsPresent ? "isa_and_present" : "isa_and_nonnull"; + + std::string Replacement = + (ReplaceFunc + RHSString.substr(Func->getName().size())).str(); diag(MatchedDecl->getBeginLoc(), - "isa_and_nonnull<> is preferred over an explicit test for null " + "%0<> is preferred over an explicit test for null " "followed by calling isa<>") + << ReplaceFunc << FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(), MatchedDecl->getEndLoc()), Replacement); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -317,6 +317,11 @@ ` when warning would be emitted for a const local variable to which NRVO is applied. +- Updated :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals + ` check to + use the ``*and_present`` and ``*if_present`` templates added in + `D123901 `_. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals.rst --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals.rst @@ -26,7 +26,7 @@ if (var && isa(var)) {} // is replaced by: - if (isa_and_nonnull(var.foo())) {} + if (isa_and_present(var.foo())) {} // Other cases are ignored, e.g.: if (auto f = cast(y)->foo()) {} 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 --- 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 @@ -1,5 +1,5 @@ -// RUN: %check_clang_tidy %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t - +// RDUN: %check_clang_tidy -check-suffixes=,NO-PRESENT %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t +// RUN: %check_clang_tidy -check-suffixes=,PRESENT %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t -- -- -DISA_AND_PRESENT struct X; struct Y; struct Z { @@ -9,6 +9,7 @@ bool baz(Y*); }; +namespace llvm { template bool isa(Y *); template @@ -17,6 +18,14 @@ X *dyn_cast(Y *); template X *dyn_cast_or_null(Y *); +template +X *dyn_cast_if_present(Y *); +#ifdef ISA_AND_PRESENT +template +bool isa_and_present(Y *); +#endif +} // namespace llvm +using namespace llvm; bool foo(Y *y, Z *z) { if (auto x = cast(y)) @@ -63,32 +72,51 @@ if (y && isa(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(y)) + // CHECK-MESSAGES-PRESENT: :[[@LINE-2]]:7: warning: isa_and_present<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals] + // CHECK-FIXES-PRESENT: if (isa_and_present(y)) + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-4]]: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-NO-PRESENT: if (isa_and_nonnull(y)) if (z->bar() && isa(z->bar())) return true; - // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred - // CHECK-FIXES: if (isa_and_nonnull(z->bar())) + // CHECK-MESSAGES-PRESENT: :[[@LINE-2]]:7: warning: isa_and_present<> is preferred + // CHECK-FIXES-PRESENT: if (isa_and_present(z->bar())) + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-4]]:7: warning: isa_and_nonnull<> is preferred + // CHECK-FIXES-NO-PRESENT: if (isa_and_nonnull(z->bar())) if (z->bar() && cast(z->bar())) return true; - // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred - // CHECK-FIXES: if (isa_and_nonnull(z->bar())) + // CHECK-MESSAGES-PRESENT: :[[@LINE-2]]:7: warning: isa_and_present<> is preferred + // CHECK-FIXES-PRESENT: if (isa_and_present(z->bar())) + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-4]]:7: warning: isa_and_nonnull<> is preferred + // CHECK-FIXES-NO-PRESENT: if (isa_and_nonnull(z->bar())) if (z->bar() && dyn_cast(z->bar())) return true; - // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred - // CHECK-FIXES: if (isa_and_nonnull(z->bar())) + // CHECK-MESSAGES-PRESENT: :[[@LINE-2]]:7: warning: isa_and_present<> is preferred + // CHECK-FIXES-PRESENT: if (isa_and_present(z->bar())) + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-4]]:7: warning: isa_and_nonnull<> is preferred + // CHECK-FIXES-NO-PRESENT: if (isa_and_nonnull(z->bar())) if (z->bar() && dyn_cast_or_null(z->bar())) return true; - // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred - // CHECK-FIXES: if (isa_and_nonnull(z->bar())) + // CHECK-MESSAGES-PRESENT: :[[@LINE-2]]:7: warning: isa_and_present<> is preferred + // CHECK-FIXES-PRESENT: if (isa_and_present(z->bar())) + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-4]]:7: warning: isa_and_nonnull<> is preferred + // CHECK-FIXES-NO-PRESENT: if (isa_and_nonnull(z->bar())) + + if (z->bar() && dyn_cast_if_present(z->bar())) + return true; + // CHECK-MESSAGES-PRESENT: :[[@LINE-2]]:7: warning: isa_and_present<> is preferred + // CHECK-FIXES-PRESENT: if (isa_and_present(z->bar())) + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-4]]:7: warning: isa_and_nonnull<> is preferred + // CHECK-FIXES-NO-PRESENT: if (isa_and_nonnull(z->bar())) bool b = z->bar() && cast(z->bar()); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: isa_and_nonnull<> is preferred - // CHECK-FIXES: bool b = isa_and_nonnull(z->bar()); + // CHECK-MESSAGES-PRESENT: :[[@LINE-1]]:12: warning: isa_and_present<> is preferred + // CHECK-FIXES-PRESENT: bool b = isa_and_present(z->bar()); + // CHECK-MESSAGES-NO-PRESENT: :[[@LINE-3]]:12: warning: isa_and_nonnull<> is preferred + // CHECK-FIXES-NO-PRESENT: bool b = isa_and_nonnull(z->bar()); // These don't trigger a warning. if (auto x = cast(y)->foo())