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

Why does Pod not work with 2 generic-fields? #278

Open
Lokathor opened this issue Oct 15, 2024 Discussed in #277 · 5 comments
Open

Why does Pod not work with 2 generic-fields? #278

Lokathor opened this issue Oct 15, 2024 Discussed in #277 · 5 comments
Labels
proc-macros I don't do proc-macros, but I accepts PRs about them.

Comments

@Lokathor
Copy link
Owner

Lokathor commented Oct 15, 2024

Discussed in #277

Originally posted by DasLixou October 15, 2024
I have a struct

struct MyStruct<T> {
    a: T,
    b: T,
}

and deriving Pod on it errors because multiple generics don't seem to be allowed, but I can't find any documentation of the reason.
So, stupidly asked, why?


at first glance it looks like that code should work? did you also use repr(C) in the actual code? if repr(C) is in the picture, then your type is basically the same as [T; 2].

If that doesn't work then probably it's just that the proc-macro derive isn't sufficiently advanced, and someone could probably do a PR to support that. I'm going to make this into an issue so that this can be checked on longer term.

@Lokathor Lokathor added the proc-macros I don't do proc-macros, but I accepts PRs about them. label Oct 15, 2024
@DasLixou
Copy link

https://github.com/Lokathor/bytemuck/blob/main/derive/src/traits.rs#L69-L78
seem to be the lines in charge of the problem.
Currently, repr(C) isn't allowed in that scenario because it isn't repr(packed), but repr(packed) isn't allowed either because it isn't repr(C) (and also unsafe to use)

@DasLixou
Copy link

If the derive macro already assures that the generics are : Pod (please correct/assure me on that :D), then changing the packed requirement to just an repr(C) should be fine iguess?

@Lokathor
Copy link
Owner Author

The proc-macro is mostly watched over by @fu5ha and @danielhenrymantilla , but yes that sounds correct

@danielhenrymantilla
Copy link
Contributor

In order to handle the general <T>-generic case, we'd need to be able to emit the following code:

#![feature(generic_const_exprs)]

pub struct MyStruct<T> {
    a: T,
    b: u8,
}

pub unsafe trait Pod {}

unsafe
impl<T> Pod for MyStruct<T>
where
    // no padding (among other things)
    [(); 0 - ( // overflows if the following predicate is true
        ::core::mem::size_of::<Self>() // is the size of the whole strictly bigger than
                                       // the sum of its parts? It means we got padding.
        >
        (
            0
            + ::core::mem::size_of::<T>() // a
            + ::core::mem::size_of::<u8>() // b
        )
    ) as usize]:,
{}
  • so that MyStruct<u16> does not get to be Pod.

  • Playground

But this requires generic_const_exprs, which is not only nightly-only, but actually, an incomplete_feature ⚠️


Here, we could introduce a very specific subset of syntactic heuristics. The one that could apply here, since it's a pattern which is not completely unheard of, is the "array with named indices", sort to speak, such as Vec3<T> in many 3D engines and whatnot:

struct Name<SingleGeneric> {
    a: SingleGeneric,
    bunch: SingleGeneric,
    of: SingleGeneric,
    fields: SingleGeneric,
    all: SingleGeneric,
    with: SingleGeneric,
    the: SingleGeneric,
    same: SingleGeneric,
    type: SingleGeneric,
}
  • (we could even allow any more complex expression of SingleGeneric, on condition that it be equal for each field, but this, then, could run into two issues:

    • types may contain macro invocations, which in turn may yield distinct types;
    • types may involve span and hygiene (e.g., $crate), wherein distinct choices of that hygiene and span could yield semantically distinct types, but whose .to_string() comparison in the macro reälm would compare equal.

    I guess we could try to instead defer to Rust, and emit a where clause along the lines of FirstField : IsEqualTo<SecondField>, SecondField : IsEqualTo<ThirdField>, /* etc. */ 🤔)

@Lokathor
Copy link
Owner Author

I think at least trying to support deriving when there's all fields the same type would be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proc-macros I don't do proc-macros, but I accepts PRs about them.
Projects
None yet
Development

No branches or pull requests

3 participants