Member

This comment has been hidden.

Amanieu commented on Oct 29

@bors try

bors commented on Oct 29

Trying commit 51f24f6 with merge 1b5ce95...

bors commented on Oct 29

Try build successful - checks-actions
Build commit: 1b5ce95 (1b5ce95284bc3170a1f4a55c6ec8a06b3e85ff73)

Amanieu commented on Oct 29

@craterbot check

craterbot commented on Oct 29

Experiment pr-78461 created and queued.
Automatically detected try build 1b5ce95
You can check out the queue and this experiment's details.

Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot commented on Nov 5

Experiment pr-78461 is now running

Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot commented on Nov 8

Experiment pr-78461 is completed!
5 regressed and 10 fixed (128186 total)
Open the full report.

If you notice any spurious failure please add them to the blacklist!
Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

KodrAus commented on Nov 18

The Allocator WG has a roadmap here that this all fits into: rust-lang/wg-allocators#7

This is implementing this part of the relevant RFC.

The linked issue about an allocator-aware Box<T, A> was closed, but it looks like we did land it in a later PR: #77187

TimDiekmann commented on Nov 18

The linked pull request #71873 was used to test, if a crater run will break anything and was never supposed to be merged.

KodrAus commented on Nov 18

Yeh, I haven’t been closely following the Allocators WG so when I first saw this PR I thought it looked like a really big change compared to the amount of context we had in the OP, so thought I’d drop in a few links to things I found while trying to catch up

Is there any other background that might be good to have here?

TimDiekmann commented on Nov 18

Yeah, very true, I added the roadmap to the OP. Just before #77187 we solved all blocking issues on the roadmap to keep the generic parameter unstable.

Amanieu commented on Nov 19

@bors r+ rollup=never

bors commented on Nov 19

Commit 51f24f6 has been approved by Amanieu

bors commented on Nov 19

Testing commit 51f24f6 with merge f66719d...

bors commented on Nov 19

Test failed - checks-actions

bors commented on Nov 19

This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: #78449

bors commented on Nov 19

Commit a600410 has been approved by Amanieu

bors commented on Nov 19

Testing commit a600410 with merge e6a62b2...

bors commented on Nov 19

Test failed - checks-actions

TimDiekmann commented on Nov 20

Can't really say, why this failed. Somehow, lldb failed to print hash_map in debuginfo/pretty-std-collections.rs when testing stage 2:

// lldb-command:print hash_map
// lldbg-check:[...]$2 = size=4 { [0] = { 0 = 1 1 = 10 } [1] = { 0 = 2 1 = 20 } [2] = { 0 = 3 1 = 30 } [3] = { 0 = 4 1 = 40 } }
// lldbr-check:(std::collections::hash::map::HashMap<u64, u64, [...]>) hash_map = size=4 size=4 { [0] = { 0 = 1 1 = 10 } [1] = { 0 = 2 1 = 20 } [2] = { 0 = 3 1 = 30 } [3] = { 0 = 4 1 = 40 } }

Error:

Error while trying to register breakpoint callback, id = 1, message = error: could not get num args: can't find callable: breakpoint_callback

run
Process 65506 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 frame #0: 0x000000010003d3d9 a`pretty_std_collections::main::h1ef1d7ce5967b6a7 at pretty-std-collections.rs:158:5 155 hash_set.insert(i); 156 } 157 -> 158 zzz(); // #break ^ 159 } 160 161 fn zzz() { Target 0: (a) stopped. Process 65506 launched: '/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/debuginfo/pretty-std-collections.lldb/a' (x86_64) 
print vec_deque
(alloc::collections::vec_deque::VecDeque<int>) $0 = size=3 { [0] = 5 [1] = 3 [2] = 7 } 
print vec_deque2
(alloc::collections::vec_deque::VecDeque<int>) $1 = size=7 { [0] = 2 [1] = 3 [2] = 4 [3] = 5 [4] = 6 [5] = 7 [6] = 8 } 
print hash_map

------------------------------------------
stderr:
------------------------------------------
error: need to add support for DW_TAG_base_type '()' encoded with DW_ATE = 0x7, bit_size = 0
clangclang: CommandLine Error: Option ': CommandLine Error: Option 'h' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options
h' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options

bors commented 28 days ago

Testing commit a600410 with merge a1a13b2...

bors commented 28 days ago

Test successful - checks-actions
Approved by: Amanieu
Pushing a1a13b2 to master...

H2CO3 commented 22 days ago

Is there some place for discussing the fact that these changes break the layout and ABI of Box and Vec? If the allocator is not zero-sized, then we no longer have size_of::<Box<_>>() == size_of::<*mut T>() and size_of::<Vec<_>>() == 3 * size_of::<usize>(), which was guaranteed previously. This has paramount implications for FFI and unsafe code.

oli-obk commented 22 days ago

I would assume that any such guarantees only apply to literally Box<T> and Vec<T> and not Box<T, Something> or Vec<T, Something>. That said, I think that is best discussed in https://github.com/rust-lang/wg-allocators and not a merged PR, since such comments easily get lost.

TimDiekmann commented 21 days ago

I would assume that any such guarantees only apply to literally Box<T> and Vec<T> and not Box<T, Something> or Vec<T, Something>.

I'd say this applies to any Box<T, A> where A: ZST but, as suggested, the WG-repository is the right place.

H2CO3 commented 21 days ago

@oli-obk Thanks!