Changes to GlobalAlloc #51160

Amanieu · 2018-05-29T10:34:07Z

As discussed with @SimonSapin at Rustfest Paris, this PR makes several changes to GlobalAlloc:

Opaque is changed from an extern type to a u8 newtype (with a private member). This allows .offset to work on *mut Opaque.
GlobalAlloc signatures are changed to take NonNull<Opaque> as parameters and return Result<NonNull<Opaque>, AllocErr>. This has the same size as *mut Opaque but matches the Alloc trait and generally makes it easier to write custom allocators in Rust.

SimonSapin · 2018-05-29T11:17:26Z

r=me

Since we’ve already done FCP to stabilize in #49668 (which may have been premature but oh well), let’s get team sign off:

@rfcbot fcp merge

rfcbot · 2018-05-29T11:17:27Z

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

typesafe-to-a-fault (Changes to GlobalAlloc #51160 (comment))

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

SimonSapin · 2018-05-29T11:06:20Z

src/liballoc/alloc.rs

    // We do not allocate for Box<T> when T is ZST, so deallocation is also not necessary.
    if size != 0 {
        let layout = Layout::from_size_align_unchecked(size, align);
-        Global.dealloc(ptr as *mut Opaque, layout);
+        Global.dealloc(NonNull::new_unchecked(ptr.as_ptr() as *mut Opaque), layout);


This can be NonNull::from(ptr).cast()

This can be NonNull::from(ptr).cast()

That's true in many other places. Same for things like ptr.as_ptr() as *mut u8 which can probably be ptr.cast().as_ptr()

My comment was mostly about using the safe From impl instead of the unsafe new_unchecked. I don’t have a strong opinion on cast() v.s. as.

gnzlbg · 2018-05-29T11:49:40Z

src/libcore/alloc.rs

+/// An opaque, byte-sized type. Used for pointers to allocated memory.
+///
+/// This type can only be used behind a pointer like `*mut Opaque` or `ptr::NonNull<Opaque>`.
+/// Such pointers are similar to C’s `void*` type.


Is this still the case?

rust-highfive · 2018-05-29T12:36:49Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:48:38] ................................................................i...................................
[00:48:43] ....................................................................................................
[00:48:49] ....................................................................................................
[00:48:55] .............................................................................................i......
[00:48:58] ...........iiiiiiiii...................................................
[00:48:58] 
[00:48:58] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:49:48] ................................................................i...................................
[00:49:53] ....................................................................................................
[00:49:58] ....................................................................................................
[00:50:04] .............................................................................................i......
[00:50:07] ...........iiiiiiiii...................................................
[00:50:07] 
[00:50:07]  finished in 68.804
[00:50:07] travis_fold:end:test_ui_nll

