arbitrary icon indicating copy to clipboard operation
arbitrary copied to clipboard

List of breaking changes for next breaking release

Open fitzgen opened this issue 7 months ago • 6 comments

  • [ ] https://github.com/rust-fuzz/arbitrary/pull/216
  • [ ] remove split between try_size_hint and size_hint
  • [ ] bump MSRV to latest stable
  • [ ] https://github.com/rust-fuzz/arbitrary/issues/201
  • [ ] flip size_hint's parameter from current depth to max-depth
  • [ ] add support for a..b ranges in Unstructured::int_in_range
  • [ ] not breaking, but could be nice: https://github.com/rust-fuzz/arbitrary/issues/191
  • [ ] stop returning an error from many Unstructured methods that are actually infallible now (and maybe also introduce try_* variants that return a Result and don't create dummy values when the underlying data is exhausted)

Anything else?

fitzgen avatar Apr 08 '25 22:04 fitzgen

I was just working on improving some Arbitrary implementations, and I think un-splitting try_size_hint would be very much worthwhile to reduce the number of possible mistakes.

Other suggestions:

  • #201 — add a SizeHint struct to make it much less verbose to construct size hints, since they could be combined using + and | operators (instead of methods).
  • Consider flipping the definition of depth so that instead of accepting the current depth with a hard-coded MAX_DEPTH, the parameter is itself a maximum further depth, and recursion decrements rather than increments it. That way, if an application finds itself hitting a depth limit it can just ask for more depth instead of having to modify the library.

kpreid avatar Apr 08 '25 22:04 kpreid

P.S. I'd be happy to try writing a PR for the SizeHint breaking change if there is interest.

kpreid avatar Apr 09 '25 00:04 kpreid

Consider a custom type for easier size hints #201 — add a SizeHint struct to make it much less verbose to construct size hints, since they could be combined using + and | operators (instead of methods).

Sounds good to me. I'm fine with the operator overloading sugar, but I think there should still be named methods that the operators just forward to for better rust docs discoverability.

Consider flipping the definition of depth so that instead of accepting the current depth with a hard-coded MAX_DEPTH, the parameter is itself a maximum further depth, and recursion decrements rather than increments it. That way, if an application finds itself hitting a depth limit it can just ask for more depth instead of having to modify the library.

Also sounds good to me 👍

P.S. I'd be happy to try writing a PR for the SizeHint breaking change if there is interest.

For just the types or the trait methods as well? Either way, that would be very welcome!

fitzgen avatar Apr 09 '25 15:04 fitzgen

https://github.com/rust-fuzz/arbitrary/issues/191 wouldn't be a breaking change, but is another pet issue I have but haven't gotten around to implementing. If we are doing a breaking change, that probably involves some amount of outwards communication, and having this as a cool new feature to help motivate upgrades would be nice to bundle in as well.

fitzgen avatar Apr 09 '25 16:04 fitzgen

A note I got from an arbitrary user in private chat: support regular a..b ranges (instead of only a..=b ranges) in Unstructured::int_in_range.

Note that we can't only have a..b ranges, or else you wouldn't be able to express including the int's max value in the range, but with some trait sugar we should be able to support both in the same method.

fitzgen avatar Apr 09 '25 16:04 fitzgen

PR #218 adds the SizeHint type and combines try_size_hint() into size_hint(). (It does not change the meaning of the depth parameter.)

kpreid avatar Apr 10 '25 15:04 kpreid