Index: clang-tidy/abseil/AbseilTidyModule.cpp =================================================================== --- clang-tidy/abseil/AbseilTidyModule.cpp +++ clang-tidy/abseil/AbseilTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "AutoMakeUniqueCheck.h" #include "StringFindStartswithCheck.h" namespace clang { @@ -19,6 +20,8 @@ class AbseilModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "abseil-auto-make-unique"); CheckFactories.registerCheck( "abseil-string-find-startswith"); } Index: clang-tidy/abseil/AutoMakeUniqueCheck.h =================================================================== --- clang-tidy/abseil/AutoMakeUniqueCheck.h +++ clang-tidy/abseil/AutoMakeUniqueCheck.h @@ -0,0 +1,38 @@ +//===--- AutoMakeUniqueCheck.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_AUTOMAKEUNIQUECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_AUTOMAKEUNIQUECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace abseil { + +/// Replace: +/// std::unique_ptr x = make_unique(...); +/// with: +/// auto x = make_unique(...); +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-auto-make-unique.html +class AutoMakeUniqueCheck : public ClangTidyCheck { +public: + AutoMakeUniqueCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace abseil +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_AUTOMAKEUNIQUECHECK_H Index: clang-tidy/abseil/AutoMakeUniqueCheck.cpp =================================================================== --- clang-tidy/abseil/AutoMakeUniqueCheck.cpp +++ clang-tidy/abseil/AutoMakeUniqueCheck.cpp @@ -0,0 +1,88 @@ +//===--- AutoMakeUniqueCheck.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 "AutoMakeUniqueCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace abseil { + +void AutoMakeUniqueCheck::registerMatchers(MatchFinder* finder) { + if (!getLangOpts().CPlusPlus) return; + + using clang::ast_matchers::isTemplateInstantiation; + auto is_instantiation = decl(anyOf(cxxRecordDecl(isTemplateInstantiation()), + varDecl(isTemplateInstantiation()), + functionDecl(isTemplateInstantiation()))); + // There should be no additional expressions inbetween. + // E.g. this statement contains implicitCastExpr which makes it not eligible: + // std::unique_ptr p = make_unique(); + finder->addMatcher( + varDecl(has(exprWithCleanups(has(cxxConstructExpr( + has(ignoringParenImpCasts(cxxBindTemporaryExpr( + has(ignoringParenImpCasts(callExpr(callee(functionDecl( + hasAnyName("absl::MakeUnique", "absl::make_unique", + "gtl::MakeUnique", "std::make_unique"), + decl().bind("make_unique_decl"))))))))))))), + hasType(classTemplateSpecializationDecl( + hasName("std::unique_ptr"), + hasTemplateArgument( + 1, refersToType(qualType(hasDeclaration(cxxRecordDecl( + hasName("std::default_delete")))))))), + unless(hasType(autoType())), + unless(anyOf(is_instantiation, hasAncestor(is_instantiation)))) + .bind("var_decl"), + this); +} + +const Type* GetUniquePtrType(const VarDecl* var) { + const auto* unique = dyn_cast_or_null( + var->getType()->getAsCXXRecordDecl()); + if (!unique) return nullptr; + QualType type = unique->getTemplateArgs().get(0).getAsType(); + return type->getUnqualifiedDesugaredType(); +} + +const Type* GetMakeUniqueType(const FunctionDecl* make_unique_decl) { + const auto& template_arg = + make_unique_decl->getTemplateSpecializationInfo()->TemplateArguments->get( + 0); + return template_arg.getAsType()->getUnqualifiedDesugaredType(); +} + +void AutoMakeUniqueCheck::check( + const ast_matchers::MatchFinder::MatchResult& result) { + const auto* var_decl = result.Nodes.getNodeAs("var_decl"); + const auto* make_unique_decl = + result.Nodes.getNodeAs("make_unique_decl"); + if (var_decl->isOutOfLine()) { + // "auto Struct::field = make_unique<...>();" doesn't work in GCC. + return; + } + if (GetUniquePtrType(var_decl) != GetMakeUniqueType(make_unique_decl)) { + // The variable and the make_unique expression do not share the same type. + // Ignore this. + return; + } + + diag(var_decl->getTypeSpecStartLoc(), + "use 'auto' to avoid repeating the type name") + << FixItHint::CreateReplacement( + clang::CharSourceRange::getCharRange( + var_decl->getTypeSpecStartLoc(), var_decl->getLocation()), + "auto "); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang Index: clang-tidy/abseil/CMakeLists.txt =================================================================== --- clang-tidy/abseil/CMakeLists.txt +++ clang-tidy/abseil/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyAbseilModule AbseilTidyModule.cpp + AutoMakeUniqueCheck.cpp StringFindStartswithCheck.cpp LINK_LIBS Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,11 @@ Improvements to clang-tidy -------------------------- +- New :doc:`abseil-auto-make-unique + ` check. + + FIXME: add release notes. + - New :doc:`readability-magic-numbers ` check. Index: docs/clang-tidy/checks/abseil-auto-make-unique.rst =================================================================== --- docs/clang-tidy/checks/abseil-auto-make-unique.rst +++ docs/clang-tidy/checks/abseil-auto-make-unique.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - abseil-auto-make-unique + +abseil-auto-make-unique +======================= + +The check suggests replacing: + +.. code-block:: c++ + + std::unique_ptr x = MakeUnique(...); + +with + +.. code-block:: c++ + + auto x = MakeUnique(...); + +The initializer already spells the type, so it can be safely omitted from the +declaration while making the code more readable. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ ================= .. toctree:: + abseil-auto-make-unique abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 Index: test/clang-tidy/abseil-auto-make-unique.cpp =================================================================== --- test/clang-tidy/abseil-auto-make-unique.cpp +++ test/clang-tidy/abseil-auto-make-unique.cpp @@ -0,0 +1,115 @@ +// RUN: %check_clang_tidy %s abseil-auto-make-unique %t + +namespace std { +template +struct default_delete {}; + +template > +class unique_ptr { + public: + unique_ptr(); + ~unique_ptr(); + explicit unique_ptr(T*); + template + unique_ptr(unique_ptr&&); +}; +template +unique_ptr make_unique(Args&&...); +} // namespace std + +namespace absl { +template +std::unique_ptr MakeUnique(Args&&...); +template +std::unique_ptr make_unique(Args&&...); +} // namespace absl +using absl::make_unique; + +typedef int integer; + +struct Base {}; +struct Derived : public Base {}; + +void Primitive() { + std::unique_ptr x = absl::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto x = absl::make_unique(); + + std::unique_ptr< int >y = absl::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto y = absl::make_unique(); + + const std::unique_ptr z = absl::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: const auto z = absl::make_unique(); + + std::unique_ptr t = absl::make_unique(); +} + +void Typedefs() { + std::unique_ptr x = absl::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto x = absl::make_unique(); + + std::unique_ptr y = absl::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto y = absl::make_unique(); + + std::unique_ptr z = absl::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto z = absl::make_unique(); +} + +void Class() { + std::unique_ptr base = make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto base = make_unique(); + + std::unique_ptr base2(make_unique()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto base2(make_unique()); + + // Different type. No change. + std::unique_ptr z = make_unique(); + std::unique_ptr z2(make_unique()); +} + +template +void f() { + std::unique_ptr x = make_unique(); +} + +void Negatives() { + // Different deleter. No change. + struct MyDeleter {}; + std::unique_ptr z3 = make_unique(); + std::unique_ptr z4(make_unique()); + + f(); +} + +std::unique_ptr global_var = make_unique(); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'auto' to avoid repeating the type name +// CHECK-FIXES: auto global_var = make_unique(); + +struct Struct { + static std::unique_ptr static_field; +}; +// This code with "auto" replaced doesn't compile in GCC. +std::unique_ptr Struct::static_field = make_unique(); + +void FunctionWithStatic() { + static std::unique_ptr static_var = make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto static_var = make_unique(); +} + +void OtherNames() { + std::unique_ptr x = absl::MakeUnique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto x = absl::MakeUnique(); + + std::unique_ptr y = std::make_unique(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name + // CHECK-FIXES: auto y = std::make_unique(); +}