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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
