hintonda updated this revision to Diff 193379.
hintonda added a comment.
- Remove spaces in docs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59802/new/
https://reviews.llvm.org/D59802
Files:
clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
Index: clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t
+
+struct X;
+struct Y;
+struct Z {
+ int foo();
+ X *bar();
+};
+
+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 (z->bar() && isa<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method '{{.*}}' is called twice and could be expensive [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+ // CHECK-FIXES: if (dyn_cast_or_null<Y>(z->bar()))
+
+ if (z->bar() && cast<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method {{'.*'}} is called twice
+ // CHECK-FIXES: if (dyn_cast_or_null<Y>(z->bar()))
+
+ if (z->bar() && dyn_cast<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method {{'.*'}} is called twice
+ // CHECK-FIXES: if (dyn_cast_or_null<Y>(z->bar()))
+
+ if (z->bar() && dyn_cast_or_null<Y>(z->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method {{'.*'}} is called twice
+ // CHECK-FIXES: if (dyn_cast_or_null<Y>(z->bar()))
+
+ bool b = z->bar() && cast<Y>(z->bar());
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: method {{'.*'}} is called twice
+ // CHECK-FIXES: bool b = dyn_cast_or_null<Y>(z->bar());
+
+ // These don't trigger a warning.
+ if (auto x = cast<Z>(y)->foo())
+ return true;
+ while (auto x = cast<Z>(y)->foo())
+ break;
+ if (cast<Z>(y)->foo())
+ return true;
+ while (cast<Z>(y)->foo())
+ break;
+ if (y && isa<X>(y))
+ return true;
+ if (y && cast<X>(z->bar()))
+ return true;
+ if (z && cast<Z>(y)->foo())
+ return true;
+ bool b2 = y && cast<X>(z);
+
+#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)
+
+ 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;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - llvm-prefer-isa-or-dyn-cast-in-conditionals
+
+llvm-prefer-isa-or-dyn-cast-in-conditionals
+===========================================
+
+Looks at conditionals and finds cases of ``cast<>``, which will assert
+rather than return a null pointer, and ``dyn_cast<>`` where the return
+value is not captured. Additionally, finds cases that match the
+pattern ``var.foo() && isa<X>(var.foo())``, where the method is called
+twice and could be expensive.
+
+.. 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.foo() && isa<T>(var.foo())) {}
+ // is replaced by:
+ if (dyn_cast_or_null<T>(var.foo())) {}
+
+ // Other cases are ignored, e.g.:
+ if (auto f = cast<Z>(y)->foo()) {}
+ if (cast<Z>(y)->foo()) {}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/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/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,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 cases of ``cast<>``, which will
+ assert rather than return a null pointer, and ``dyn_cast<>`` where
+ the return value is not captured. Additionally, finds cases that
+ match the pattern ``var.foo() && isa<X>(var.foo())``, where the
+ method is called twice and could be expensive.
+
- New :doc:`openmp-exception-escape
<clang-tidy/checks/openmp-exception-escape>` check.
Index: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/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 cases of ``cast<>``, which will
+/// assert rather than return a null pointer, and ``dyn_cast<>`` where
+/// the return value is not captured. Additionally, finds cases that match the
+/// pattern ``var.foo() && isa<X>(var.foo())``, where the method is called twice
+/// and could be expensive.
+///
+/// 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.foo() && isa<T>(var.foo())) {}
+/// // is replaced by:
+/// if (dyn_cast_or_null<T>(var.foo())) {}
+/// \endcode
+///
+/// // Other cases are ignored, e.g.:
+/// \code
+/// if (auto f = cast<Z>(y)->foo()) {}
+/// if (cast<Z>(y)->foo()) {}
+/// \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/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
@@ -0,0 +1,154 @@
+//===--- 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;
+
+ Finder->addMatcher(
+ stmt(anyOf(
+ ifStmt(anyOf(
+ has(declStmt(containsDeclaration(
+ 0, varDecl(hasInitializer(
+ callExpr(allOf(unless(isMacroID()),
+ callee(namedDecl(hasName("cast")))))
+ .bind("cast-assign")))))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(allOf(unless(isMacroID()),
+ anyOf(callee(namedDecl(hasName("cast"))),
+ callee(namedDecl(hasName("dyn_cast"))
+ .bind("dyn_cast")))))
+ .bind("cast")))))),
+ whileStmt(anyOf(
+ has(declStmt(containsDeclaration(
+ 0, varDecl(hasInitializer(
+ callExpr(allOf(unless(isMacroID()),
+ callee(namedDecl(hasName("cast")))))
+ .bind("cast-assign")))))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(allOf(unless(isMacroID()),
+ anyOf(callee(namedDecl(hasName("cast"))),
+ callee(namedDecl(hasName("dyn_cast"))
+ .bind("dyn_cast")))))
+ .bind("cast")))))),
+ doStmt(hasCondition(implicitCastExpr(
+ has(callExpr(allOf(unless(isMacroID()),
+ anyOf(callee(namedDecl(hasName("cast"))),
+ callee(namedDecl(hasName("dyn_cast"))
+ .bind("dyn_cast")))))
+ .bind("cast"))))),
+ binaryOperator(
+ allOf(
+ unless(isExpansionInFileMatching(
+ "llvm/include/llvm/Support/Casting.h")),
+ hasOperatorName("&&"),
+ hasLHS(
+ implicitCastExpr(has(cxxMemberCallExpr().bind("lhs")))),
+ hasRHS(anyOf(
+ callExpr(allOf(unless(isMacroID()),
+ allOf(hasArgument(0, cxxMemberCallExpr()
+ .bind("arg")),
+ callee(namedDecl(hasName("isa"))
+ .bind("func")))))
+ .bind("rhs"),
+ implicitCastExpr(has(
+ callExpr(
+ allOf(
+ unless(isMacroID()),
+ allOf(hasArgument(
+ 0, cxxMemberCallExpr().bind("arg")),
+ callee(
+ namedDecl(
+ anyOf(hasName("cast"),
+ hasName("dyn_cast"),
+ hasName(
+ "dyn_cast_or_null")))
+ .bind("func")))))
+ .bind("rhs")))))))
+ .bind("and"))),
+ this);
+}
+
+void PreferIsaOrDynCastInConditionalsCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CallExpr>("cast-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>("cast")) {
+ 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 *Arg = Result.Nodes.getNodeAs<CXXMemberCallExpr>("arg");
+ const auto *LHS = Result.Nodes.getNodeAs<CXXMemberCallExpr>("lhs");
+ const auto *RHS = Result.Nodes.getNodeAs<Expr>("rhs");
+ const auto *Func = Result.Nodes.getNodeAs<NamedDecl>("func");
+
+ CharSourceRange ArgRange =
+ CharSourceRange::getTokenRange(Arg->getSourceRange());
+ std::string ArgString(
+ Lexer::getSourceText(ArgRange, *Result.SourceManager, getLangOpts()));
+
+ CharSourceRange LHSRange =
+ CharSourceRange::getTokenRange(LHS->getSourceRange());
+ std::string LHSString(
+ Lexer::getSourceText(LHSRange, *Result.SourceManager, getLangOpts()));
+
+ CharSourceRange RHSRange =
+ CharSourceRange::getTokenRange(RHS->getSourceRange());
+ std::string RHSString(
+ Lexer::getSourceText(RHSRange, *Result.SourceManager, getLangOpts()));
+
+ std::string Replacement("dyn_cast_or_null");
+ Replacement += RHSString.substr(Func->getName().size());
+
+ diag(MatchedDecl->getBeginLoc(),
+ "method %0 is called twice and could be expensive")
+ << LHS->getMethodDecl()
+ << FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),
+ MatchedDecl->getEndLoc()),
+ Replacement);
+ }
+}
+
+} // namespace llvm
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/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 {
@@ -23,6 +24,8 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
+ CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>(
+ "llvm-prefer-isa-or-dyn-cast-in-conditionals");
CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
"llvm-namespace-comment");
CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
Index: clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -4,6 +4,7 @@
HeaderGuardCheck.cpp
IncludeOrderCheck.cpp
LLVMTidyModule.cpp
+ PreferIsaOrDynCastInConditionalsCheck.cpp
TwineLocalCheck.cpp
LINK_LIBS
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
