JDevlieghere updated this revision to Diff 84700.
JDevlieghere added a comment.

- Added test cases suggested by @Prazek
- Added test cases suggested by @alexfh

I don't match on `CXXUnresolvedConstructExpr` so the template dependent cases 
are not impacted by this check. This is what I intended, because we cannot make 
assumptions about the constructor being explicit, narrowing, etc.


Repository:
  rL LLVM

https://reviews.llvm.org/D28768

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReturnBracedInitListCheck.cpp
  clang-tidy/modernize/ReturnBracedInitListCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-return-braced-init-list.rst
  test/clang-tidy/modernize-return-braced-init-list.cpp

Index: test/clang-tidy/modernize-return-braced-init-list.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-return-braced-init-list.cpp
@@ -0,0 +1,173 @@
+// RUN: %check_clang_tidy %s modernize-return-braced-init-list %t -- --
+// -std=c++14
+
+namespace std {
+typedef decltype(sizeof(int)) size_t;
+
+// libc++'s implementation
+template <class _E>
+class initializer_list {
+  const _E *__begin_;
+  size_t __size_;
+
+  initializer_list(const _E *__b, size_t __s)
+      : __begin_(__b),
+        __size_(__s) {}
+
+public:
+  typedef _E value_type;
+  typedef const _E &reference;
+  typedef const _E &const_reference;
+  typedef size_t size_type;
+
+  typedef const _E *iterator;
+  typedef const _E *const_iterator;
+
+  initializer_list() : __begin_(nullptr), __size_(0) {}
+
+  size_t size() const { return __size_; }
+  const _E *begin() const { return __begin_; }
+  const _E *end() const { return __begin_ + __size_; }
+};
+
+template <typename T>
+class vector {
+public:
+  vector(T){};
+  vector(std::initializer_list<T>) {}
+};
+}
+
+class Bar {};
+
+class Foo {
+public:
+  Foo(Bar);
+  explicit Foo(Bar, unsigned int);
+  Foo(unsigned int);
+};
+
+class Baz {
+public:
+  Foo m() {
+    Bar b;
+    return Foo(b);
+    // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+    // CHECK-FIXES: return {b};
+  }
+};
+
+class Quux : public Foo {
+public:
+  Quux(Bar bar) : Foo(bar){};
+};
+
+Foo f() {
+  Bar b;
+  return Foo(b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {b};
+}
+
+Foo f2() {
+  Bar b;
+  return {b};
+}
+
+auto f3() {
+  Bar b;
+  return Foo(b);
+}
+
+#define A(b) Foo(b)
+
+Foo f4() {
+  Bar b;
+  return A(b);
+}
+
+Foo f5() {
+  Bar b;
+  return Quux(b);
+}
+
+Foo f6() {
+  Bar b;
+  return Foo(b, 1);
+}
+
+std::vector<int> f7() {
+  int i = 1;
+  return std::vector<int>(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {i};
+}
+
+Bar f8() {
+  return {};
+}
+
+Bar f9() {
+  return Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {};
+}
+
+Bar f10() {
+  return Bar{};
+}
+
+Foo f11(Bar b) {
+  return Foo(b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {b};
+}
+
+Foo f12() {
+  return f11(Bar());
+}
+
+Foo f13() {
+  return Foo(Bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {Bar()};
+}
+
+Foo f14() {
+  // FIXME: Type narrowing should not occur!
+  return Foo(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {-1};
+}
+
+Foo f15() {
+  return Foo(f10());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type from the declaration, use a braced initializer list instead [modernize-return-braced-init-list]
+  // CHECK-FIXES: return {f10()};
+}
+
+template <typename T>
+T f16() {
+  return T();
+}
+
+template <typename T>
+Foo f17(T t) {
+  return Foo(t);
+}
+
+template <typename T>
+class BazT {
+public:
+  T m() {
+    Bar b;
+    return T(b);
+  }
+
+  Foo m2(T t) {
+    return Foo(t);
+  }
+};
+
+auto v1 = []() { return std::vector<int>({1, 2}); }();
+auto v2 = []() -> std::vector<int> { return std::vector<int>({1, 2}); }();
Index: docs/clang-tidy/checks/modernize-return-braced-init-list.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-return-braced-init-list.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - modernize-return-braced-init-list
+
+modernize-return-braced-init-list
+=================================
+
+Replaces explicit calls to the constructor in a return with a braced
+initializer list. This way the return type is not needlessly duplicated in the
+return type and the return statement.
+
+.. code:: c++
+
+  Foo bar() {
+    Baz baz;
+    return Foo(baz);
+  }
+
+  // transforms to:
+
+  Foo bar() {
+    Baz baz;
+    return {baz};
+  }
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -109,6 +109,7 @@
    modernize-raw-string-literal
    modernize-redundant-void-arg
    modernize-replace-auto-ptr
+   modernize-return-braced-init-list
    modernize-shrink-to-fit
    modernize-use-auto
    modernize-use-bool-literals
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,11 @@
 Improvements to clang-tidy
 --------------------------
 
-The improvements are...
+- New `modernize-return-braced-init-list
+    <http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html>`_ check
+
+  Replaces explicit calls to the constructor in a return statement by a braced
+  initializer list so that the return type is not needlessly repeated.
 
 Improvements to include-fixer
 -----------------------------
Index: clang-tidy/modernize/ReturnBracedInitListCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ReturnBracedInitListCheck.h
@@ -0,0 +1,36 @@
+//===--- ReturnBracedInitListCheck.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_MODERNIZE_RETURN_BRACED_INIT_LIST_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_RETURN_BRACED_INIT_LIST_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Use a braced init list for return statements rather than unnecessary
+/// repeating the return type name.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html
+class ReturnBracedInitListCheck : public ClangTidyCheck {
+public:
+  ReturnBracedInitListCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_RETURN_BRACED_INIT_LIST_H
Index: clang-tidy/modernize/ReturnBracedInitListCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ReturnBracedInitListCheck.cpp
@@ -0,0 +1,106 @@
+//===--- ReturnBracedInitListCheck.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 "ReturnBracedInitListCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void ReturnBracedInitListCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  // Skip list initialization and constructors with an initializer list.
+  auto ConstructExpr =
+      cxxConstructExpr(
+          unless(anyOf(isListInitialization(), hasDescendant(initListExpr()))))
+          .bind("ctor");
+
+  auto HasConstructExpr = has(ConstructExpr);
+
+  auto CtorAsArgument = materializeTemporaryExpr(
+      anyOf(HasConstructExpr, has(cxxFunctionalCastExpr(HasConstructExpr))));
+
+  Finder->addMatcher(
+      functionDecl(isDefinition(), // Declarations don't have return statements.
+                   returns(unless(anyOf(builtinType(), autoType()))),
+                   hasDescendant(returnStmt(hasReturnValue(
+                       has(cxxConstructExpr(has(CtorAsArgument)))))))
+          .bind("fn"),
+      this);
+}
+
+void ReturnBracedInitListCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedFunctionDecl = Result.Nodes.getNodeAs<FunctionDecl>("fn");
+  const auto *MatchedConstructExpr =
+      Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+
+  // Don't make replacements in macro.
+  SourceLocation Loc = MatchedConstructExpr->getExprLoc();
+  if (Loc.isMacroID())
+    return;
+
+  // Skip explicit construcotrs.
+  if (MatchedConstructExpr->getConstructor()->isExplicit())
+    return;
+
+  // Make sure that the return type matches the constructed type.
+  const QualType returnType =
+      MatchedFunctionDecl->getReturnType().getCanonicalType();
+  const QualType constructType =
+      MatchedConstructExpr->getType().getCanonicalType();
+  if (returnType != constructType)
+    return;
+
+  auto Diag = diag(Loc, "to avoid repeating the return type from the "
+                        "declaration, use a braced initializer list instead");
+
+  int i = 0;
+  for (Decl *D :
+       MatchedConstructExpr->getConstructor()->getAsFunction()->decls()) {
+    if (const auto *VD = dyn_cast<VarDecl>(D)) {
+      if (MatchedConstructExpr->getArg(i++)->getType().getCanonicalType() !=
+          VD->getType().getCanonicalType())
+        return;
+    }
+  }
+
+  const SourceRange CallParensRange =
+      MatchedConstructExpr->getParenOrBraceRange();
+
+  // Return if there is no explicit constructor call.
+  if (CallParensRange.isInvalid())
+    return;
+
+  // Range for constructor name and opening brace.
+  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+      Loc, CallParensRange.getBegin().getLocWithOffset(-1));
+
+  Diag << FixItHint::CreateRemoval(CtorCallSourceRange)
+       << FixItHint::CreateReplacement(
+              CharSourceRange::getTokenRange(CallParensRange.getBegin(),
+                                             CallParensRange.getBegin()),
+              "{")
+       << FixItHint::CreateReplacement(
+              CharSourceRange::getTokenRange(CallParensRange.getEnd(),
+                                             CallParensRange.getEnd()),
+              "}");
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -19,6 +19,7 @@
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
+#include "ReturnBracedInitListCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
@@ -53,6 +54,8 @@
         "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
+    CheckFactories.registerCheck<ReturnBracedInitListCheck>(
+        "modernize-return-braced-init-list");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -13,6 +13,7 @@
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
+  ReturnBracedInitListCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to