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

fix: add generator for arbitrary sized arrays #131

Merged
merged 2 commits into from
Mar 30, 2023
Merged

Conversation

camshaft
Copy link
Owner

@camshaft camshaft commented Mar 30, 2023

There were a few recent changes that broke the build in s2n-quic:

  • In Fix #[derive(TypeGenerator)] #126, the generic bounds in derives were changed and broke implementations using arbitrary sized arrays:

    #[derive(TypeGenerator)]
    struct Thing<T, const LEN: usize> {
      data: [T; LEN],
    }

    This is because before we generated a where clause for the whole field type:

    where [T; LEN]: TypeGenerator

    and now we generate it just for the generic type:

    where T: TypeGenerator

    To work around this, I've implemented TypeGenerator for all array lengths, which is a good idea to do anyway.

  • In do not require importing bolero_generator or TypeGenerator in order to derive #109, it was set up to prioritize importing TypeGenerator from the bolero crate, if possible. This causes issues for crates like s2n-quic-core where we export TypeGenerator impls with a feature flag and only use bolero as a dev-dependency. I've swapped the priorities and this is resolved.

To try and prevent this kind of breakage in the future, I've added a workflow to test s2n-quic against any bolero changes. Note that the versions do need to match (they currently don't since the s2n-quic update is blocked on these issues) but I was able to test locally and make sure these are fixed. Moving forward, we should still be able to catch issues before bumping bolero versions.

@camshaft camshaft marked this pull request as ready for review March 30, 2023 20:06
@camshaft camshaft requested a review from adpaco-aws March 30, 2023 20:06
Copy link
Collaborator

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Oops! Sorry for breaking the build in s2n-quic 😥

Just a few comments/questions.

.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
bolero-generator/src/array.rs Outdated Show resolved Hide resolved
bolero-generator/src/array.rs Show resolved Hide resolved
@camshaft camshaft requested a review from adpaco-aws March 30, 2023 21:51
Copy link
Collaborator

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Thanks, @camshaft !

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 this pull request may close these issues.

2 participants