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

Potential rustc bug related to deriving ByteEq #217

Closed
LilyIsTrans opened this issue Jan 4, 2024 · 2 comments · Fixed by #219
Closed

Potential rustc bug related to deriving ByteEq #217

LilyIsTrans opened this issue Jan 4, 2024 · 2 comments · Fixed by #219

Comments

@LilyIsTrans
Copy link

Right off the bat, here's the minimum reproducible example:
Cargo.toml:

[package]
name = "bug_rep"
version = "0.1.0"
edition = "2021"

[dependencies]
bytemuck = { version = "1.14.0", features = ["derive"] }
#[derive(bytemuck::ByteEq)]
pub struct Magnitude<const BYTES: usize>([u8; 0]);

The bug still occurs identically if swapping out bytemuck::ByteEq for bytemuck::ByteHash.

The error from cargo check is:

error[E0107]: missing generics for struct `Magnitude`
 --> src/lib.rs:2:12
  |
2 | pub struct Magnitude<const BYTES: usize>([u8; 0]);
  |            ^^^^^^^^^ expected 1 generic argument
  |
note: struct defined here, with 1 generic parameter: `BYTES`
 --> src/lib.rs:2:12
  |
2 | pub struct Magnitude<const BYTES: usize>([u8; 0]);
  |            ^^^^^^^^^ ------------------
help: add missing generic argument
  |
2 | pub struct Magnitude<BYTES><const BYTES: usize>([u8; 0]);
  |                     +++++++

For more information about this error, try `rustc --explain E0107`.
error: could not compile `bug_rep` (lib) due to 2 previous errors

Following it's inane suggestion, unsurprisingly, does not fix the issue.

I got this from the rather more reasonable code:

[package]
name = "magnitude"
version = "0.1.0"
edition = "2021"

[dependencies]
bytemuck = { version = "1.14.0", features = ["derive", "min_const_generics"] }
#[derive(Clone, Copy, bytemuck::Zeroable, bytemuck::Pod, bytemuck::ByteEq, bytemuck::ByteHash)]
#[repr(transparent)]
pub struct Magnitude<const BYTES: usize>([u8; BYTES]);

Which gives an identical error message other than that it shows the longer source code.

    Checking magnitude v0.1.0 (/home/lily/dev/magnitude)
error[E0107]: missing generics for struct `Magnitude`
 --> src/static_precision/mod.rs:3:12
  |
3 | pub struct Magnitude<const BYTES: usize>([u8; BYTES]);
  |            ^^^^^^^^^ expected 1 generic argument
  |
note: struct defined here, with 1 generic parameter: `BYTES`
 --> src/static_precision/mod.rs:3:12
  |
3 | pub struct Magnitude<const BYTES: usize>([u8; BYTES]);
  |            ^^^^^^^^^ ------------------
help: add missing generic argument
  |
3 | pub struct Magnitude<BYTES><const BYTES: usize>([u8; BYTES]);
  |                     +++++++

For more information about this error, try `rustc --explain E0107`.
error: could not compile `magnitude` (lib) due to 3 previous errors

Considering how inane the suggestion from rustc is, I suspect this may actually be a compiler bug, but I want to get a second opinion before I go opening an issue claiming a compiler error, especially for code involving macros I didn't write.

@zachs18
Copy link
Contributor

zachs18 commented Jan 4, 2024

(cargo expand output on the example given above)

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
pub struct Magnitude<const BYTES : usize>([u8; 0]);
impl ::core::cmp::PartialEq for Magnitude {
    #[inline]
    #[must_use]
    fn eq(&self, other: &Self) -> bool {
        ::bytemuck::bytes_of(self) == ::bytemuck::bytes_of(other)
    }
}
impl ::core::cmp::Eq for Magnitude {}

This isn't a compiler bug, the ByteEq derive macro doesn't currently emit generics on the PartialEq and Eq impls (which it probably should, or should emit an error that it doesn't support generics).

@zachs18
Copy link
Contributor

zachs18 commented Jan 6, 2024

Opened a PR to allow deriving ByteEq and ByteHash for types with generics.

Note that ByteEq and ByteHash require the type implement NoUninit, which cannot currently be derived for generic structs, and must be manually verified to be correct and implemented.

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

Successfully merging a pull request may close this issue.

2 participants