---
travis_time:start:test_run-pass
Check compiletest suite=run-pass mode=run-pass (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:50:07] 
[00:50:07] running 3015 tests
[00:50:21] ........FFF.........................................................................................
[00:50:54] ....................................................................................................
[00:51:09] ....................................................................................................
[00:51:22] ....................................................................................................
[00:51:44] ....................................................................................................
---
[00:59:16] ---- [run-pass] run-pass/allocator/custom.rs stdout ----
[00:59:16] 
[00:59:16] error: compilation failed!
[00:59:16] status: exit code: 101
[00:59:16] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/allocator/custom.rs" "--target=x86_64-unknown-linux-gnu" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/allocator/custom/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/allocator/custom/auxiliary"
[00:59:16] ------------------------------------------
[00:59:16] 
[00:59:16] ------------------------------------------
[00:59:16] stderr:
[00:59:16] stderr:
[00:59:16] ------------------------------------------
[00:59:16] error[E0053]: method `alloc` has an incompatible type for trait
[00:59:16]   --> /checkout/src/test/run-pass/allocator/custom.rs:26:5
[00:59:16]    |
[00:59:16] 26 |     unsafe fn alloc(&self, layout: Layout) -> *mut Opaque {
[00:59:16]    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found *-ptr
[00:59:16]    |
[00:59:16]    = note: expected type `unsafe fn(&A, std::heap::Layout) -> std::result::Result<std::ptr::NonNull<std::heap::Opaque>, std::heap::AllocErr>`
[00:59:16]               found type `unsafe fn(&A, std::heap::Layout) -> *mut std::heap::Opaque`
[00:59:16] 
[00:59:16] error[E0053]: method `dealloc` has an incompatible type for trait
[00:59:16]   --> /checkout/src/test/run-pass/allocator/custom.rs:31:5
[00:59:16]    |
[00:59:16] 31 |     unsafe fn dealloc(&self, ptr: *mut Opaque, layout: Layout) {
[00:59:16]    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `std::ptr::NonNull`, found *-ptr
[00:59:16]    |
[00:59:16]    = note: expected type `unsafe fn(&A, std::ptr::NonNull<std::heap::Opaque>, std::heap::Layout)`
[00:59:16]               found type `unsafe fn(&A, *mut std::heap::Opaque, std::heap::Layout)`
[00:59:16] error: aborting due to 2 previous errors
[00:59:16] 
[00:59:16] For more information about this error, try `rustc --explain E0053`.
[00:59:16] 
[00:59:16] 
[00:59:16] ------------------------------------------
[00:59:16] 
[00:59:16] thread '[run-pass] run-pass/allocator/custom.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3053:9
[00:59:16] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:59:16] 
[00:59:16] ---- [run-pass] run-pass/allocator/xcrate-use.rs stdout ----
[00:59:16] 
[00:59:16] error: auxiliary build of "/checkout/src/test/run-pass/allocator/auxiliary/custom.rs" failed to compile: 
[00:59:16] status: exit code: 101
[00:59:16] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/allocator/auxiliary/custom.rs" "--target=x86_64-unknown-linux-gnu" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/allocator/xcrate-use/auxiliary" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/allocator/xcrate-use/auxiliary"
[00:59:16] ------------------------------------------
[00:59:16] 
[00:59:16] ------------------------------------------
[00:59:16] stderr:
[00:59:16] stderr:
[00:59:16] ------------------------------------------
[00:59:16] error[E0053]: method `alloc` has an incompatible type for trait
[00:59:16]   --> /checkout/src/test/run-pass/allocator/auxiliary/custom.rs:22:5
[00:59:16]    |
[00:59:16] 22 |     unsafe fn alloc(&self, layout: Layout) -> *mut Opaque {
[00:59:16]    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found *-ptr
[00:59:16]    |
[00:59:16]    = note: expected type `unsafe fn(&A, std::heap::Layout) -> std::result::Result<std::ptr::NonNull<std::heap::Opaque>, std::heap::AllocErr>`
[00:59:16]               found type `unsafe fn(&A, std::heap::Layout) -> *mut std::heap::Opaque`
[00:59:16] 
[00:59:16] error[E0053]: method `dealloc` has an incompatible type for trait
[00:59:16]   --> /checkout/src/test/run-pass/allocator/auxiliary/custom.rs:27:5
[00:59:16]    |
[00:59:16] 27 |     unsafe fn dealloc(&self, ptr: *mut Opaque, layout: Layout) {
[00:59:16]    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `std::ptr::NonNull`, found *-ptr
[00:59:16]    |
[00:59:16]    = note: expected type `unsafe fn(&A, std::ptr::NonNull<std::heap::Opaque>, std::heap::Layout)`
[00:59:16]               found type `unsafe fn(&A, *mut std::heap::Opaque, std::heap::Layout)`
[00:59:16] error: aborting due to 2 previous errors
[00:59:16] 
[00:59:16] For more information about this error, try `rustc --explain E0053`.
[00:59:16] 
[00:59:16] 
[00:59:16] ------------------------------------------
[00:59:16] 
[00:59:16] thread '[run-pass] run-pass/allocator/xcrate-use.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3053:9
[00:59:16] 
[00:59:16] ---- [run-pass] run-pass/allocator/xcrate-use2.rs stdout ----
[00:59:16] 
[00:59:16] error: auxiliary build of "/checkout/src/test/run-pass/allocator/auxiliary/custom.rs" failed to compile: 
[00:59:16] status: exit code: 101
[00:59:16] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/allocator/auxiliary/custom.rs" "--target=x86_64-unknown-linux-gnu" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/allocator/xcrate-use2/auxiliary" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/allocator/xcrate-use2/auxiliary"
[00:59:16] ------------------------------------------
[00:59:16] 
[00:59:16] ------------------------------------------
[00:59:16] stderr:
[00:59:16] stderr:
[00:59:16] ------------------------------------------
[00:59:16] error[E0053]: method `alloc` has an incompatible type for trait
[00:59:16]   --> /checkout/src/test/run-pass/allocator/auxiliary/custom.rs:22:5
[00:59:16]    |
[00:59:16] 22 |     unsafe fn alloc(&self, layout: Layout) -> *mut Opaque {
[00:59:16]    |     ^^^^^^^^^87.rs" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/realloc-16687/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/realloc-16687/auxiliary"
[00:59:16] ------------------------------------------
[00:59:16] 
[00:59:16] ------------------------------------------
[00:59:16] stderr:
[00:59:16] stderr:
[00:59:16] ------------------------------------------
[00:59:16] error[E0599]: no method named `as_opaque` found for type `std::ptr::NonNull<u8>` in the current scope
[00:59:16]   --> /checkout/src/test/run-pass/realloc-16687.rs:67:52
[00:59:16]    |
[00:59:16] 67 |         Global.dealloc(NonNull::new_unchecked(ptr).as_opaque(), layout);
[00:59:16] 
[00:59:16] 
[00:59:16] error[E0599]: no method named `as_opaque` found for type `std::ptr::NonNull<u8>` in the current scope
[00:59:16]   --> /checkout/src/test/run-pass/realloc-16687.rs:75:62
[00:59:16]    |
[00:59:16] 75 |         let ret = Global.realloc(NonNull::new_unchecked(ptr).as_opaque(), old.clone(), new.size())
[00:59:16] 
[00:59:16] error: aborting due to 2 previous errors
[00:59:16] 
[00:59:16] For more information about this error, try `rustc --explain E0599`.
---
[00:59:16] 
[00:59:16] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:59:16] 
[00:59:16] 
[00:59:16] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:59:16] 
[00:59:16] 
[00:59:16] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:59:16] Build completed unsuccessfully in 0:13:05
[00:59:16] Build completed unsuccessfully in 0:13:05
[00:59:16] Makefile:58: recipe for target 'check' failed
[00:59:16] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:33c72396
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

sfackler · 2018-05-29T14:27:43Z

I'm not super opposed to the Result<NonNull<Unique>, AllocErr> type, but in what way does it make it easier to write allocators in Rust?

SimonSapin · 2018-05-29T15:43:30Z

With Result return types, impls can use the ? operator

gnzlbg · 2018-05-29T16:13:48Z

With Result return types, impls can use the ? operator

If we ever augment the error with state Result would also be required.

rust-highfive · 2018-05-29T16:44:56Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:45:46] ................................................................i...................................
[00:45:50] ....................................................................................................
[00:45:56] ....................................................................................................
[00:46:02] .............................................................................................i......
[00:46:05] ...........iiiiiiiii...................................................
[00:46:05] 
[00:46:05] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:46:54] ................................................................i...................................
[00:46:59] ....................................................................................................
[00:47:04] ....................................................................................................
[00:47:10] .............................................................................................i......
[00:47:13] ...........iiiiiiiii...................................................
[00:47:13] 
[00:47:13]  finished in 67.401
[00:47:13] travis_fold:end:test_ui_nll

