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

Feature std compatibility #13

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

cehteh
Copy link
Contributor

@cehteh cehteh commented Jan 23, 2025

Note this is work in progress and contains breaking changes.

This is a very early draft. method names and semantic may change.
* introduces a helper union to fix simplify alignment calculations
  no need for cmp and phantom data anymore
* add simple testcase that triggered the miri issue
* change end_ptr_atomic_mut(), using the rewritten start_ptr()
Since we always point to a valid allocation we can use NonNull here.
This will benefit from the niche optimization:
 size_of::<HeaderVec<H,T>>() == size_of::<Option<HeaderVec<H,T>>>

Also adds a test to verfiy that HeaderVec are always lean and niche optimized.
These methods have the same API/Semantic than the std::Vec methods.
They are useful when one wants to extend a HeaderVec in place in some complex way
like for example appending chars to a u8 headervec with `encode_utf8()`
Having a HeaderVec being zero length is mostly useless because it has to reallocated instanly
when anything becomes pushed. This clearly should be avoided!

Nevertheless supporting zero length takes out a corner-case and a potential panic and removes
the burden for users explicitly ensuring zero length HeaderVecs don't happen in
practice. Generally improving software reliability.
For backwards compatibility this is not (yet) enabled by default.
Will be used for all API's that require functionality provided by the rust stdlib.
This makes it comply with the std Vec's API
This is a safety measure, normally resize_cold() wouldn't been called when here is nothing to
do. But we want to ensure that if this ever happens we don't run into a panicking corner case.
This introduce a breaking change starting to modifying all methods that
indicate a realloc by returning a Option(*const()) into stdlib compatible
signatures and variants taking a closure for the fixup.

This commits starts with reserve and shrink methods, more in the following commits.

The `WeakFixupFn` is required for Drain and other iterators which do significant work
in the destructor (possibly reallocating the headervec there). The closure
can be passed along and called when required.

Compatibility note:
When it is not trivially possible to refactor fixup functionality to a closure then
old code can be migrated by:

  let maybe_realloc: Option<*const ()> = {
      let mut opt_ptr = None;
      hv.reserve_with_weakfix(&mut self, 10000, |ptr| opt_ptr = Some(ptr));
      opt_ptr
  };

  // now maybe_realloc is like the old Option<*const ()> return

  if let Some(ptr) = maybe_realloc {
        // fixup code using ptr
  }

Note 2:
do we want legacy wrapper functions that have the old behavior eg.

  #[deprecated("upgrade to the new API")]
  pub fn reserve_legacy(&mut self, additional: usize) -> Option<*const ()> {
      let mut opt_ptr = None;
      self.reserve_with_weakfix(additional, |ptr| opt_ptr = Some(ptr));
      opt_ptr
  };
@cehteh cehteh force-pushed the feature-std-compatibility branch from 57058bf to 9775e59 Compare January 23, 2025 16:48
These are mostly the std Vec compatible From impls. By default this creates `HeaderVec<(), T>`
Additionally when one wants to pass a header one can do that by a `WithHeader(H,T)` tuple
struct. The later is mostly for convenience.

I took my liberty to introduce my xmacro crate here. The macro expands to ~120 lines of code.
When this dependency is not wanted it could be replaced with the expanded code.
This simplifies the `From` impls and should generally be more useful.
We could enabled these in non-std envorinments since HeaderVec depends on alloc, but i delay
the decision about this for now.
This removes the xmacro in favor of a generic From implementation for
`H: Default` and data constructed from `AsRef<[T]>`
@cehteh cehteh force-pushed the feature-std-compatibility branch from a5cef85 to 09d6827 Compare January 24, 2025 13:06
Code is taken from the stdlib Vec and adapted to HeaderVec
Software that uses this crate in a no_std environment will now have to clear the `std` flag
manually.

Rationale:

Creating documentation and testing things that require `std` would require a lot conditional
compilation tricks. Things would easily go under the Radar when buildiong doc and running
tests with `no_std` being the default.

Most users likely expect the `std` feature to be enabled by default.

Still if it this is a problem we can keep `no_std` being the default with some effort.
This add the drain functionality similar to std Vec's drain to HeaderVec.
The `with_weakfix()` things are not needed for Drain (I was wrong in a earlier commit message)
but they will be required for upcoming Splice functionality.

Since vec::Drain depends on a few nightly features internally but we want to stay compatible
with stable a few things are backported from nightly in `future_slice`. OTOH we can already
stabilize Drain::keep_rest().

Most code was taken from std::vec and minimally adapted to work for HeaderVec.
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.

1 participant