[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

A few more nits.



================
Comment at: clang-tidy/modernize/ReturnBracedInitListCheck.cpp:66
+  auto Diag = diag(Loc, "to avoid repeating the return type from the "
+                        "declaration, use a braced initializer list instead");
+
----------------
nit: Use a semicolon before "use": `; use`.


================
Comment at: docs/clang-tidy/checks/modernize-return-braced-init-list.rst:8
+initializer list. This way the return type is not needlessly duplicated in the
+return type and the return statement.
+
----------------
"return type is not ... duplicated in the return type" doesn't read well. "return type is not ... duplicated in the function definition" maybe?


================
Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:71
+  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};
----------------
Please truncate the static repeated parts of the CHECK lines around the 80th column (e.g. after "type" or after "declaration;", if the latter still fits into 80 columns). The first CHECK line should be complete though.


================
Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:72
+  // 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};
+}
----------------
This pattern is not reliable, since there are multiple return statements involving `b` in this file. The only way check patterns are bound to the line is using the content (for this reason `CHECK-MESSAGES` contains `:[[@LINE-x]]:...`).

In case of `CHECK-FIXES` the easiest way to ensure patterns match only what is intended, is to make their content unique. Here I'd give all variables unique names (`b1`, `b2`, ..., for example). If there are multiple identical lines that contain no identifiers, they can be made unique by adding comments (and matching them).


Repository:
  rL LLVM

https://reviews.llvm.org/D28768