r=me if @RalfJung can confirm it's a correct approach

Do you suggestions on alternatives to raw pointer cast? If not, I guess the code should be merged as-is.

This might be a shot in the dark but would something like the following work (and deal with the FIXME):

pub fn filter_map<F>(self, f: F) -> Option<P<T>> where
    F: FnOnce(T) -> Option<T>,
{
    unsafe {
        // Transmute to `Box<MaybeUninit<T>>` so if `f` `panic`s or returns
        // `None` the `Box` will be freed but not the inner T.
        let mut maybe_box: Box<MaybeUninit<T>> = mem::transmute(self.ptr);
        let x = f(ptr::read(maybe_box.as_ptr()))?;
        maybe_box.set(x);
        Some(P { ptr: mem::transmute(maybe_box) })
    }
}

I think @ollie27's suggestion is pretty good, any opinions/alternatives regarding the use of transmute?

It seems to me that using ManuallyDrop instead of MaybeUninit is sufficient?

@RalfJung the snippet above uses MaybeUninit::set.

I guess this would work:

pub fn filter_map<F>(self, f: F) -> Option<P<T>> where
    F: FnOnce(T) -> Option<T>,
{
    unsafe {
        // Transmute to `Box<ManuallyDrop<T>>` so if `f` `panic`s or returns
        // `None` the `Box` will be freed but not the inner T.
        let mut manaully_drop_box: Box<ManuallyDrop<T>> = mem::transmute(self.ptr);
        let x = f(ManuallyDrop::take(&mut manaully_drop_box))?;
        *manaully_drop_box = ManuallyDrop::new(x);
        Some(P { ptr: mem::transmute(manaully_drop_box) })
    }
}

Yeah, something like that.

Either way though I think it'd be good to avoid "open" transmute and use helper functions instead. Turning Box<MaybeUninit<T>> into Box<T> should probably even become a stable (unsafe) operation some day.

ping from triage @ishitatsuyuki @eddyb what's the update on this?

r? @RalfJung

@ishitatsuyuki could you update this to one of the proposed panic-safe schemes? If not I can r+ this, because it is an improvement, but the other version is even better.

Actually I invented one that is drastically simpler, based on ManuallyDrop.

The pains are with the Box<[T]> methods. I know that the best way would be adding some methods to Box itself, but I'm not inclined to do that since that's not future-proof (a pre-RFC has been on internals, for example).

Not sure if I like changing P itself -- for once, didn't you forget to add a impl Drop?

But it seems like some local private conversion functions between Box<T> and Box<ManuallyDrop<T>> would help. Long-term we probably want something like this in libstd. (We also want that for MaybeUninit, this should probably be co-designed.)

I spent some effort and wrote things again. Hopefully this is what you wanted.

I will do the tracking issue thing tomorrow.

@rust-lang/libs any advise on how to proceed here -- should we add two new unstable methods with a tracking issue, or instead add two local private helper functions in syntax::ptr?

Per #58021 (comment) it looks like the one use case for those methods can be solved with plain Box<T> without ManuallyDrop. Unless there’s another use case, let’s not add public APIs.

I hadn't even thought so far, but you are right... this proper move tracking for boxes entirely sidesteps the need for ManuallyDrop. We can even write these methods in safe code.

(Short on time this week, will update this weekend)

⌛ Testing commit a03e20d with merge e68bf8a...

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing e68bf8a to master...

Discussed in this week's T-compiler meeting. Approved for beta-backport.

(In parallel, @pnkfelix wants to make a regression test. But than need not hold up the backport.)

(In parallel, @pnkfelix wants to make a regression test. But than need not hold up the backport.)

I was careless and misread the patch, thinking that this was some fn map and fn filter_map in libstd; but that wasn't true at all, its in libsyntax. I'm not going to worry about a regression test.