[clang-tools-extra] r205854 - Extend the check to detect patterns like 'ptr.get() == nullptr'
Samuel Benzaquen
sbenza at google.com
Wed Apr 9 14:09:57 PDT 2014
Revision rL205913 was submitted to fix this some time ago.
Sorry for the break.
_Sam
On Wed, Apr 9, 2014 at 4:47 PM, Aaron Ballman <aaron at aaronballman.com>wrote:
> This appears to have broken one of the build bots:
> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/1540
>
> ~Aaron
>
> On Wed, Apr 9, 2014 at 10:17 AM, Samuel Benzaquen <sbenza at google.com>
> wrote:
> > 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;
> > }
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140409/8614da76/attachment.html>
More information about the cfe-commits
mailing list