[PATCH] D147991: [LLVM][Casting.h] Fix dyn_cast for std::unique_ptr.

[Aleksandr Bezzubikov via Phabricator via llvm-commits]

zuban32 added a comment.

In D147991#4260050 <https://reviews.llvm.org/D147991#4260050>, @bzcheeseman wrote:

> The test looks fine to me, but I'm now realizing some tricky behavior. If you std::move a unique pointer into a dyn_cast, and casting fails, you now have no pointer anymore (it's been moved already). In the unit test, if BP couldn't be cast to `foo` I believe you'd see this behavior. I vaguely recall hitting some of this in the past, does it make sense to have UniquePtrCast take `unique_ptr<> &` rather than `unique_ptr<> &&` and use `.release()/.reset()`?

Oh, you're right, haven't tried that yet but I'm pretty sure that's what's going to happen. I think we could just make dyn_cast take lvalue reference to `unique_ptr` instead of an rvalue one, then check it with `isa<>`, and then either move the pointer further or just return nullptr leaving the original pointer intact. I'll update the patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147991/new/

https://reviews.llvm.org/D147991