[clang-tools-extra] r205854 - Extend the check to detect patterns like 'ptr.get() == nullptr'
Samuel Benzaquen
sbenza at google.com
Wed Apr 9 07:17:23 PDT 2014
Author: sbenza
Date: Wed Apr 9 09:17:23 2014
New Revision: 205854
URL: http://llvm.org/viewvc/llvm-project?rev=205854&view=rev
Log:
Extend the check to detect patterns like 'ptr.get() == nullptr'
Summary:
Extend the check to detect patterns like 'ptr.get() == nullptr'
It detects == and != when any argument is a ptr.get() and the other is a
nullptr.
Only supports standard smart pointer types std::unique_ptr and std::shared_ptr.
Does not support the case 'ptr.get() == other.get()' yet.
Reviewers: djasper
CC: cfe-commits, alexfh
Differential Revision: http://llvm-reviews.chandlerc.com/D3294
Added:
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh (with props)
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_output.sh (with props)
Modified:
clang-tools-extra/trunk/clang-tidy/misc/RedundantSmartptrGet.cpp
clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get-fix.cpp
clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/RedundantSmartptrGet.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/RedundantSmartptrGet.cpp?rev=205854&r1=205853&r2=205854&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantSmartptrGet.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantSmartptrGet.cpp Wed Apr 9 09:17:23 2014
@@ -16,8 +16,19 @@ using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
-void RedundantSmartptrGet::registerMatchers(MatchFinder *Finder) {
+namespace {
+internal::Matcher<Expr> callToGet(internal::Matcher<Decl> OnClass) {
+ return memberCallExpr(
+ on(expr(anyOf(hasType(OnClass),
+ hasType(qualType(pointsTo(decl(OnClass).bind(
+ "ptr_to_ptr")))))).bind("smart_pointer")),
+ callee(methodDecl(hasName("get")))).bind("redundant_get");
+}
+
+void registerMatchersForGetArrowStart(MatchFinder *Finder,
+ MatchFinder::MatchCallback *Callback) {
const auto QuacksLikeASmartptr = recordDecl(
+ recordDecl().bind("duck_typing"),
has(methodDecl(hasName("operator->"),
returns(qualType(pointsTo(type().bind("op->Type")))))),
has(methodDecl(hasName("operator*"),
@@ -25,35 +36,50 @@ void RedundantSmartptrGet::registerMatch
has(methodDecl(hasName("get"),
returns(qualType(pointsTo(type().bind("getType")))))));
- const auto CallToGet =
- memberCallExpr(on(expr(hasType(recordDecl(QuacksLikeASmartptr)))
- .bind("smart_pointer")),
- callee(methodDecl(hasName("get")))).bind("redundant_get");
-
- const auto ArrowCallToGet =
- memberCallExpr(
- on(expr(hasType(qualType(pointsTo(recordDecl(QuacksLikeASmartptr)))))
- .bind("smart_pointer")),
- callee(methodDecl(hasName("get")))).bind("redundant_get");
-
// Catch 'ptr.get()->Foo()'
- Finder->addMatcher(
- memberExpr(isArrow(), hasObjectExpression(ignoringImpCasts(CallToGet))),
- this);
+ Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(),
+ hasObjectExpression(ignoringImpCasts(
+ callToGet(QuacksLikeASmartptr)))),
+ Callback);
- // Catch '*ptr.get()'
+ // Catch '*ptr.get()' or '*ptr->get()'
Finder->addMatcher(
- unaryOperator(hasOperatorName("*"), hasUnaryOperand(CallToGet)), this);
+ unaryOperator(hasOperatorName("*"),
+ hasUnaryOperand(callToGet(QuacksLikeASmartptr))),
+ Callback);
+}
+
+void registerMatchersForGetEquals(MatchFinder *Finder,
+ MatchFinder::MatchCallback *Callback) {
+ // This one is harder to do with duck typing.
+ // The operator==/!= that we are looking for might be member or non-member,
+ // might be on global namespace or found by ADL, might be a template, etc.
+ // For now, lets keep a list of known standard types.
- // Catch '*ptr->get()'
+ const auto IsAKnownSmartptr = recordDecl(
+ anyOf(hasName("::std::unique_ptr"), hasName("::std::shared_ptr")));
+
+ // Matches against nullptr.
Finder->addMatcher(
- unaryOperator(hasOperatorName("*"), hasUnaryOperand(ArrowCallToGet))
- .bind("ptr_to_ptr"),
- this);
+ binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ hasEitherOperand(ignoringImpCasts(nullPtrLiteralExpr())),
+ hasEitherOperand(callToGet(IsAKnownSmartptr))),
+ Callback);
+ // TODO: Catch ptr.get() == other_ptr.get()
+}
+
+
+} // namespace
+
+void RedundantSmartptrGet::registerMatchers(MatchFinder *Finder) {
+ registerMatchersForGetArrowStart(Finder, this);
+ registerMatchersForGetEquals(Finder, this);
}
namespace {
bool allReturnTypesMatch(const MatchFinder::MatchResult &Result) {
+ if (Result.Nodes.getNodeAs<Decl>("duck_typing") == nullptr)
+ return true;
// Verify that the types match.
// We can't do this on the matcher because the type nodes can be different,
// even though they represent the same type. This difference comes from how
@@ -71,14 +97,20 @@ bool allReturnTypesMatch(const MatchFind
void RedundantSmartptrGet::check(const MatchFinder::MatchResult &Result) {
if (!allReturnTypesMatch(Result)) return;
- bool IsPtrToPtr = Result.Nodes.getNodeAs<Expr>("ptr_to_ptr") != nullptr;
+ bool IsPtrToPtr = Result.Nodes.getNodeAs<Decl>("ptr_to_ptr") != nullptr;
+ bool IsMemberExpr = Result.Nodes.getNodeAs<Expr>("memberExpr") != nullptr;
const Expr *GetCall = Result.Nodes.getNodeAs<Expr>("redundant_get");
const Expr *Smartptr = Result.Nodes.getNodeAs<Expr>("smart_pointer");
+ if (IsPtrToPtr && IsMemberExpr) {
+ // Ignore this case (eg. Foo->get()->DoSomething());
+ return;
+ }
+
StringRef SmartptrText = Lexer::getSourceText(
CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
*Result.SourceManager, LangOptions());
- // Replace *foo->get() with **foo, and foo.get() with foo.
+ // Replace foo->get() with *foo, and foo.get() with foo.
std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str();
diag(GetCall->getLocStart(), "Redundant get() call on smart pointer.")
<< FixItHint::CreateReplacement(GetCall->getSourceRange(), Replacement);
Added: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh?rev=205854&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh (added)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh Wed Apr 9 09:17:23 2014
@@ -0,0 +1,12 @@
+#!/bin/sh
+#
+# Run clang-tidy in fix mode and verify the result.
+
+INPUT_FILE=$1
+CHECK_TO_RUN=$2
+TEMPORARY_FILE=$3.cpp
+
+grep -Ev "// *[A-Z-]+:" ${INPUT_FILE} > ${TEMPORARY_FILE}
+clang-tidy ${TEMPORARY_FILE} -fix --checks=${CHECK_TO_RUN} \
+ --disable-checks="" -- --std=c++11
+FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE}
Propchange: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
------------------------------------------------------------------------------
svn:executable = *
Added: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_output.sh
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_output.sh?rev=205854&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_output.sh (added)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_output.sh Wed Apr 9 09:17:23 2014
@@ -0,0 +1,10 @@
+#!/bin/sh
+#
+# Run clang-tidy and verify its command line output.
+
+INPUT_FILE=$1
+CHECK_TO_RUN=$2
+
+clang-tidy --checks=${CHECK_TO_RUN} --disable-checks="" ${INPUT_FILE} \
+ -- --std=c++11 \
+ | FileCheck ${INPUT_FILE}
Propchange: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_output.sh
------------------------------------------------------------------------------
svn:executable = *
Modified: clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get-fix.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get-fix.cpp?rev=205854&r1=205853&r2=205854&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get-fix.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get-fix.cpp Wed Apr 9 09:17:23 2014
@@ -1,6 +1,6 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: clang-tidy %t.cpp -fix -checks=misc-redundant-smartptr-get --
-// RUN: FileCheck -input-file=%t.cpp %s
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-redundant-smartptr-get %t
+
+#include <memory>
struct Bar {
void Do();
@@ -39,4 +39,11 @@ void Positive() {
int_ptr ip;
int i = *ip.get();
// CHECK: int i = *ip;
+
+ std::unique_ptr<int> uu;
+ std::shared_ptr<double> *ss;
+ bool bb = uu.get() == nullptr;
+ // CHECK: bool bb = uu == nullptr;
+ bb = nullptr != ss->get();
+ // CHECK: bb = nullptr != *ss;
}
Modified: clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get.cpp?rev=205854&r1=205853&r2=205854&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/redundant-smartptr-get.cpp Wed Apr 9 09:17:23 2014
@@ -1,22 +1,8 @@
-// RUN: clang-tidy --checks=misc-redundant-smartptr-get %s -- | FileCheck %s
+// RUN: $(dirname %s)/check_clang_tidy_output.sh %s misc-redundant-smartptr-get
// CHECK-NOT: warning
-namespace std {
-
-template <typename T>
-struct MakeRef {
- typedef T& type;
-};
-
-template <typename T>
-struct unique_ptr {
- T* get();
- T* operator->();
- // This simulates libstdc++'s implementation of unique_ptr.
- typename MakeRef<T>::type operator*();
-};
-} // namespace std
+#include <memory>
struct int_ptr {
int* get();
@@ -65,6 +51,14 @@ void Positive() {
int i = *ip.get();
// CHECK: :[[@LINE-1]]:12: warning: Redundant get() call on smart pointer.
// CHECK: int i = *ip.get();
+
+ bool bb = u.get() == nullptr;
+ // CHECK: :[[@LINE-1]]:13: warning: Redundant get() call on smart pointer.
+ // CHECK: u.get() == nullptr;
+ std::shared_ptr<double> *sp;
+ bb = nullptr != sp->get();
+ // CHECK: :[[@LINE-1]]:19: warning: Redundant get() call on smart pointer.
+ // CHECK: nullptr != sp->get();
}
// CHECK-NOT: warning
@@ -77,4 +71,9 @@ void Negative() {
Fail2().get()->Do();
const Bar& b = *Fail1().get();
(*Fail2().get()).Do();
+
+ int_ptr ip;
+ bool bb = std::unique_ptr<int>().get() == NULL;
+ bb = ip.get() == nullptr;
+ bb = u->get() == NULL;
}
More information about the cfe-commits
mailing list