[PATCH] D16259: Add clang-tidy readability-redundant-return check

[Richard via cfe-commits]

LegalizeAdulthood marked 8 inline comments as done.

================
Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24
@@ +23,3 @@
+      functionDecl(isDefinition(), returns(asString("void")),
+                   has(compoundStmt(hasAnySubstatement(returnStmt()))))
+          .bind("fn"),
----------------
aaron.ballman wrote:
> LegalizeAdulthood wrote:
> > aaron.ballman wrote:
> > > Would be best to restrict this to a return statement that has no expression if we don't want to diagnose this:
> > > ```
> > > void g();
> > > 
> > > void f() {
> > >   return g();
> > > }
> > > ```
> > > Either way, it would be good to have a test that ensures this isn't mangled.
> > How about transforming this odd looking duck into
> > 
> > ```
> > void g();
> > 
> > void f() {
> >   g();
> > }
> > ```
> > 
> > ?
> I think in the context of this check, that would be fine.
For now I have opted to simply ignore such odd ducks.

================
Comment at: test/clang-tidy/readability-redundant-return.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s readability-redundant-return %t
+
----------------
alexfh wrote:
> Please add tests with macros and templates that ensure:
>   * no replacements in macro bodies
>   * correct replacements in macro arguments (that are not inside another macro's body)
>   * replacements in template bodies with multiple instantiations are applied only once (adding `unless(isInTemplateInstantiation())` filter to the matcher is a usual way to handle this).
When I added template test cases, I wasn't sure how to write a test case for what you requested.  I wrote test code that multiply instantiated a template and only see my check triggering on the template body as written in the code.


http://reviews.llvm.org/D16259