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: use anon-const for wrapping expanded impls #427

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

shekhirin
Copy link
Contributor

@shekhirin shekhirin commented Feb 26, 2024

On latest nightly, with the introduction of rust-lang/rust#120363, proptest derives don't pass lints anymore, e.g.

error: non-local `impl` definition, they should be avoided as they go against expectation
  --> crates/primitives/src/account.rs:15:12
   |
15 | pub struct Account {
   |            ^^^^^^^
   |
   = help: move this `impl` block outside the of the current constant `_IMPL_ARBITRARY_FOR_Account`
   = note: an `impl` definition is non-local if it is nested inside an item and neither the type nor the trait are at the same nesting level as the `impl` block
   = note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
   = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
   = note: the derive macro `AccountPropTestArbitrary` may come from an old version of the `proptest_derive` crate, try updating your dependency with `cargo update -p proptest_derive`
   = note: `-D non-local-definitions` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(non_local_definitions)]`
   = note: this error originates in the derive macro `AccountPropTestArbitrary` (in Nightly builds, run with -Z macro-backtrace for more info)

This fix is similar to dtolnay/typetag@d51f4ad, i.e. use anon const expressions instead of named ones.

@shekhirin shekhirin marked this pull request as ready for review February 26, 2024 11:48
@shekhirin
Copy link
Contributor Author

shekhirin commented Feb 26, 2024

@matthew-russo PTAL when you have time, as it blocks nightly CIs on projects like Reth (https://github.com/paradigmxyz/reth/actions/runs/8048254493/job/21979197918)

@matthew-russo
Copy link
Member

@shekhirin

looks like nightly has failing unit tests
https://github.com/proptest-rs/proptest/actions/runs/8048322898/job/22017111787?pr=427#step:13:770

i'll try to give a look one evening this week. if you get to it first, let me know

@shekhirin
Copy link
Contributor Author

@matthew-russo ah, it's an easy one, fixed! Can you please re-run the CI?

@matthew-russo matthew-russo merged commit b1be99d into proptest-rs:master Mar 4, 2024
5 checks passed
@matthew-russo
Copy link
Member

Thanks, I'll try to cut a release this week as we have a couple fixes pending

@DaniPopes
Copy link
Contributor

Hey @matthew-russo, any updates on this? I saw proptest was released but not proptest-derive, was this missed? Thanks!

@matthew-russo
Copy link
Member

Sorry I had to take some time away to deal with projects at home. I'll be triaging issues this week and throw cutting a derive release in there as well

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.

3 participants