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

Make AnyBitPattern derive work for generic structs #101

Merged

Conversation

cky
Copy link
Contributor

@cky cky commented Apr 21, 2022

(Note: despite the similar titles, this PR is not related to #83 at all. This PR is specific to the changes introduced in #91.)

When using AnyBitPattern derive, an automatic derive for Zeroable is created as well. However, the latter derive is missing generic parameters for the struct, which means you get errors like the following:

error[E0107]: missing generics for struct `AnyBitPatternTest`
   --> tests/basic.rs:107:8
    |
107 | struct AnyBitPatternTest<A: AnyBitPattern, B: AnyBitPattern> {
    |        ^^^^^^^^^^^^^^^^^ expected 2 generic arguments
    |
note: struct defined here, with 2 generic parameters: `A`, `B`
   --> tests/basic.rs:107:8
    |
107 | struct AnyBitPatternTest<A: AnyBitPattern, B: AnyBitPattern> {
    |        ^^^^^^^^^^^^^^^^^ -                 -
help: add missing generic arguments
    |
107 | struct AnyBitPatternTest<A, B><A: AnyBitPattern, B: AnyBitPattern> {
    |        ~~~~~~~~~~~~~~~~~~~~~~~

The help text is worse than useless, because the suggested fix makes things worse.

@cky cky force-pushed the allow-generic-structs-to-use-any-bit-pattern-derive branch from ffef78a to 0fd8cc8 Compare April 21, 2022 03:58
@Lokathor
Copy link
Owner

@fu5ha you get to review this one!

Copy link
Collaborator

@fu5ha fu5ha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm :)

@fu5ha
Copy link
Collaborator

fu5ha commented Apr 21, 2022

Not sure why this is failing on nightly tho

@cky
Copy link
Contributor Author

cky commented Apr 21, 2022

Wonder if it's using the old macro expansion? Certainly the error message suggests so. Didn't you run into similar CI bogosity in #91?

@cky
Copy link
Contributor Author

cky commented Apr 21, 2022

Just so I know I haven't gone nuts, here's the nightly and miri runs on my computer: https://gist.github.com/cky/f80cc8b82632bd10deaebcb1a5668172

@cky
Copy link
Contributor Author

cky commented Apr 22, 2022

Is there a way to rerun the CI? I could technically push an empty commit just to trigger one but if there's a way to actually just rerun, I'd rather not junk up the branch with empty commits. ;-)

@Lokathor
Copy link
Owner

I'll try a close and re-open.

@Lokathor Lokathor closed this Apr 22, 2022
@Lokathor Lokathor reopened this Apr 22, 2022
@Lokathor
Copy link
Owner

@cky sorry I haven't looked at this one in ages. It seems by now there's a small conflict in a test file. Do you want to fix that up and then I think we can merge this?

@Lokathor Lokathor merged commit 8391afa into Lokathor:main Mar 24, 2023
@Lokathor Lokathor added the semver-minor semver minor change label Mar 24, 2023
leod pushed a commit to leod/bytemuck that referenced this pull request Jun 3, 2023
* Allow generic structs to use `AnyBitPattern` derive.

* Attempt to nudge the CI into retrying.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor semver minor change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants