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

Version 0.8.0 checklist #58

Closed
2 of 10 tasks
Centril opened this issue Jun 12, 2018 · 10 comments
Closed
2 of 10 tasks

Version 0.8.0 checklist #58

Centril opened this issue Jun 12, 2018 · 10 comments

Comments

@Centril
Copy link
Collaborator

Centril commented Jun 12, 2018

Some proposed TODOs for version 0.8.0:

  • Raise minimum rustc version to 1.27; lands on 2018-06-21 (9 days).
    Benefits:
  • Implement Strategy for RangeInclusive<T> where T is a float (which requires .start() and .end()).
    • Can be done via #[cfg(MIN_1_127)] logic if we want to make v0.8.0 available for 1.26.0.
  • Arbitrary for SIMD stuff;
    • Can be done via #[cfg(MIN_1_127)] logic if we want to make v0.8.0 available for 1.26.0.
  • Land proptest_derive
    • Basic draft design is done
    • Need to update to latest quote, proc-macro2, syn crates.
    • Filtering of strategies
    • prob attribute on enum variants
    • mdbook documentation (serde style, https://serde.rs/)
    • Issues around recursive, esp. mutually recursive types remaining.
  • Land some form of "showable & shrinkable closures" a la [Test.QuickCheck.Function](https://hackage.haskell.org/package/QuickCheck-2.11.3/docs/Test-QuickCheck-Function.html)
    • Very much vaporware right now;
      • Some designs went nowhere some time ago...
      • I'll try some new designs in the coming days.
@AltSysrq
Copy link
Collaborator

rustc version 1.27 … 9 days

Not too bad of a delay I guess, and the things it adds seem worth it.

proptest_derive

I'd personally vote for having a minimum viable release first (eg, without filtering/prob/recursive types) and then build up from there incrementally rather than a "big bang" release since it doesn't seem like there's a strong need for them to come all at once.

showable & shrinkable closures

I don't feel like this is worth blocking 0.8.0 on; it's not like adding it will be a huge breaking change.

@Centril
Copy link
Collaborator Author

Centril commented Jun 12, 2018

I'd personally vote for having a minimum viable release first

I think that works; I think (but I'm not 100% sure) that prob and filter will be strictly additive (no breakage) and recursive logic will most likely need to be opt in and will also probably require some annotations from the user because of undecidability in dealing with mutually recursive types in general.

I don't feel like this is worth blocking 0.8.0 on; it's not like adding it will be a huge breaking change.

Agreed -- I'm quite sure that it is a purely additive change;
Hopefully, I'll be fast enough to land this before 1.27 lands anyways ;)

@AltSysrq
Copy link
Collaborator

Since 1.27 only stabilises the arch::{x86,x86_64} modules and not simd, is it really worth adding support for that now? The types there are fairly low-level and weakly-typed, which makes precludes any one-size-fits-all support.

@Centril
Copy link
Collaborator Author

Centril commented Jun 22, 2018

Not sure; it should still be possible to implement __m256 and such through _mm256_set_ps. Should be fairly easy to get good coverage with macros?

My primary motivation for 1.27 is RangeInclusive tho and to make API consistent on that front.

There are some FIXMEs and TODOs that are unrelated to 1.27 that I'd like you to look at... primarily in https://github.com/AltSysrq/proptest/blob/0.8.0-changes/src/bits.rs#L30-L31 and https://github.com/AltSysrq/proptest/blob/0.8.0-changes/src/sample.rs#L55-L56 since I am less familiar with those parts..

(Probably won't have time to finish proptest_derive so that it is polished for a release unfortunately if you want 0.8 shipped in the coming week... but I don't think proptest_derive influences decisions about proptest 0.8 -- other than to have a nicer release notes)

(Haven't had time to work enough on Showable and Shrinkable Closures -- so it should for sure not block 0.8)

@AltSysrq
Copy link
Collaborator

it should still be possible to implement __m256 and such

Possible, yes, but the types aren't well-defined enough for a one-size-fits-all solution.

some FIXMEs and TODOs that are unrelated to 1.27 that I'd like you to look at

Addressed those and all others that looked immediately fixable. Let me know if I missed something; if not, I'll get 0.8.0 published fairly soon.

@Centril
Copy link
Collaborator Author

Centril commented Jun 24, 2018

Possible, yes, but the types aren't well-defined enough for a one-size-fits-all solution.

Alright; Given that we bumped to 1.27, it can be easily added without breakage if we find a good way in the future and someone needs implementations of those :)

Addressed those and all others that looked immediately fixable. Let me know if I missed something; if not, I'll get 0.8.0 published fairly soon.

Nice work! ❤️

I have one concern re. the movement towards impl Trait in argument position... While it is nice, it also means that you are not permitted to use turbofish at all (not even to specify those type variables which are explicitly quantified that remain). The instances you changed don't seem likely to ever need turbofish, but you never know. I primarily want to highlight the trade-off. I don't mind it being changed to arg: impl Trait :)

Let me know if I missed something;

https://blog.rust-lang.org/2018/06/21/Rust-1.27.html

In Rust 1.27 you can use dyn Trait, which we should do instead of the bare trait syntax where trait objects are currently involved. That's just a matter of style tho and can be fixed in a patch version, so it is not urgent.

There's also #[must_use] which can now be put on functions, which could help users in some cases since strategies usually do nothing unless they are used, for example. In some cases, #[must_use] should probably be placed on the Strategy types themselves. For example, Iterators in the standard library typically say:

#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]

https://doc.rust-lang.org/nightly/std/iter/struct.Map.html

@AltSysrq
Copy link
Collaborator

dyn Trait

Right, I had intended on moving to that. Thanks for pointing it out.

#[must_use]

Good suggestion, I'll see where that applies.

@AltSysrq
Copy link
Collaborator

it also means that you are not permitted to use turbofish at all

Yep, which is why I did it across the board now rather than planning on adopting it opportunistically. IMHO the way it simplifies function signatures in the docs is worth the small risk that it breaks someone's code, especially since we have a whole slew of minor breaking changes in this version anyway.

@AltSysrq
Copy link
Collaborator

0.8.0 is released

@Centril
Copy link
Collaborator Author

Centril commented Jun 24, 2018

Hurray! 🦀

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

No branches or pull requests

2 participants