---
[01:28:21] 
[01:28:21] failures:
[01:28:21] 
[01:28:21] ---- /checkout/src/doc/unstable-book/src/language-features/global-allocator.md - global_allocator (line 29) stdout ----
[01:28:21] error[E0053]: method `alloc` has an incompatible type for trait
[01:28:21]   --> /checkout/src/doc/unstable-book/src/language-features/global-allocator.md:38:5
[01:28:21]    |
[01:28:21] 10 |     unsafe fn alloc(&self, layout: Layout) -> *mut Opaque {
[01:28:21]    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found *-ptr
[01:28:21]    |
[01:28:21]    = note: expected type `unsafe fn(&MyAllocator, std::heap::Layout) -> std::result::Result<std::ptr::NonNull<std::heap::Opaque>, std::heap::AllocErr>`
[01:28:21]               found type `unsafe fn(&MyAllocator, std::heap::Layout) -> *mut std::heap::Opaque`
[01:28:21] 
[01:28:21] error[E0053]: method `dealloc` has an incompatible type for trait
[01:28:21]   --> /checkout/src/doc/unstable-book/src/language-features/global-allocator.md:42:5
[01:28:21]    |
[01:28:21] 14 |     unsafe fn dealloc(&self, ptr: *mut Opaque, layout: Layout) {
[01:28:21]    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `std::ptr::NonNull`, found *-ptr
[01:28:21]    |
[01:28:21]    = note: expected type `unsafe fn(&MyAllocator, std::ptr::NonNull<std::heap::Opaque>, std::heap::Layout)`
[01:28:21]               found type `unsafe fn(&MyAllocator, *mut std::heap::Opaque, std::heap::Layout)`
[01:28:21] thread '/checkout/src/doc/unstable-book/src/language-features/global-allocator.md - global_allocator (line 29)' panicked at 'couldn't compile the test', librustdoc/test.rs:325:17
[01:28:21] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:28:21] 
[01:28:21] 
---
[01:28:21] 
[01:28:21] 
[01:28:21] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:28:21] Build completed unsuccessfully in 0:44:54
[01:28:21] make: *** [check] Error 1
[01:28:21] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:010d4bb8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

