hintonda updated this revision to Diff 192416.
hintonda added a comment.
- Added dyn_cast and dyn_cast_or_null, and provided fixit's for all of them.
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/AvoidCastInConditionalCheck.cpp
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
clang/include/clang/Basic/LLVM.h
Index: clang/include/clang/Basic/LLVM.h
===================================================================
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
template<typename T> class ArrayRef;
template<typename T> class MutableArrayRef;
template<typename T> class OwningArrayRef;
+ template <typename T, unsigned N, typename C> class SmallSet;
template<unsigned InternalLen> class SmallString;
template<typename T, unsigned N> class SmallVector;
template<typename T> class SmallVectorImpl;
@@ -66,6 +67,7 @@
using llvm::Optional;
using llvm::OwningArrayRef;
using llvm::SaveAndRestore;
+ using llvm::SmallSet;
using llvm::SmallString;
using llvm::SmallVector;
using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,102 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %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 *LongVarName) {
+ if (auto x = cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: if (auto x = dyn_cast<X>(y))
+
+ if (cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: if (isa<X>(y))
+
+ if (dyn_cast<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: if (isa<X>(y))
+
+ if (dyn_cast_or_null<X>(y))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: {{.*dyn_cast_or_null<> not used.*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: if (y && isa<X>(y))
+
+ if (dyn_cast_or_null<X>(LongVarName->bar()))
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: {{.*dyn_cast_or_null<> not used.*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: if (LongVarName->bar() && isa<X>(LongVarName->bar()))
+
+ // These don't trigger a warning.
+ if (auto x = cast<Z>(y)->foo())
+ return true;
+ if (cast<Z>(y)->foo())
+ return true;
+
+ while (auto x = cast<X>(y))
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (auto x = dyn_cast<X>(y))
+
+ while (cast<X>(y))
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (isa<X>(y))
+
+ while (dyn_cast<X>(y))
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (isa<X>(y))
+
+ while (dyn_cast_or_null<X>(y))
+ break;
+ // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (y && isa<X>(y))
+
+ // These don't trigger a warning.
+ while (auto x = cast<Z>(y)->foo())
+ break;
+ while (cast<Z>(y)->foo())
+ break;
+
+ do {
+ break;
+ } while (cast<X>(y));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (isa<X>(y));
+
+ do {
+ break;
+ } while (dyn_cast<X>(y));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (isa<X>(y));
+
+ do {
+ break;
+ } while (dyn_cast_or_null<X>(y));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (y && isa<X>(y))
+
+ do {
+ break;
+ } while (dyn_cast_or_null<X>(LongVarName->bar()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+ // CHECK-FIXES: while (LongVarName->bar() && isa<X>(LongVarName->bar()))
+
+ return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==============================
+
+ Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+ ``do`` statements, which will assert rather than return a null
+ pointer. It also finds uses of ``dyn_cast<>`` and
+ ``dyn_cast_or_null`` in conditionals where the return value is
+ not captured.
+
+.. 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 (dyn_cast_or_null<X>(LongVarName->bar())) {}
+ // is replaced by:
+ if (LongVarName->bar() && isa<X>(LongVarName->bar())) {}
+
+ // These are ignored.
+ 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
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) <hicpp-use-nullptr>
hicpp-use-override (redirects to modernize-use-override) <hicpp-use-override>
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) <hicpp-vararg>
+ llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
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-avoid-cast-in-conditional
+ <clang-tidy/checks/llvm-avoid-cast-in-conditional>` check.
+
+ Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+ ``do`` statements, which will assert rather than return a null
+ pointer. It also finds uses of ``dyn_cast<>`` and
+ ``dyn_cast_or_null`` in conditionals where the return value is not
+ captured.
+
- New :doc:`openmp-exception-escape
<clang-tidy/checks/openmp-exception-escape>` check.
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
namespace tidy {
namespace utils {
-typedef llvm::SmallSet<llvm::StringRef, 5> HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet<StringRef, 5>;
/// \brief Checks whether expansion location of \p Loc is in header file.
bool isExpansionLocInHeaderFile(
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
@@ -10,6 +10,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "../readability/NamespaceCommentCheck.h"
+#include "AvoidCastInConditionalCheck.h"
#include "HeaderGuardCheck.h"
#include "IncludeOrderCheck.h"
#include "TwineLocalCheck.h"
@@ -21,6 +22,8 @@
class LLVMModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<AvoidCastInConditionalCheck>(
+ "llvm-avoid-cast-in-conditional");
CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
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
@@ -1,6 +1,7 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyLLVMModule
+ AvoidCastInConditionalCheck.cpp
HeaderGuardCheck.cpp
IncludeOrderCheck.cpp
LLVMTidyModule.cpp
Index: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
@@ -0,0 +1,63 @@
+//===--- AvoidCastInConditionalCheck.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 "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm {
+
+/// \brief Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+/// ``do`` statements, which will assert rather than return a null
+/// pointer. It also finds uses of ``dyn_cast<>`` and
+/// ``dyn_cast_or_null`` in conditionals where the return value is
+/// not captured.
+///
+/// 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 (dyn_cast_or_null<X>(LongVarName->bar())) {}
+/// // is replaced by:
+/// if (LongVarName->bar() && isa<X>(LongVarName->bar())) {}
+/// \endcode
+///
+/// // These are ignored.
+/// \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-avoid-cast-in-conditional.html
+class AvoidCastInConditionalCheck : public ClangTidyCheck {
+public:
+ AvoidCastInConditionalCheck(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/AvoidCastInConditionalCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
@@ -0,0 +1,117 @@
+//===--- AvoidCastInConditionalCheck.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 "AvoidCastInConditionalCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm {
+
+void AvoidCastInConditionalCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ Finder->addMatcher(
+ ifStmt(anyOf(
+ hasConditionVariableStatement(declStmt(containsDeclaration(
+ 0, varDecl(
+ hasInitializer(callExpr(callee(namedDecl(hasName("cast"))))
+ .bind("cast-assign")))))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("cast")))).bind("cast")))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("dyn_cast"))))
+ .bind("dyn_cast")))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("dyn_cast_or_null"))))
+ .bind("dyn_cast_or_null")))))),
+ this);
+
+ Finder->addMatcher(
+ whileStmt(anyOf(
+ has(declStmt(containsDeclaration(
+ 0, varDecl(
+ hasInitializer(callExpr(callee(namedDecl(hasName("cast"))))
+ .bind("cast-assign")))))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("cast")))).bind("cast")))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("dyn_cast"))))
+ .bind("dyn_cast")))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("dyn_cast_or_null"))))
+ .bind("dyn_cast_or_null")))))),
+ this);
+
+ Finder->addMatcher(
+ doStmt(anyOf(
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("cast")))).bind("cast")))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("dyn_cast"))))
+ .bind("dyn_cast")))),
+ hasCondition(implicitCastExpr(
+ has(callExpr(callee(namedDecl(hasName("dyn_cast_or_null"))))
+ .bind("dyn_cast_or_null")))))),
+ this);
+}
+
+void AvoidCastInConditionalCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CallExpr>("cast-assign")) {
+ auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+ auto 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")) {
+ auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+ auto 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), "isa");
+ } else if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CallExpr>("dyn_cast")) {
+ auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+ auto EndLoc = StartLoc.getLocWithOffset(StringRef("dyn_cast").size() - 1);
+
+ diag(MatchedDecl->getBeginLoc(), "return value from dyn_cast<> not used")
+ << FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc), "isa");
+ } else if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CallExpr>("dyn_cast_or_null")) {
+ auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+ auto EndLoc =
+ StartLoc.getLocWithOffset(StringRef("dyn_cast_or_null").size() - 1);
+
+ assert(MatchedDecl->getNumArgs() == 1 &&
+ "Too many args for dyn_cast_or_null<>()");
+ auto Arg = MatchedDecl->getArg(0);
+ auto CRange = CharSourceRange::getTokenRange(Arg->getSourceRange());
+ std::string Replacement(
+ Lexer::getSourceText(CRange, *Result.SourceManager, getLangOpts()));
+ Replacement += " && isa";
+
+ diag(MatchedDecl->getBeginLoc(),
+ "return value from dyn_cast_or_null<> not used")
+ << FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc),
+ Replacement);
+ }
+}
+
+} // namespace llvm
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
