This revision was automatically updated to reflect the committed changes. Closed by commit rL359142: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals (authored by dhinton, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.
Changed prior to commit: https://reviews.llvm.org/D59802?vs=195270&id=196522#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h clang-tools-extra/trunk/docs/ReleaseNotes.rst clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
Index: clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp @@ -0,0 +1,135 @@ +//===--- PreferIsaOrDynCastInConditionalsCheck.cpp - clang-tidy +//---------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "PreferIsaOrDynCastInConditionalsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace ast_matchers { +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers + +namespace tidy { +namespace llvm { + +void PreferIsaOrDynCastInConditionalsCheck::registerMatchers( + MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + auto Condition = hasCondition(implicitCastExpr(has( + callExpr( + allOf(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(allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), + callee(namedDecl(hasName("cast"))))) + .bind("assign")))))), + Condition); + + auto CallExpression = + callExpr( + allOf(unless(isMacroID()), unless(cxxMemberCallExpr()), + allOf(callee(namedDecl(anyOf(hasName("isa"), hasName("cast"), + hasName("cast_or_null"), + hasName("dyn_cast"), + hasName("dyn_cast_or_null"))) + .bind("func")), + hasArgument(0, anyOf(declRefExpr().bind("arg"), + cxxMemberCallExpr().bind("arg")))))) + .bind("rhs"); + + Finder->addMatcher( + stmt(anyOf(ifStmt(Any), whileStmt(Any), doStmt(Condition), + binaryOperator( + allOf(unless(isExpansionInFileMatching( + "llvm/include/llvm/Support/Casting.h")), + hasOperatorName("&&"), + hasLHS(implicitCastExpr().bind("lhs")), + hasRHS(anyOf(implicitCastExpr(has(CallExpression)), + CallExpression)))) + .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); + + diag(MatchedDecl->getBeginLoc(), + "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); + + StringRef Message = + "cast<> in conditional will assert rather than return a null pointer"; + if (Result.Nodes.getNodeAs<NamedDecl>("dyn_cast")) + Message = "return value from dyn_cast<> not used"; + + diag(MatchedDecl->getBeginLoc(), Message) + << FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc), "isa"); + } else if (const auto *MatchedDecl = + 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())); + + StringRef ArgString(Lexer::getSourceText( + CharSourceRange::getTokenRange(Arg->getSourceRange()), + *Result.SourceManager, getLangOpts())); + + 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()); + + diag(MatchedDecl->getBeginLoc(), + "isa_and_nonnull<> is preferred over an explicit test for null " + "followed by calling isa<>") + << FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(), + MatchedDecl->getEndLoc()), + Replacement); + } +} + +} // namespace llvm +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt @@ -4,6 +4,7 @@ HeaderGuardCheck.cpp IncludeOrderCheck.cpp LLVMTidyModule.cpp + PreferIsaOrDynCastInConditionalsCheck.cpp TwineLocalCheck.cpp LINK_LIBS Index: clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp @@ -12,6 +12,7 @@ #include "../readability/NamespaceCommentCheck.h" #include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" +#include "PreferIsaOrDynCastInConditionalsCheck.h" #include "TwineLocalCheck.h" namespace clang { @@ -25,6 +26,8 @@ CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order"); CheckFactories.registerCheck<readability::NamespaceCommentCheck>( "llvm-namespace-comment"); + CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>( + "llvm-prefer-isa-or-dyn-cast-in-conditionals"); CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local"); } }; Index: clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h +++ clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h @@ -0,0 +1,64 @@ +//===--- PreferIsaOrDynCastInConditionalsCheck.h - clang-tidy ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDCASTINCONDITIONALCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDCASTINCONDITIONALCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace llvm { + +/// \brief Looks at conditionals and finds and replaces cases of ``cast<>``, which will +/// assert rather than return a null pointer, and ``dyn_cast<>`` where +/// the return value is not captured. Additionally, finds and replaces cases that match the +/// pattern ``var && isa<X>(var)``, where ``var`` is evaluated twice. +/// +/// Finds cases like these: +/// \code +/// if (auto x = cast<X>(y)) {} +/// // is replaced by: +/// if (auto x = dyn_cast<X>(y)) {} +/// +/// if (cast<X>(y)) {} +/// // is replaced by: +/// if (isa<X>(y)) {} +/// +/// if (dyn_cast<X>(y)) {} +/// // is replaced by: +/// if (isa<X>(y)) {} +/// +/// if (var && isa<T>(var)) {} +/// // is replaced by: +/// if (isa_and_nonnull<T>(var.foo())) {} +/// \endcode +/// +/// // Other cases are ignored, e.g.: +/// \code +/// if (auto f = cast<Z>(y)->foo()) {} +/// if (cast<Z>(y)->foo()) {} +/// if (X.cast(y)) {} +/// \endcode +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.html +class PreferIsaOrDynCastInConditionalsCheck : public ClangTidyCheck { +public: + PreferIsaOrDynCastInConditionalsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace llvm +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDCASTINCONDITIONALCHECK_H Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -142,6 +142,15 @@ <clang-tidy/checks/modernize-use-override>` now supports `OverrideSpelling` and `FinalSpelling` options. +- New :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals + <clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals>` check. + + Looks at conditionals and finds and replaces cases of ``cast<>``, + which will assert rather than return a null pointer, and + ``dyn_cast<>`` where the return value is not captured. Additionally, + finds and replaces cases that match the pattern ``var && + isa<X>(var)``, where ``var`` is evaluated twice. + - New :doc:`openmp-exception-escape <clang-tidy/checks/openmp-exception-escape>` check. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst @@ -0,0 +1,34 @@ +.. title:: clang-tidy - llvm-prefer-isa-or-dyn-cast-in-conditionals + +llvm-prefer-isa-or-dyn-cast-in-conditionals +=========================================== + +Looks at conditionals and finds and replaces cases of ``cast<>``, +which will assert rather than return a null pointer, and +``dyn_cast<>`` where the return value is not captured. Additionally, +finds and replaces cases that match the pattern ``var && +isa<X>(var)``, where ``var`` is evaluated twice. + +.. code-block:: c++ + + // Finds these: + if (auto x = cast<X>(y)) {} + // is replaced by: + if (auto x = dyn_cast<X>(y)) {} + + if (cast<X>(y)) {} + // is replaced by: + if (isa<X>(y)) {} + + if (dyn_cast<X>(y)) {} + // is replaced by: + if (isa<X>(y)) {} + + if (var && isa<T>(var)) {} + // is replaced by: + if (isa_and_nonnull<T>(var.foo())) {} + + // Other cases are ignored, e.g.: + if (auto f = cast<Z>(y)->foo()) {} + if (cast<Z>(y)->foo()) {} + if (X.cast(y)) {} Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -178,6 +178,7 @@ llvm-header-guard llvm-include-order llvm-namespace-comment + llvm-prefer-isa-or-dyn-cast-in-conditionals llvm-twine-local misc-definitions-in-headers misc-misplaced-const Index: clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp +++ clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp @@ -0,0 +1,132 @@ +// RUN: %check_clang_tidy %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t + +struct X; +struct Y; +struct Z { + int foo(); + X *bar(); + X *cast(Y*); + bool baz(Y*); +}; + +template <class X, class Y> +bool isa(Y *); +template <class X, class Y> +X *cast(Y *); +template <class X, class Y> +X *dyn_cast(Y *); +template <class X, class Y> +X *dyn_cast_or_null(Y *); + +bool foo(Y *y, Z *z) { + if (auto x = cast<X>(y)) + return true; + // 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)) + + while (auto x = cast<X>(y)) + break; + // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional + // CHECK-FIXES: while (auto x = dyn_cast<X>(y)) + + if (cast<X>(y)) + return true; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: cast<> in conditional + // CHECK-FIXES: if (isa<X>(y)) + + while (cast<X>(y)) + break; + // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional + // CHECK-FIXES: while (isa<X>(y)) + + do { + break; + } while (cast<X>(y)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast<> in conditional + // CHECK-FIXES: while (isa<X>(y)); + + if (dyn_cast<X>(y)) + return true; + // 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)) + + while (dyn_cast<X>(y)) + break; + // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used + // CHECK-FIXES: while (isa<X>(y)) + + do { + break; + } while (dyn_cast<X>(y)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return value from dyn_cast<> not used + // CHECK-FIXES: while (isa<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 (z->bar() && isa<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<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 + // CHECK-FIXES: if (isa_and_nonnull<Y>(z->bar())) + + if (z->bar() && dyn_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())) + + 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()); + + // These don't trigger a warning. + if (auto x = cast<Z>(y)->foo()) + return true; + if (auto x = z->cast(y)) + return true; + while (auto x = cast<Z>(y)->foo()) + break; + if (cast<Z>(y)->foo()) + return true; + if (z->cast(y)) + return true; + while (cast<Z>(y)->foo()) + break; + if (y && cast<X>(z->bar())) + return true; + if (z && cast<Z>(y)->foo()) + return true; + bool b2 = y && cast<X>(z); + if(z->cast(y)) + return true; + if (z->baz(cast<Y>(z))) + return true; + +#define CAST(T, Obj) cast<T>(Obj) +#define AUTO_VAR_CAST(X, Y, Z) auto X = cast<Y>(Z) +#define ISA(T, Obj) isa<T>(Obj) +#define ISA_OR_NULL(T, Obj) Obj &&isa<T>(Obj) + + // Macros don't trigger warning. + if (auto x = CAST(X, y)) + return true; + if (AUTO_VAR_CAST(x, X, z)) + return true; + if (z->bar() && ISA(Y, z->bar())) + return true; + if (ISA_OR_NULL(Y, z->bar())) + return true; + + return false; +}
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
