Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditio…
…nals Summary: 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 cases like these: if (auto x = cast<X>(y)) <...> if (cast<X>(y)) <...> // But not cases like these: if (auto f = cast<Z>(y)->foo()) <...> if (cast<Z>(y)->foo()) <...> Reviewers: alexfh, rjmccall, hokein, aaron.ballman, JonasToth Reviewed By: aaron.ballman Subscribers: xbolva00, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits Tags: #clang-tools-extra, #clang Differential Revision: https://reviews.llvm.org/D59802 llvm-svn: 359142
- Loading branch information
Showing
8 changed files
with
379 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
135 changes: 135 additions & 0 deletions
135
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
64 changes: 64 additions & 0 deletions
64
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
...ls-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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)) {} |
Oops, something went wrong.