t-8ch added a comment. @steakhal Thanks for your review! I think I have addressed all your points.
In D131067#3702044 <https://reviews.llvm.org/D131067#3702044>, @steakhal wrote: > Please, consider stating the motivation behind this change. This is now added. > For me, by looking at the modified test, it feels like we would lose > legitimate findings (aka. true-positive reports) by applying this change. The change will indeed loose legitimate findings. We would have to also analyze the source code of the called function to see if it takes the address of the parameter. But for callees which we don't have access to the AST this would still not work. The existing escape-analysis works the same way, marking anything as escaped that *could* have escaped. See the existing test `basicLambda` or the non-reference example: void foo() { int x = 0; if (&x != &x) { return; } // This is clearly a dead store but as we have taken the address of x above it is treated as escaped. x = 3; } > From my perspective, the store to `i` is //dead//, since it will be > immediately overwritten in the `referenceParameters()` function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131067/new/ https://reviews.llvm.org/D131067 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
