Stream: t-libs/wg-allocators

Topic: Box with allocator


Tim Diekmann (May 03 2019 at 06:26, on Zulip):

We should continue the discussion about Box here @Mike Hommey :)

Mike Hommey (May 03 2019 at 06:27, on Zulip):

Yes, that would be more effective than github.

Tim Diekmann (May 03 2019 at 06:31, on Zulip):

Do you want to create a new PR btw? There has to be changed quiet a bit.

Yes, I'm very excited about progressing on this :D

Mike Hommey (May 03 2019 at 06:40, on Zulip):

Not sure about a new PR. Has your DispatchFromDyn stuff been merged yet?

Tim Diekmann (May 03 2019 at 06:42, on Zulip):

Yes, it's merged

Mike Hommey (May 03 2019 at 06:44, on Zulip):

Aha! Let me refresh my patch right away, then.

Mike Hommey (May 03 2019 at 08:00, on Zulip):

Still getting

error[E0378]: the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, `PhantomData` fields, and nothing else

on master, with

impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Box<U>> for Box<T> {}
Tim Diekmann (May 03 2019 at 08:10, on Zulip):

Did you pull the latest master?

Tim Diekmann (May 03 2019 at 08:10, on Zulip):

AFAIK it was merged yesterday after dozens of rerolls

Mike Hommey (May 03 2019 at 08:12, on Zulip):

I'm on 08bfe16129b0621bc90184f8704523d4929695ef.

Mike Hommey (May 03 2019 at 08:12, on Zulip):

What was your PR?

Mike Hommey (May 03 2019 at 08:18, on Zulip):

Facepalm, this is stage0.

Tim Diekmann (May 03 2019 at 08:31, on Zulip):

Oh, yeah. This requires #[cfg(stage0)]. Take a look at TimDiekmann@94031fd :)

Mike Hommey (May 03 2019 at 08:33, on Zulip):

Do you think we can get your PR uplifted to beta, so that we can update the stage0 and not have to deal with the whole "duplicate boxed.rs" thing. Or we wait 3 more weeks...

Tim Diekmann (May 03 2019 at 08:38, on Zulip):

I'm not familiar with the beta uplifting process. Waiting 3 more weeks is pretty much IMO. With the duplicated boxed.rs it's not that much to be changed (figuring out the #[cfg(...)] flags with the combination of test and stage0 was the hardest part). When adding // FIXME: remove this ... this would be pretty straight forward to remove again, once the new stage0 compiler is used.

Mike Hommey (May 03 2019 at 08:40, on Zulip):

Considering all the things that still aren't resolved, and the time this has been baking, it doesn't feel like 3 weeks is not going to make a very large difference to me. @Simon Sapin, what do you think?

Simon Sapin (May 03 2019 at 10:10, on Zulip):

I agree that it’s unlikely everything else is gonna also be resolved in three weeks

Simon Sapin (May 03 2019 at 10:10, on Zulip):

But does this really require a new snapshot? Isn’t #[cfg(not(stage0))] working for this?

Mike Hommey (May 03 2019 at 10:32, on Zulip):

Virtually everything in boxed.rs needs to be duplicated for stage0/not(stage0), plus a few other things (like box_free, the some things in rc.rs, etc.)

Mike Hommey (May 03 2019 at 10:52, on Zulip):

Dammit, I had an idea for stage0 that could require less changes, but the base of it doesn't work... https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=e5025d6663e4db30f8084ec224e7c517
despite the type definitely being a PhantomData...
it works on nightly... fun...

Mike Hommey (May 03 2019 at 10:56, on Zulip):

this doesn't work either:
https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=aaf4b6d813686f71d43d2fc5b7527267
(it does in nightly, again ; this is a frustrating game)

Mike Hommey (May 03 2019 at 10:59, on Zulip):

so in beta, if it's not _exactly_ a PhantomData, it doesn't work. Not an associated type that resolves to a PhantomData.
And a type alias that resolves to a PhantomData doesn't work either, because they can't be used as a constructor ; plus doing type Global = PhantomData<Foo> doesn't work for the impl Alloc for Global because PhantomData is not a local type.

Mike Hommey (May 03 2019 at 11:06, on Zulip):

Ok, I have another idea that might work.

Simon Sapin (May 03 2019 at 11:22, on Zulip):

you could have #[cfg] on modules :)

Simon Sapin (May 03 2019 at 11:22, on Zulip):

or #[cfg_attr(stage0, path = "boxed_stage0.rs")]

Mike Hommey (May 03 2019 at 11:23, on Zulip):

Doesn't solve the problem for the stuff that is not in boxed.rs. But it looks like my other idea works.

Mike Hommey (May 03 2019 at 12:44, on Zulip):

Anyone has an idea what's going on? I'm getting

error[E0038]: the trait `std::boxed::FnBox` cannot be made into an object] 3/4: test
   --> src/libtest/lib.rs:176:15
    |
176 |     DynTestFn(Box<dyn FnBox() + Send>),
    |               ^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::boxed::FnBox` cannot be made into an object
    |
    = note: method `call_box`'s receiver cannot be dispatched on

error[E0038]: the trait `std::boxed::FnBox` cannot be made into an object
    --> src/libtest/lib.rs:1446:5
     |
1446 | /     fn run_test_inner(
1447 | |         desc: TestDesc,
1448 | |         monitor_ch: Sender<MonitorMsg>,
1449 | |         nocapture: bool,
...    |
1491 | |         }
1492 | |     }
     | |_____^ the trait `std::boxed::FnBox` cannot be made into an object
     |
     = note: method `call_box`'s receiver cannot be dispatched on

with https://github.com/glandium/rust/commit/b4dc331198c95a77f7e7a15b58a0312791b4a564

Simon Sapin (May 03 2019 at 12:52, on Zulip):

@Mike Hommey maybe the Receiver trait?

Simon Sapin (May 03 2019 at 12:52, on Zulip):

Or DispatchFromDyn? As in, extend the impls to be allocator-generic

Mike Hommey (May 03 2019 at 12:54, on Zulip):

That's the whole problem: DispatchFromDyn only works if the second field in Box is a ZST

Mike Hommey (May 03 2019 at 12:55, on Zulip):

But yeah, it's because of the lack of generic argument to DispatchFromDyn... maybe that's the difference with Receiver that breaks it?

Tim Diekmann (May 03 2019 at 12:55, on Zulip):

As already mentioned above, this is a working version with stage0 cfg: TimDiekmann@94031fd

Mike Hommey (May 03 2019 at 12:57, on Zulip):

doesn't make sense

Mike Hommey (May 03 2019 at 13:05, on Zulip):

mmmmm could it be related to the fact that I forgot to patch FnOnce and FnMut?

Mike Hommey (May 03 2019 at 13:09, on Zulip):

or maybe the problem is that I _did_ patch FnBox

Mike Hommey (May 03 2019 at 13:11, on Zulip):

yup, that was it

Mike Hommey (May 03 2019 at 14:18, on Zulip):

Updated PR58457

Tim Diekmann (May 03 2019 at 16:57, on Zulip):

I like the idea of the PhantomData workaround, but you shouldn't put too much effort into it, as it's just a workaround for three weeks. When duplicating boxed.rs, the dup can be removed without touching the file again :)

Last update: Nov 15 2019 at 09:45UTC