Sirraide 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.

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

Reply via email to