Stream: t-compiler/wg-nll

Topic: why-ptr-read


nikomatsakis (Jul 24 2018 at 18:18, on Zulip):

so @Santiago Pastorino was asking me about what @pnkfelix meant by this:

I'm looking at it now as well, since I didn't see any notes from @Santiago Pastorino . The main interesting complication so far is that DiagnosticBuilderimplements Drop, so you have to use ptr::read to pull out its self.diagnostic (and then mem::forget(self) afterward).

and I thought I might as well answer it here. This is in reference to the DiagnosticBuilder thread...

nikomatsakis (Jul 24 2018 at 18:18, on Zulip):

... the situation is that you cannot move out of a field from a type that implements Drop

nikomatsakis (Jul 24 2018 at 18:18, on Zulip):

e.g., if Foo implements Drop, then you cannot do let x = foo.a

nikomatsakis (Jul 24 2018 at 18:18, on Zulip):

the reason for this is that -- when the Drop for foo should run -- it would have an "incomplete" value

nikomatsakis (Jul 24 2018 at 18:19, on Zulip):

and it was felt it would be too confusing to e.g. have the Drop not run if you have moved out from the value (too easy to screw up)

nikomatsakis (Jul 24 2018 at 18:19, on Zulip):

so in this case there was a struct DiagnosticBuilder { handler, diagnostic } and we wanted to put the diagnostic field into a vector

nikomatsakis (Jul 24 2018 at 18:19, on Zulip):

we can't just move it because DiagnosticBuilder implements Drop -- it does so that it can panic if you fail to emit

nikomatsakis (Jul 24 2018 at 18:19, on Zulip):

I had suggested the simplest fix, which was to do vec.push(self.diagnostic.clone()), but that is somewhat inefficient

nikomatsakis (Jul 24 2018 at 18:20, on Zulip):

in that it does a clone

nikomatsakis (Jul 24 2018 at 18:20, on Zulip):

@pnkfelix opted instead to use ptr::read, which is an unsafe function that copies a value without moving it -- kind of cheats and duplicates a value that is not supposed to be duplicated

nikomatsakis (Jul 24 2018 at 18:20, on Zulip):

he then used mem::forget to cause the original value to be ignored and to never run its destructor

nikomatsakis (Jul 24 2018 at 18:20, on Zulip):

this is mildly more efficient but obviously more error prone

pnkfelix (Jul 24 2018 at 18:21, on Zulip):

I also decided the “cost” of an unsafe block (In terms of future programmer revIew effort) was justified here

pnkfelix (Jul 24 2018 at 18:21, on Zulip):

In that if I did a clone, that also would impose future effort for people to review and understand what was going on

pnkfelix (Jul 24 2018 at 18:22, on Zulip):

Namely that it’s not really duplicating state; it wants to be a move but we can’t express that via safe code

pnkfelix (Jul 24 2018 at 18:23, on Zulip):

And so I just decided that ptr::read + mem::forget directly expresses intent

pnkfelix (Jul 24 2018 at 18:24, on Zulip):

That is: I really was doing this for readability, not efficiency

pnkfelix (Jul 24 2018 at 18:24, on Zulip):

But it’s possible my model of readability is busted

nikomatsakis (Jul 24 2018 at 18:24, on Zulip):

heh :)

nikomatsakis (Jul 24 2018 at 18:25, on Zulip):

Namely that it’s not really duplicating state; it wants to be a move but we can’t express that via safe code

I wonder ... it would be kind of neat to move some operation for this. meh.

simulacrum (Jul 24 2018 at 18:29, on Zulip):

If the fields are public in theory can't you pattern match safely?

nikomatsakis (Jul 24 2018 at 18:33, on Zulip):

I think I'd prefer for there to be an explicit operation to "disable" the dtor

nikomatsakis (Jul 24 2018 at 18:33, on Zulip):

in any case "pattern matching" is not visible in MIR land

nikomatsakis (Jul 24 2018 at 18:33, on Zulip):

so I'd prefer for that not to be the rule :)

nikomatsakis (Jul 24 2018 at 18:34, on Zulip):

in most cases, I suspect (as in this one...) you really only want to grab a single field

simulacrum (Jul 24 2018 at 18:35, on Zulip):

let Foo { field, ... } = my_var; though, right?

simulacrum (Jul 24 2018 at 18:36, on Zulip):

I agree it's not every explicit though

Santiago Pastorino (Jul 24 2018 at 19:22, on Zulip):

@nikomatsakis thanks for the clarification

pnkfelix (Jul 24 2018 at 19:26, on Zulip):

I think I'd prefer for there to be an explicit operation to "disable" the dtor

(there have been proposals for ways to do this; usually to simultaneously disable the dtor and move all the fields, or move into an "interior view" of the whole struct)

Jake Goulding (Jul 24 2018 at 20:17, on Zulip):

I've created these functions before, decomposing a type into a tuple of it's parts

Jake Goulding (Jul 24 2018 at 20:17, on Zulip):

I noticed hyper did something similar https://docs.rs/hyper/0.12.7/hyper/struct.Request.html#method.into_parts

simulacrum (Jul 24 2018 at 20:20, on Zulip):

With the proposal to special-case ManuallyDrop we could potentially make it special in that you can move out of fields from it too

simulacrum (Jul 24 2018 at 20:21, on Zulip):

So you'd do something like `let field = ManuallyDrop::new(bar).field;

Last update: Nov 21 2019 at 13:10UTC