Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

Update the derive crate #100

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Michael-F-Bryan
Copy link

@Michael-F-Bryan Michael-F-Bryan commented Sep 7, 2018

I just encountered an issue where the heapSizeOf derive wasn't able to parse dyn Trait in a struct, so I thought I'd make a PR to update heapsize_derive to use the latest versions of syn and quote... Unfortunately almost all of syn's API has changed since the custom derive was originally written, so the simple cargo update turned into a rewrite 😞

We'll probably want to test this PR against a large-ish codebase that already uses heapsize to make sure I didn't accidentally introduce any regressions.


This change is Reviewable

@Michael-F-Bryan
Copy link
Author

When trying to run the tests on my machine I encountered some funny behaviour with Box<T>. Does anyone know why these tests might be failing?

#[test]
fn sanity_check_heapsize_works_as_expected() {
    assert_eq!(Box::new(1_u8).heap_size_of_children(), mem::size_of::<u8>());
    assert_eq!([Box::new(1), Box::new(2)].heap_size_of_children(), 2 * 4);
}

@jdm
Copy link
Member

jdm commented Sep 7, 2018

What is the test failure message?

@Michael-F-Bryan
Copy link
Author

This is what I'm seeing when running the tests:

---- sanity_check_heapsize_works_as_expected stdout ----
thread 'sanity_check_heapsize_works_as_expected' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `1`', test.rs:21:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- tuple_struct stdout ----
thread 'tuple_struct' panicked at 'assertion failed: `(left == right)`
  left: `24`,
 right: `9`', test.rs:13:5

---- normal_struct stdout ----
thread 'normal_struct' panicked at 'assertion failed: `(left == right)`
  left: `32`,
 right: `17`', test.rs:43:5

@jdm It looks like Box::new(1_u8).heap_size_of_children() is returning the size of the box (i.e. the 64-bit pointer) instead of the size of the u8.

@Michael-F-Bryan
Copy link
Author

Michael-F-Bryan commented Sep 8, 2018

@jdm I made appveyor test the heapsize_derive crate and apparently the sanity test is now passing on Windows.

Perhaps this is something particular to whatever allocator is being used on my machine? Apparently the amount of heap memory used by a Box<[f64; 10]> is zero...

#[test]
fn sanity_check_heapsize_works_as_expected() {
    let array = [1.0_f64; 10];
    let boxed = Box::new(array);

    assert_eq!(boxed.heap_size_of_children(), mem::size_of_val(&array));
}
---- sanity_check_heapsize_works_as_expected stdout ----
thread 'sanity_check_heapsize_works_as_expected' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `80`', test.rs:24:5

@Michael-F-Bryan
Copy link
Author

It looks like the CI failure is because the top-level heapsize crate contains tests which depend on nightly APIs that no longer exist (in particular, the allocator_api feature which has been moved around).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants