Shell scripts of repo not working due to wee_alloc compilation errors

[CPerezz]

Describe the Bug

Execution of .check, .build & .test fails due to a compilation problem.
I commented out the i686-pc-windows-gnu target-related checks since I can't run them in my env and the compilation of wee_alloc fails with output:

++ dirname ./check.sh
+ cd .
+ cd ./wee_alloc
+ cargo check
   Compiling wee_alloc v0.4.5 (path_to/wee_alloc/wee_alloc)
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
+ cargo check --target wasm32-unknown-unknown
   Compiling wee_alloc v0.4.5 (path_to/wee_alloc/wee_alloc)
    Finished dev [unoptimized + debuginfo] target(s) in 0.18s
+ cargo check --features size_classes
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
+ cargo check --features size_classes --target wasm32-unknown-unknown
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
+ cd -
path_to/wee_alloc
+ cd ./test
+ cargo check
   Compiling wee_alloc v0.4.5 (path_to/wee_alloc/wee_alloc)
    Checking env_logger v0.5.13
error[E0432]: unresolved imports `core::alloc::Alloc`, `core::alloc::AllocErr`
   --> wee_alloc/src/lib.rs:221:27
    |
221 |         use core::alloc::{Alloc, AllocErr};
    |                           ^^^^^  ^^^^^^^^
    |                           |      |
    |                           |      no `AllocErr` in `alloc`
    |                           |      help: a similar name exists in the module: `AllocError`
    |                           no `Alloc` in `alloc`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
error: could not compile `wee_alloc`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

It happens the same with the other scripts.

Steps to Reproduce

Just run any of check, build or test scripts.

Expected Behavior

I would expect the imports to be correct. Alloc does not exist in libcore, it was renamed to Allocator AFAIK. Same happens with AllocErr which was renamed to AllocError.

Also the Alloc trait-functions have been updated.

If I'm not missing something or ommiting any info. And the repo is still mantained, I would like to update that and solve this issue. Otherways, please, could you tell me what I'm missing ?

[rusty122]

I took a stab at fixing this here rusty122@ccae9a3

Current error is:

error[E0053]: method `allocate` has an incompatible type for trait
    --> wee_alloc/src/lib.rs:1149:5
     |
