[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D71846#1796883 <https://reviews.llvm.org/D71846#1796883>, @njames93 wrote:

> So I wasn't happy with the vagueness of the else after return check implementation. Almost finished rewriting the check to properly handle the if statements with condition variables based on scope restrictions and where decls are used etc.
>
>   int *varInitAndCondition() {
>     if (int *X = g(); X != nullptr) {
>       return X;
>     } else {
>       h(&X);
>       return X;
>     }
>   }
>   int *varInitAndConditionUnusedInElseWithDecl() {
>     int *Y = g();
>     if (int *X = g(); X != nullptr) {
>       return X;
>     } else {
>       int *Y = g();
>       h(&Y);
>     }
>     return Y;
>   }
>
>
> transforms to
>
>   int *varInitAndCondition() {
>     int *X = g();
>     if (X != nullptr) {
>       return X;
>     }
>     h(&X);
>     return X;
>   }
>   int *varInitAndConditionUnusedInElseWithDecl() {
>     int *Y = g();
>     if (int *X = g(); X != nullptr) {
>       return X;
>     } else {
>       int *Y = g();
>       h(&Y);
>     }
>     return Y;
>   }
>
>
> There's a few more test cases but that's the general idea. I'm having a little trouble writing the test cases as I can't run them on my windows machine to verify they report correctly


Hmm, I'm not convinced the new behavior is better in all cases, and I suspect this may boil down to user preference (suggesting we may want an option to control the behavior). There are competing stylistic decisions to be made for `varInitAndCondition()` -- one is the `else` after a `return`, but the other is the scope of the variable `X`. The behavior now forces the user to prefer removing the `else` after the `return` over hiding the variable `X` in a logical scope. For the contrived example provided, this isn't a huge deal because the function is small. However, in real-world code with larger function bodies, I can see wanting to prefer the name hiding. WDYT?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71846/new/

https://reviews.llvm.org/D71846