erichkeane wrote:
> > IS a pattern folks actually use intentionally. So making this an error
> > isn't something we can really do.
>
> Right, I think I wasn’t quite awake yet earlier because for some reason I was
> only thinking about the very specific use case where the `if` statement
> checks if a pointer is null, but you might reasonably do this for e.g. a
> `std::expected` where one branch consumes the value and the other the error.
>
> After thinking about this some more, I wonder if it would make sense to
> restrict this warning to situations where we both
>
> 1. the variable declaration is also the condition of the `if` statement,
> and
>
> 2. it’s very unlikely that consuming the ‘falsy’ value on the else branch
> would ever be useful.
>
I think that is reasonable, though we'd need to make sure `falsy` doesn't
include `contextual conversion to bool` using `user defined functions` I think.
But bool/int/etc with builtin conversions to `false` are probably OK.
>
> Not doing 1. would cause us to diagnose cases such as
>
> ```c++
> if (Foo* x = foo(); not x) {
> // ...
> } else {
> bar(*x); // We should not diagnose this.
> }
> ```
>
> And 2. should help minimise false positives; e.g. using a variable that is
> known to be null in the else branch feels like a bit of a weird thing to
> do—though I guess someone could reuse it by assigning to it.
>
> > We can actually do this via
> > https://clang.llvm.org/doxygen/classclang_1_1Scope.html I think instead? I
> > wonder if anyone else has better ideas
>
> Yeah, that’s more or less what I meant by ‘somehow have scope information to
> figure out where a given use of it is’.
https://github.com/llvm/llvm-project/pull/156436
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
