Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Differing results for std::mem::size_of computation #115028

Closed
feds01 opened this issue Aug 20, 2023 · 2 comments
Closed

Differing results for std::mem::size_of computation #115028

feds01 opened this issue Aug 20, 2023 · 2 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@feds01
Copy link

feds01 commented Aug 20, 2023

I have a project that is split into two repositories:

The hi repository specifies through cargo to be dependent on the master branch of hashc.

hashc repository crates contain some static_assert_size! assertions, which are implemented in the following way (exactly as it is in rustc itself):

pub macro static_assert_size($ty:ty, $size:expr) {
    const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
}

Building hashc on its own with the following assertions in place does not produce an error:

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
mod size_asserts {
    use hash_utils::assert::static_assert_size;

    use super::*;

    static_assert_size!(Statement, 64);
    static_assert_size!(Terminator, 104);
    static_assert_size!(RValue, 40);
}

However, building hi repository trips one of the assertions as having a mismatching size:

error[E0308]: mismatched types
    --> /Users/alex/.cargo/git/checkouts/hashc-3190687590a997bc/556827f/compiler/hash-utils/src/assert.rs:6:28
     |
5    | pub macro static_assert_size($ty:ty, $size:expr) {
     | ---------------------------- in this expansion of `static_assert_size!`
6    |     const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 104 elements, found one with 96 elements
     |
    ::: /Users/alex/.cargo/git/checkouts/hashc-3190687590a997bc/556827f/compiler/hash-ir/src/ir.rs:1534:37
     |
1534 |     static_assert_size!(Terminator, 104);
     |     ------------------------------------
     |     |                               |
     |     |                               help: consider specifying the actual array length: `96`
     |     in this macro invocation

It is bizarre that this is happening on the same machine with the same toolchain, but the crates are being required through a repository rather than a local crate.

rustc --version --verbose:

rustc 1.73.0-nightly (6ef7d16be 2023-08-19)
binary: rustc
commit-hash: 6ef7d16be0fb9d6ecf300c27990f4bff49d22d46
commit-date: 2023-08-19
host: x86_64-apple-darwin
release: 1.73.0-nightly
LLVM version: 17.0.0
@feds01 feds01 added the C-bug Category: This is a bug. label Aug 20, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 20, 2023
@feds01 feds01 changed the title Differing results for std::mem::size_of computation in Differing results for std::mem::size_of computation Aug 20, 2023
@feds01
Copy link
Author

feds01 commented Aug 20, 2023

My apologies, after a more detailed investigation this seems as if it is an issue with updating project dependencies. One of the project dependencies smallvec is specified by the hashc lockfile to be at 1.10.0, but running cargo update bumps smallvec to 1.11.0 which introduces the breaking change. Should this be closed, or perhaps moved to cargo repository?

@Noratrieb
Copy link
Member

Doesn't look like a bug to me then, just a confusing consequence of checking in library lockfiles. One thing is that emitting hard errors in size asserts is a pretty bad thing in general to do as changes in the layout algorithm (or as we've seen here, in dependencies) could cause build failures. They should be #[test]s instead (rustc is special here because it controls the layout algorithm :D).
Thanks for the report anyways!

@Noratrieb Noratrieb closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2023
@Noratrieb Noratrieb added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 22, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this issue Aug 22, 2023
Add disclaimer on size assertion macro

Sometimes people are inspired by rustc to add size assertions to their code and copy the macro. This is bad because it causes hard build errors. rustc happens to be special where it makes this okay.

For example, see rust-lang#115028 (not sure whether they were directly inspired by this function), but I think I've also seen other cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

No branches or pull requests

3 participants