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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24
@@ +20,6 @@
+
+void g(int i) {
+  if (i < 0) {
+    return;
+  }
+  if (i < 10) {
----------------
LegalizeAdulthood wrote:
> kimgr wrote:
> > LegalizeAdulthood wrote:
> > > kimgr wrote:
> > > > What happens to guard clauses invoking void functions?
> > > > 
> > > >     void h() {
> > > >     }
> > > > 
> > > >     void g(int i) {
> > > >       if(i < 0) {
> > > >         return h();
> > > >       }
> > > >     }
> > > > 
> > > Nothing because the last statement of the `compoundStmt` that is the function body is an `if` statement and not a `return` statement.
> > > 
> > > That is exactly why lines 21-24 are in the test suite :).
> > Ah, I hadn't understood the mechanics of the check. I read the docs, and now I do! Don't mind me :-)
> I had thought about doing a deeper analysis of the control flow to look for such cases, but I will leave that for later.
> 
> For instance, the following code may plausibly appear in a code base:
> 
> ```
> void f() {
>   do_stuff();
>   {
>     lock l(mutex);
>     do_locked_stuff();
>     return;
>   }
> }
> ```
> 
> I haven't tried this on this patch, but I suspect it would do nothing; I will add some examples of these more complicated cases to the test suite to show that the implementation doesn't yet handle more advanced flow analysis.
> 
> In this case, the `return` is similarly redundant, as well as a `return` as the last statement of an `if` as you mentioned.  However, I wanted to start with something simple and get feedback on that before attempting to do more advanced cases.
> 
I think that starting simple and expanding later is the right approach.


http://reviews.llvm.org/D16259