rollup + hopefully final prep before 1.0 release #123
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It turns out that 'v1' exists but isn't really pully supported. Which makes sense. It's easier to just push stuff to master. So we switch to that too. Closes #122 Ref: #122 (comment)
We make an absolute mess of our tests so that 'cargo miri test' will complete in reasonable time. I hate this, but Miri is worth it. Ref #121
When using 'get_unchecked' twice where one is a mutable borrow, it ends up creating UB under the "stacked borrows" model. Which isn't adopted yet. Still, it seems likely that it will? So we fix it by deriving both pointers to 'ptr::copy' from the same 'get_unchecked_mut' call. Closes #121
Previously, we were using 'end_ptr' by computing a zero-length pointer by offseting from the end of the haystack. But for pointer provenance reasons, it is more correct to computer the end pointer by adding to the start pointer. I don't believe this is necessary for the forward case, but do it there too "just in case" and also to make the code more similar to the reverse case. Closes #121
Miri warns about this, so we fix it. I have no particular attachment to integer-to-pointer casts, and it seems like the tide might be shifting against them. Since we don't need an integer-to-pointer cast here, we drop it. I believe it was written the way it was (from the 'memchr crate, which itself should be fixed) because it made more intuitive sense to me. Alas...
This slightly tweaks the documentation of the various OsStr/Path conversion routines to cover the possible future where `OsStr::as_bytes` (and probably also `OsStr::from_bytes`) exists. Namely, the point of these routines was always to get at the underlying byte representation of OS strings, but doing so without building a WTF-8 decoder/encoder. So if the raw bytes do wind up getting exposed in a platform independent way, then we'll switch all of these routines to use the raw bytes and they'll all become, effectively, no-ops. But for now, we still continue to choose to sacrifice a little correctness on Windows in exchange for a lot of simplicity. Closes #115
And switch to Rust 2011 and 'resolver = 2'. The idea here is to overhaul bstr features to make use of the new resolver in Cargo. But this requires Rust 1.60. This does make the MSRV a bit newer than I would hope, but it's at least a few releases old at this point. And if I don't do this now, then I either have to accept a sub-optimal feature setup for 1.0 forever, or delay 1.0 until Rust 1.60 is "old enough." Instead, we can just bump to Rust 1.60 now. For people too squeamish to move to such a new Rust, they can stick to bstr 0.2.0 until they're ready to move on. Ref: #40 (comment)
This gets rid of the 'serde1-{std,alloc,core}' features and just
replaces it with a single proper 'serde' feature. We make it work as one
would expect with the other crate features by using the new 'dep:' and
'pkg?' Cargo.toml feature syntax.
This is a breaking change because the 'serde1' features no longer exist.
Instead, you just need to enable the 'serde' feature and it should
automatically do the right thing depending on whether you also have
'std', 'alloc' or neither set.
Ref: https://doc.rust-lang.org/cargo/reference/features.html
Ref: #40 (comment)
Closed
epage
approved these changes
Sep 2, 2022
There was a problem hiding this comment.
Another benefit I've found to dep: syntax is that it will provide the user a cleaner, more relevant feature list when they cargo add bstr. As I've been using cargo add more, its been increasing my motivation to go and update the MSRV of every crate of mine to use dep: (hard to overcome inertia for the low-edit crates).
|
@epage Yeah that is a nice win, I agree. It is much much better to not conflate optional dependencies and features, especially given that they treated as API surface. Definitely a big win! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first lines of each commit message should give a good breakdown of what's in here. Notably, this moves the MSRV to Rust 1.60 and makes use of the new
dep:andpkg?feature syntax to get rid of theserde1-{std,alloc,core}hack.cc @epage