unique_ptr and llvm cast machinery

Anyone have opinions on whether the cast machinery should be taught to
handle unique_ptr?

Presumably that'd involve cast, etc, returning raw pointers when it
was passed references to unique_ptr, which might be
surprising/error-prone?

But the only errors would be:

1) double delete - if the result of the cast was used to take
ownership because the caller didn't realize there was a unique_ptr
that held ownership (eg: a function that returns a reference to a
unique_ptr, passed to cast, then ownership taken)

2) dangling pointers - but this wouldn't be more or less likely with
the change to cast.

I'm not convinced that allowing it to accept unique_ptr types would be significantly less typing than requiring users to pass the result of the get() method to the cast and it does sound like hiding the explicit ownership control would be more error prone.

That said, it might be useful to have variants of dyn_cast<> that would take a unique_ptr and return another unique_ptr to the cast type, complete with ownership transfer, or leave the original unique_ptr intact if the cast failed.

David

That said, it might be useful to have variants of dyn_cast<> that would take a unique_ptr and return another unique_ptr to the cast type, complete with ownership transfer, or leave the original unique_ptr intact if the cast failed.

I suppose the name would be dyn_move<> or something?

Yup.

How often is this useful?

I've used this pattern a few times in non-LLVM code (collect a vector unique pointers to some superclass, then pop them off the vector and dispatch them to some handlers as unique pointers to something else. I don't know how useful it would be in LLVM code.

On a semi-related note, we've noticed that with transparent superpage support, we get a big speedup with GCC, but not with LLVM, implying that LLVM fragments its heap a lot more (resulting in more TLB churn, even with large pages). If anyone is interested in memory management stuff in LLVM, improving this could likely give a noticeable speedup...

David

Alrighty - minor (in the sense that I don't see anyone else putting
their hands up with opinions) preference towards not adding
unique_ptr->raw pointer support to the cast machinery.

I'll just add get() calls for now (or dereferences in the case of
cast<T>, probably). Keep an eye out for the patches & we'll see how it
looks in practice & whether there are other ways to tidy it up.

(minor rant: I still find it unfortunate that I need a pointer to use
dyn_cast... (not dyn_cast_or_null - obviously that needs a pointer) -
cast from T& to Optional<T&> (or T*) would be nice, but I got shot
down on that a while ago)