https://github.com/Sirraide commented:
So from a high-level point of view, I definitely think there is value in having this because I personally have made this mistake *so* many times, and honestly, I think we should just straight-up error on it (or at least make it a warning that defaults to an error) because I cannot think of a situation where doing this would ever be valid. That said, using an AST visitor for this is... probably not great because we now do an AST traversal on every `else` branch ever, which sounds like it might hurt performance quite a bit—that said, this is pretty much my only concern w/ this patch, so if it turns out that this isn’t the case at all then this approach is fine I suppose. CC @nikic could you put this on llvm-compile-time-tracker so we know what the overhead of this is if any? At the same time, I’m not sure I can think of a better way to do this. I suppose another option might be to add a flag to `VarDecl` that keeps track of whether it’s declared in the condition of an `if` statement and then somehow have scope information to figure out where a given use of it is relative to that `if` statement, but that sounds like it’d get a bit complicated, and I’m not sure running this sort of check on every DRE would be any better... (Side note: sometimes when there are performance considerations we prefer to move these sorts of diagnostics into the static analyser, but I think this is situation is bad enough to where we want to do this in the compiler proper—at least I can think of many instances where this would have helped me quite a bit.) https://github.com/llvm/llvm-project/pull/156436 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
