diff options
6 files changed, 252 insertions, 3 deletions
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index 8c9b5dd3c0b..3dd97bf0b4f 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyMiscModule ArgumentCommentCheck.cpp MiscTidyModule.cpp + RedundantSmartptrGet.cpp LINK_LIBS clangAST diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index f7f657addfa..b89e2dcad48 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "ArgumentCommentCheck.h" +#include "RedundantSmartptrGet.h" namespace clang { namespace tidy { @@ -18,9 +19,12 @@ namespace tidy { class MiscModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { - CheckFactories.addCheckFactory( - "misc-argument-comment", - new ClangTidyCheckFactory<ArgumentCommentCheck>()); + CheckFactories.addCheckFactory( + "misc-argument-comment", + new ClangTidyCheckFactory<ArgumentCommentCheck>()); + CheckFactories.addCheckFactory( + "misc-redundant-smartptr-get", + new ClangTidyCheckFactory<RedundantSmartptrGet>()); } }; diff --git a/clang-tools-extra/clang-tidy/misc/RedundantSmartptrGet.cpp b/clang-tools-extra/clang-tidy/misc/RedundantSmartptrGet.cpp new file mode 100644 index 00000000000..ec08c142c18 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/RedundantSmartptrGet.cpp @@ -0,0 +1,88 @@ +//===--- RedundantSmartptrGet.cpp - clang-tidy ----------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "RedundantSmartptrGet.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void RedundantSmartptrGet::registerMatchers(MatchFinder *Finder) { + const auto QuacksLikeASmartptr = recordDecl( + has(methodDecl(hasName("operator->"), + returns(qualType(pointsTo(type().bind("op->Type")))))), + has(methodDecl(hasName("operator*"), + returns(qualType(references(type().bind("op*Type")))))), + 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); + + // Catch '*ptr.get()' + Finder->addMatcher( + unaryOperator(hasOperatorName("*"), hasUnaryOperand(CallToGet)), this); + + // Catch '*ptr->get()' + Finder->addMatcher( + unaryOperator(hasOperatorName("*"), hasUnaryOperand(ArrowCallToGet)) + .bind("ptr_to_ptr"), + this); +} + +namespace { +bool allReturnTypesMatch(const MatchFinder::MatchResult &Result) { + // 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 + // the type is referenced (eg. through a typedef, a type trait, etc). + const Type *OpArrowType = + Result.Nodes.getNodeAs<Type>("op->Type")->getUnqualifiedDesugaredType(); + const Type *OpStarType = + Result.Nodes.getNodeAs<Type>("op*Type")->getUnqualifiedDesugaredType(); + const Type *GetType = + Result.Nodes.getNodeAs<Type>("getType")->getUnqualifiedDesugaredType(); + return OpArrowType == OpStarType && OpArrowType == GetType; +} +} // namespace + +void RedundantSmartptrGet::check(const MatchFinder::MatchResult &Result) { + if (!allReturnTypesMatch(Result)) return; + + bool IsPtrToPtr = Result.Nodes.getNodeAs<Expr>("ptr_to_ptr") != nullptr; + const Expr *GetCall = Result.Nodes.getNodeAs<Expr>("redundant_get"); + const Expr *Smartptr = Result.Nodes.getNodeAs<Expr>("smart_pointer"); + + StringRef SmartptrText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Smartptr->getSourceRange()), + *Result.SourceManager, LangOptions()); + // 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); +} + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/misc/RedundantSmartptrGet.h b/clang-tools-extra/clang-tidy/misc/RedundantSmartptrGet.h new file mode 100644 index 00000000000..32268c2dab1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/RedundantSmartptrGet.h @@ -0,0 +1,34 @@ +//===--- RedundantSmartptrGet.h - clang-tidy --------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_SMARTPTR_GET_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_SMARTPTR_GET_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief Find and remove redundant calls to smart pointer's .get() method. +/// +/// Examples: +/// ptr.get()->Foo() ==> ptr->Foo() +/// *ptr.get() ==> *ptr +/// *ptr->get() ==> **ptr +class RedundantSmartptrGet : public ClangTidyCheck { +public: + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANT_SMARTPTR_GET_H + diff --git a/clang-tools-extra/test/clang-tidy/redundant-smartptr-get-fix.cpp b/clang-tools-extra/test/clang-tidy/redundant-smartptr-get-fix.cpp new file mode 100644 index 00000000000..630956e7d9a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/redundant-smartptr-get-fix.cpp @@ -0,0 +1,42 @@ +// 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 + +struct Bar { + void Do(); + void ConstDo() const; +}; +struct BarPtr { + Bar* operator->(); + Bar& operator*(); + Bar* get(); +}; +struct int_ptr { + int* get(); + int* operator->(); + int& operator*(); +}; + +void Positive() { + BarPtr u; + // CHECK: BarPtr u; + BarPtr().get()->Do(); + // CHECK: BarPtr()->Do(); + + u.get()->ConstDo(); + // CHECK: u->ConstDo(); + + Bar& b = *BarPtr().get(); + // CHECK: Bar& b = *BarPtr(); + + (*BarPtr().get()).ConstDo(); + // CHECK: (*BarPtr()).ConstDo(); + + BarPtr* up; + (*up->get()).Do(); + // CHECK: (**up).Do(); + + int_ptr ip; + int i = *ip.get(); + // CHECK: int i = *ip; +} diff --git a/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp b/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp new file mode 100644 index 00000000000..b43e38b3da7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp @@ -0,0 +1,80 @@ +// RUN: clang-tidy --checks=misc-redundant-smartptr-get %s -- | FileCheck %s + +// 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 + +struct int_ptr { + int* get(); + int* operator->(); + int& operator*(); +}; + +struct Bar { + void Do(); + void ConstDo() const; +}; + +struct Fail1 { + Bar* get(); +}; +struct Fail2 { + Bar* get(); + int* operator->(); + int& operator*(); +}; + +void Positive() { + std::unique_ptr<Bar> u; + std::unique_ptr<Bar>().get()->Do(); + // CHECK: :[[@LINE-1]]:3: warning: Redundant get() call on smart pointer. [misc-redundant-smartptr-get] + // CHECK: std::unique_ptr<Bar>().get()->Do(); + + u.get()->ConstDo(); + // CHECK: :[[@LINE-1]]:3: warning: Redundant get() call on smart pointer. + // CHECK: u.get()->ConstDo(); + + Bar& b = *std::unique_ptr<Bar>().get(); + // CHECK: :[[@LINE-1]]:13: warning: Redundant get() call on smart pointer. + // CHECK: Bar& b = *std::unique_ptr<Bar>().get(); + + (*std::unique_ptr<Bar>().get()).ConstDo(); + // CHECK: :[[@LINE-1]]:5: warning: Redundant get() call on smart pointer. + // CHECK: (*std::unique_ptr<Bar>().get()).ConstDo(); + + std::unique_ptr<Bar>* up; + (*up->get()).Do(); + // CHECK: :[[@LINE-1]]:5: warning: Redundant get() call on smart pointer. + // CHECK: (*up->get()).Do(); + + int_ptr ip; + int i = *ip.get(); + // CHECK: :[[@LINE-1]]:12: warning: Redundant get() call on smart pointer. + // CHECK: int i = *ip.get(); +} + +// CHECK-NOT: warning + +void Negative() { + std::unique_ptr<Bar>* u; + u->get()->Do(); + + Fail1().get()->Do(); + Fail2().get()->Do(); + const Bar& b = *Fail1().get(); + (*Fail2().get()).Do(); +} |