alexcrichton · 2018-05-29T17:43:06Z

@rfcbot concern typesafe-to-a-fault

I have yet to be personally convinced that Result<NonNull<Foo>, AllocErr> offers benefit over *mut Foo. They both have the same memory representation and convey the same meaning. The former, Result, has the advantage of allowing you to extract a value that is statically known to be non-null and also works with ?.

The cons of this approach, however, I feel are:

This is a wordy signature. Four separate types are mentioned here (Result, NonNull, Foo, and AllocErr). Two of these probably have to be imported (NonNull and AllocErr).
The actual tangible benefit of this seems like it's very small. It seems like allocators would have to do a lot of juggling to make sure the types are all aligned and mentioned in the right place.
For global allocators, this is a signature that's practically never called. Instead users are almost always implementing a global allocator rather than invoking one. In that sense I don't seen much real need to optimize for ergonomics of callers here.
Most global allocator impementations today are actually coming from raw pointers. Things like malloc itself, jemalloc, dlmalloc, etc. None of them use Result as they're all defined in C, so it's yet another layer of extra work to do to add these Rust-specific interfaces.

Overall I feel the cons outweigh the pros for global allocators. I personally feel the same way for the Alloc trait as well (which has not been stabilized) in that raw pointers would be better suited than Result. In any case, though, we have not stabilized Alloc and AFAIK we're still somewhat far away from that, so there's no need for its unstable designed to influence something that's already had an FCP for stabilization.

Finally I believe the intention with *mut Opaque was to add impl *mut Opaque { fn offset(...) }. I'm not sure if this has been attempted though, but does it not work?

SimonSapin · 2018-05-29T19:25:16Z

Name resolution for extern type methods is also buggy at the moment: #46665