1149 |     fn allocate(&self, layout: Layout) -> Result<NonNull<u8>, AllocError> {
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected slice `[u8]`, found `u8`
     |
     = note: expected fn pointer `fn(&&'b WeeAlloc<'a>, Layout) -> std::result::Result<NonNull<[u8]>, _>`
                found fn pointer `fn(&&'b WeeAlloc<'a>, Layout) -> std::result::Result<NonNull<u8>, _>`

error: aborting due to previous error

instead of a plain u8, allocate expects a slice and it looks like the Rust implementation is moving to NonNull::slice_from_raw_parts. It seems like we may need the slice_from_raw_parts nightly feature to implement the GlobalAlloc as well

[CPerezz]

Hey sorry for not answering earlier.

You're right, at least I thought the same, I quickly did some changes with the new API and:

So if you do: #![cfg_attr( feature = "nightly", feature(nonnull_slice_from_raw_parts, slice_ptr_get) )]

Then you can refactor the internals to return what the actual core::Allocator::allocate requires: Result<NonNull<[u8]>, AllocError>.

This would look like:

@@ -914,7 +918,7 @@ unsafe fn alloc_first_fit<'a>(
     align: Bytes,
     head: &Cell<*const FreeCell<'a>>,
     policy: &dyn AllocPolicy<'a>,
-) -> Result<NonNull<u8>, AllocErr> {
+) -> Result<NonNull<[u8]>, AllocErr> {
     extra_assert!(size.0 > 0);
 
     walk_free_list(head, policy, |previous, current| {
@@ -922,7 +926,12 @@ unsafe fn alloc_first_fit<'a>(
 
         if let Some(allocated) = current.try_alloc(previous, size, align, policy) {
             assert_aligned_to(allocated.data(), align);
-            return Some(unchecked_unwrap(NonNull::new(allocated.data() as *mut u8)));
+            let ptr = allocated.data() as *mut u8;
+
+            return Some(NonNull::slice_from_raw_parts(
+                unchecked_unwrap(NonNull::new(ptr)),
+                align.0,
+            ));
         }
 
         None
@@ -934,7 +943,7 @@ unsafe fn alloc_with_refill<'a, 'b>(
     align: Bytes,
     head: &'b Cell<*const FreeCell<'a>>,
     policy: &dyn AllocPolicy<'a>,
-) -> Result<NonNull<u8>, AllocErr> {
+) -> Result<NonNull<[u8]>, AllocErr> {
     if let Ok(result) = alloc_first_fit(size, align, head, policy) {
         return Ok(result);
     }
@@ -1027,7 +1036,7 @@ impl<'a> WeeAlloc<'a> {
         })
     }
 
-    unsafe fn alloc_impl(&self, layout: Layout) -> Result<NonNull<u8>, AllocErr> {
+    unsafe fn alloc_impl(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocErr> {
         let size = Bytes(layout.size());
         let align = if layout.align() == 0 {
             Bytes(1)
@@ -1039,7 +1048,10 @@ impl<'a> WeeAlloc<'a> {
             // Ensure that our made up pointer is properly aligned by using the
             // alignment as the pointer.
             extra_assert!(align.0 > 0);
-            return Ok(NonNull::new_unchecked(align.0 as *mut u8));
+            return Ok(NonNull::slice_from_raw_parts(
+                NonNull::new_unchecked(align.0 as *mut u8),
+                align.0,
+            ));
         }
 
         let word_size: Words = checked_round_up_to(size).ok_or(AllocErr)?;
@@ -1146,11 +1158,11 @@ unsafe impl<'a, 'b> Alloc for &'b WeeAlloc<'a>
 where
     'a: 'b,
 {
-    unsafe fn alloc(&mut self, layout: Layout) -> Result<NonNull<u8>, AllocErr> {
-        self.alloc_impl(layout)
+    fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocErr> {
+        unsafe { self.alloc_impl(layout) }
     }
 
-    unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout) {
+    unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
         self.dealloc_impl(ptr, layout)
     }
 }
@@ -1158,7 +1170,7 @@ where
 unsafe impl GlobalAlloc for WeeAlloc<'static> {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         match self.alloc_impl(layout) {
-            Ok(ptr) => ptr.as_ptr(),
+            Ok(slice) => slice.get_unchecked_mut(0).as_ptr(),
             Err(AllocErr) => ptr::null_mut(),
         }
     }

Then you'll realize that GlobalAllocator::alloc can no longer call Allocator::allocate unless you do:

unsafe impl GlobalAlloc for WeeAlloc<'static> {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        match self.alloc_impl(layout) {
            Ok(slice) => slice.get_unchecked_mut(0).as_ptr(),
            Err(AllocErr) => ptr::null_mut(),
        }
    }

    unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
        if let Some(ptr) = NonNull::new(ptr) {
            self.dealloc_impl(ptr, layout);
        }
    }
}

But that will force you to use #![feature(nonnull_slice_from_raw_parts)] & #![feature(slice_ptr_get)] for the entire crate.
And considering that the API is experimental, and the Allocator one is too, I'm not really sure that making any changes is a good option since they're likely to fall outdated or not work.

It might be really good to get @fitzgen 's input on that.

[rusty122]

@CPerezz I think you can actually avoid enabling the experimental features by basically just inlining the Rust implementations of those methods. get_unchecked_mut for example can be implemented as unsafe { NonNull::new_unchecked(non_null_slice.as_ptr().get_unchecked_mut(index)) } to avoid enabling slice_ptr_get and slice_from_raw_parts can be done similarly.

Agreed that requiring nightly isn't great, but I think if we can update the repo to compile/run on both stable and nightly that would be good.

Couple implementation notes:

• I think that ptr.as_mut_ptr() should be the same as ptr.get_unchecked_mut(0).as_ptr() (https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.as_mut_ptr) and can be simplified to non_null_slice.as_ptr().as_mut_ptr() - get the pointer to the slice and then a mutable pointer to the start of the slice's buffer
• realized my slice_from_raw_parts logic has an error because it's passing the size as Words instead of Bytes, so need to convert the units in alloc_first_fit

I've made some more progress on this. Most tests are passing, but there are some failures in one or two because I managed to break an invariant in the free list - will update my branch sometime today

[rusty122]

Here's my updated branch: https://github.com/rusty122/wee_alloc/tree/update-renamed-imports-and-trait-signatures

Seems like there's a bug involving a CellHeader getting overwritten somehow :/

[rusty122]

nvm! Bug was that in the stress test I had updated the realloc method call to grow, but it should actually be shrink when the new layout is smaller than the original allocation layout. PR is up and passing the appveyor CI tests - seems like the Travis CI build hasn't been triggered, but not sure why (looks like it needs to get migrated to travis-ci.com in the next few weeks because travis-ci.org is shutting down?)

@fitzgen could you review when you get a chance, or is there someone else from the team that could review?