Index: clang-tidy/abseil/AbseilTidyModule.cpp =================================================================== --- clang-tidy/abseil/AbseilTidyModule.cpp +++ clang-tidy/abseil/AbseilTidyModule.cpp @@ -13,6 +13,7 @@ #include "DurationDivisionCheck.h" #include "DurationFactoryFloatCheck.h" #include "FasterStrsplitDelimiterCheck.h" +#include "MakeUniqueCheck.h" #include "NoInternalDependenciesCheck.h" #include "NoNamespaceCheck.h" #include "RedundantStrcatCallsCheck.h" @@ -32,6 +33,8 @@ "abseil-duration-factory-float"); CheckFactories.registerCheck( "abseil-faster-strsplit-delimiter"); + CheckFactories.registerCheck( + "abseil-make-unique"); CheckFactories.registerCheck( "abseil-no-internal-dependencies"); CheckFactories.registerCheck("abseil-no-namespace"); Index: clang-tidy/abseil/CMakeLists.txt =================================================================== --- clang-tidy/abseil/CMakeLists.txt +++ clang-tidy/abseil/CMakeLists.txt @@ -5,6 +5,7 @@ DurationDivisionCheck.cpp DurationFactoryFloatCheck.cpp FasterStrsplitDelimiterCheck.cpp + MakeUniqueCheck.cpp NoInternalDependenciesCheck.cpp NoNamespaceCheck.cpp RedundantStrcatCallsCheck.cpp Index: clang-tidy/abseil/MakeUniqueCheck.h =================================================================== --- /dev/null +++ clang-tidy/abseil/MakeUniqueCheck.h @@ -0,0 +1,42 @@ +//===--- MakeUniqueCheck.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_ABSEIL_MAKEUNIQUECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_MAKEUNIQUECHECK_H + +#include "../ClangTidy.h" +#include "../modernize/MakeSmartPtrCheck.h" + +namespace clang { +namespace tidy { +namespace abseil { + +/// Replace the pattern: +/// \code +/// std::unique_ptr(new type(args...)) +/// \endcode +/// +/// With the Abseil version: +/// \code +/// absl::make_unique(args...) +/// \endcode +class MakeUniqueCheck : public modernize::MakeSmartPtrCheck { +public: + MakeUniqueCheck(StringRef Name, ClangTidyContext *Context); + +protected: + SmartPtrTypeMatcher getSmartPointerTypeMatcher() const override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; +}; + +} // namespace abseil +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_MAKEUNIQUECHECK_H Index: clang-tidy/abseil/MakeUniqueCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/abseil/MakeUniqueCheck.cpp @@ -0,0 +1,48 @@ +//===--- MakeUniqueCheck.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 "MakeUniqueCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace abseil { + +MakeUniqueCheck::MakeUniqueCheck(StringRef Name, + clang::tidy::ClangTidyContext *Context) + : MakeSmartPtrCheck(Name, Context, "absl::make_unique") {} + +MakeUniqueCheck::SmartPtrTypeMatcher +MakeUniqueCheck::getSmartPointerTypeMatcher() const { + return qualType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::unique_ptr"), templateArgumentCountIs(2), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType().bind(PointerType)))), + hasTemplateArgument( + 1, templateArgument(refersToType( + qualType(hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::default_delete"), + templateArgumentCountIs(1), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType( + equalsBoundNode(PointerType)))))))))))))))); +} + +bool MakeUniqueCheck::isLanguageVersionSupported( + const LangOptions &LangOpts) const { + return LangOpts.CPlusPlus11; +} + +} // namespace abseil +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -87,6 +87,12 @@ Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the delimiter is a single character string literal and replaces with a character. +- New :doc:`abseil-make-unique + ` check. + + Checks for instances of initializing a `unique_ptr` with a direct call to + `new` and suggests using `absl::make_unique` instead. + - New :doc:`abseil-no-internal-dependencies ` check. Index: docs/clang-tidy/checks/abseil-make-unique.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/abseil-make-unique.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - abseil-make-unique + +abseil-make-unique +================== + +Checks for instances of initializing a `unique_ptr` with a direct call to +`new` and suggests using `absl::make_unique` instead. + +Replaces initialization of smart pointers: +\code + std::unique_ptr ptr = std::unique_ptr(new int(1)); +\endcode + +with the preferred usage: +\code + std::unique_ptr ptr = absl::make_unique(1); +\endcode + +per the Abseil tips and guidelines at https://abseil.io/tips/126. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -7,11 +7,12 @@ abseil-duration-division abseil-duration-factory-float abseil-faster-strsplit-delimiter + abseil-make-unique abseil-no-internal-dependencies abseil-no-namespace abseil-redundant-strcat-calls - abseil-string-find-startswith abseil-str-cat-append + abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 android-cloexec-creat @@ -151,6 +152,7 @@ hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) hicpp-static-assert (redirects to misc-static-assert) hicpp-undelegated-constructor (redirects to bugprone-undelegated-constructor) + hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) hicpp-use-auto (redirects to modernize-use-auto) hicpp-use-emplace (redirects to modernize-use-emplace) hicpp-use-equals-default (redirects to modernize-use-equals-default) @@ -159,7 +161,6 @@ hicpp-use-nullptr (redirects to modernize-use-nullptr) hicpp-use-override (redirects to modernize-use-override) hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) - hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) llvm-header-guard llvm-include-order llvm-namespace-comment Index: test/clang-tidy/abseil-make-unique.cpp =================================================================== --- /dev/null +++ test/clang-tidy/abseil-make-unique.cpp @@ -0,0 +1,130 @@ +// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \ + +// CHECK-FIXES: #include + +namespace std { + +template +class default_delete {}; + +template > +class unique_ptr { +public: + unique_ptr() {} + unique_ptr(type *ptr) {} + unique_ptr(const unique_ptr &t) = delete; + unique_ptr(unique_ptr &&t) {} + ~unique_ptr() {} + type &operator*() { return *ptr; } + type *operator->() { return ptr; } + type *release() { return ptr; } + void reset() {} + void reset(type *pt) {} + void reset(type pt) {} + unique_ptr &operator=(unique_ptr &&) { return *this; } + template + unique_ptr &operator=(unique_ptr &&) { return *this; } + +private: + type *ptr; +}; + +} // namespace std + +class A { + int x; + int y; + + public: + A(int _x, int _y): x(_x), y(_y) {} +}; + +struct Base { + Base(); +}; + +struct Derived : public Base { + Derived(); +}; + +int* returnPointer(); +void expectPointer(std::unique_ptr p); + +std::unique_ptr makeAndReturnPointer() { + // Make smart pointer in return statement + return std::unique_ptr(new int(0)); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: return absl::make_unique(0); +} + +void Positives() { + std::unique_ptr P1 = std::unique_ptr(new int(1)); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1); + + P1.reset(new int(2)); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: P1 = absl::make_unique(2); + + // Non-primitive paramter + std::unique_ptr P2 = std::unique_ptr(new A(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2); + + P2.reset(new A(3, 4)); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: P2 = absl::make_unique(3, 4); + + // No arguments to new expression + std::unique_ptr P3 = std::unique_ptr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique(); + + P3.reset(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: P3 = absl::make_unique(); + + // Nested parentheses + std::unique_ptr P4 = std::unique_ptr((new int(3))); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3); + + P4 = std::unique_ptr(((new int(4)))); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: P4 = absl::make_unique(4); + + P4.reset((new int(5))); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: P4 = absl::make_unique(5); + + // With auto + auto P5 = std::unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: auto P5 = absl::make_unique(); + + { + // No std + using namespace std; + unique_ptr Q = unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: unique_ptr Q = absl::make_unique(); + + Q = unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: Q = absl::make_unique(); + } + + // Create the unique_ptr as a parameter to a function + expectPointer(std::unique_ptr(new int())); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique] + // CHECK-FIXES: expectPointer(absl::make_unique()); +} + +void Negatives() { + // Only warn if explicitly allocating a new object + std::unique_ptr R = std::unique_ptr(returnPointer()); + R.reset(returnPointer()); + + // Only replace if the template type is same as new type + auto Pderived = std::unique_ptr(new Derived()); +}