SimonSapin · 2018-05-29T19:31:41Z

I don’t feel strongly about the result vs nullable, I don’t mind not doing that change. Its benefit is small, but I find that the benefit of diverging from Alloc is also small.

Regarding changing Opaque to a struct instead an extern type, the more important point IMO is that extern types are unlikely to be stabilized in this release cycle (or soon after) whereas I’m hoping to do so for GlobalAlloc.

rust-highfive · 2018-05-29T19:56:03Z

The job mingw-check of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:09:12] configure: llvm.assertions      := True
[00:09:12] configure: build.locked-deps    := True
[00:09:12] configure: llvm.ccache          := sccache
[00:09:12] configure: build.openssl-static := True
[00:09:12] configure: build.configure-args := ['--enable-quiet-tests', '--enable-sccache', ' ...
[00:09:12] configure: writing `config.toml` in current directory
[00:09:12] configure: 
[00:09:12] configure: run `python /checkout/x.py --help`
[00:09:12] configure: 
---
[00:15:07]     Checking libc v0.0.0 (file:///checkout/src/rustc/libc_shim)
[00:15:07]     Checking alloc v0.0.0 (file:///checkout/src/liballoc)
[00:15:07]     Checking std_unicode v0.0.0 (file:///checkout/src/libstd_unicode)
[00:15:08]     Checking alloc_system v0.0.0 (file:///checkout/src/liballoc_system)
[00:15:08] error[E0605]: non-primitive cast: `core::ptr::NonNull<core::alloc::Opaque>` as `*mut u8`
[00:15:08]    --> liballoc_system/lib.rs:287:54
[00:15:08]     |
[00:15:08] 287 |                     HeapReAlloc(GetProcessHeap(), 0, ptr as LPVOID, new_size) as *mut Opaque
[00:15:08]     |
[00:15:08]     = note: an `as` expression can only be used to convert between primitive types. Consider using the `From` trait
[00:15:08] 
[00:15:08] error: aborting due to previous error
[00:15:08] error: aborting due to previous error
[00:15:08] 
[00:15:08] For more information about this error, try `rustc --explain E0605`.
[00:15:08] error: Could not compile `alloc_system`.
[00:15:08] 
[00:15:08] Caused by:
[00:15:08]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name alloc_system liballoc_system/lib.rs --color always --error-format json --crate-type lib --emit=dep-info,metadata -C opt-level=3 -C metadata=920c1d87f1854d18 -C extra-filename=-920c1d87f1854d18 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/i686-pc-windows-gnu/release/deps --target i686-pc-windows-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/i686-pc-windows-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/release/deps --extern compiler_builtins=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/i686-pc-windows-gnu/release/deps/libcompiler_builtins-e74d4e2453f9ac82.rmeta --extern libc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/i686-pc-windows-gnu/release/deps/liblibc-2924b7d63d95589e.rmeta --extern core=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/i686-pc-windows-gnu/release/deps/libcore-0e63b5d8cba5e64e.rmeta -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/i686-pc-windows-gnu/release/build/compiler_builtins-7a2aed77e6522e53/out` (exit code: 101)
[00:16:15] error: build failed
[00:16:15] error: build failed
[00:16:15] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "i686-pc-windows-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:16:15] expected success, got: exit code: 101
[00:16:15] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1091:9
[00:16:15] travis_fold:end:stage0-std

[00:16:15] travis_time:end:stage0-std:start=1527619406781149225,finish=1527619498288126868,duration=91506977643

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

alexcrichton · 2018-05-29T20:40:18Z

If we can't get Opaque working with offset due to various bugs then I think we should just use u8 personally. This is a global allocator that is, again, not called frequently.

I also don't think we need to strive for matching Alloc. The Alloc trait is not ready for stabilization and hasn't been as heavily scrutinized historically AFAIK.

glandium · 2018-05-29T20:43:25Z

Overall I feel the cons outweigh the pros for global allocators. I personally feel the same way for the Alloc trait as well (which has not been stabilized) in that raw pointers would be better suited than Result

FWIW, changing the return type of the Alloc trait methods to Result made a lot of things simpler in the callers.

bors · 2018-05-30T14:30:05Z

☔ The latest upstream changes (presumably #50880) made this pull request unmergeable. Please resolve the merge conflicts.

SimonSapin · 2018-05-30T18:18:39Z

Per libs team discussion today we’re ready to stabilize GlobalAlloc after a few changes, but we won’t be taking either change proposed here: #49668 (comment). (I plan to make a new PR for that today and start a final Final Comment Period there.)

Thanks for the PR @Amanieu ! (And sorry I lead you to doing work that didn’t end up being used.)

rust-highfive assigned SimonSapin May 29, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2018

SimonSapin added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 29, 2018

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 29, 2018

SimonSapin approved these changes May 29, 2018

View reviewed changes

SimonSapin mentioned this pull request May 29, 2018

Tracking issue for the GlobalAlloc trait and related APIs #49668

Closed

8 tasks

Amanieu force-pushed the opaque branch from 67e2787 to da3e4e6 Compare May 29, 2018 11:35

gnzlbg reviewed May 29, 2018

View reviewed changes

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 29, 2018

Amanieu force-pushed the opaque branch from 6e632d5 to 826d066 Compare May 29, 2018 20:04

Amanieu added 4 commits May 30, 2018 16:16

Use a newtype instead of an extern type for Opaque

38e2d54

Change GlobalAlloc to use Result and NonNull instead of raw pointers

29e3fbc

Fix tests

ea89051

Fix documentation

a0fffee

Amanieu force-pushed the opaque branch from 826d066 to a0fffee Compare May 30, 2018 15:18

SimonSapin closed this May 30, 2018

rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 30, 2018

SimonSapin mentioned this pull request May 7, 2019

Coherent design of GlobalAlloc and Alloc rust-lang/wg-allocators#21

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to GlobalAlloc #51160

Changes to GlobalAlloc #51160

Amanieu commented May 29, 2018

SimonSapin commented May 29, 2018

rfcbot commented May 29, 2018 •

edited

Loading

SimonSapin May 29, 2018

glandium May 29, 2018

SimonSapin May 29, 2018

gnzlbg May 29, 2018

rust-highfive commented May 29, 2018

sfackler commented May 29, 2018

SimonSapin commented May 29, 2018

gnzlbg commented May 29, 2018

rust-highfive commented May 29, 2018

alexcrichton commented May 29, 2018

SimonSapin commented May 29, 2018

SimonSapin commented May 29, 2018

rust-highfive commented May 29, 2018

alexcrichton commented May 29, 2018

glandium commented May 29, 2018

bors commented May 30, 2018

SimonSapin commented May 30, 2018

Changes to GlobalAlloc #51160

Changes to GlobalAlloc #51160

Conversation

Amanieu commented May 29, 2018

SimonSapin commented May 29, 2018

rfcbot commented May 29, 2018 • edited Loading

SimonSapin May 29, 2018

Choose a reason for hiding this comment

glandium May 29, 2018

Choose a reason for hiding this comment

SimonSapin May 29, 2018

Choose a reason for hiding this comment

gnzlbg May 29, 2018

Choose a reason for hiding this comment

rust-highfive commented May 29, 2018

sfackler commented May 29, 2018

SimonSapin commented May 29, 2018

gnzlbg commented May 29, 2018

rust-highfive commented May 29, 2018

alexcrichton commented May 29, 2018

SimonSapin commented May 29, 2018

SimonSapin commented May 29, 2018

rust-highfive commented May 29, 2018

alexcrichton commented May 29, 2018

glandium commented May 29, 2018

bors commented May 30, 2018

SimonSapin commented May 30, 2018

rfcbot commented May 29, 2018 •

edited

Loading