[PATCH] D32571: InferAddressSpaces: Search constant expressions for addrspacecasts

[Justin Lebar via llvm-commits]

Ok.  No longer at my laptop, but lgtm with a comment somewhere helping
readers understand why it's done this way.

On Apr 28, 2017 1:02 PM, "Matt Arsenault via Phabricator" <
reviews at reviews.llvm.org> wrote:

> arsenm added inline comments.
>
>
> ================
> Comment at: lib/Transforms/Scalar/InferAddressSpaces.cpp:843
> +  for (const WeakVH &WVH : Postorder) {
> +    assert(WVH && "value was unexpectedly deleted");
> +    Value *V = WVH;
> ----------------
> arsenm wrote:
> > jlebar wrote:
> > > If we expect that the Value*s won't be deleted while in this vector,
> why are we using WeakVH at all?  Are we trying to catch use-after-frees?
> > >
> > > At least the choice to use WeakVH could use a comment somewhere, but
> if it's just fear of use-after-free's, is there some reason that this code
> is particularly dangerous as compared to other parts of LLVM, where we rely
> on asan/msan to catch this issue?
> > It's possible to make it work without WeakVH, but it fails to handle
> more cases. The RAUW used for the handling of constants needs to be tracked
> Earlier versions of the patch were more complicated and attempted to
> recursively handle all of the constants up front, but it ended up much more
> complicated than to just add WeakVH here.
>
>
> https://reviews.llvm.org/D32571
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170428/ac506115/attachment.html>