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

Add SplitAt, derive(SplitAt) #2433

Merged
merged 1 commit into from
Mar 24, 2025
Merged

Add SplitAt, derive(SplitAt) #2433

merged 1 commit into from
Mar 24, 2025

Conversation

jswrenn
Copy link
Collaborator

@jswrenn jswrenn commented Mar 10, 2025

Makes progress towards #1290.

@jswrenn jswrenn force-pushed the generalized-splits branch from c83d465 to b194be4 Compare March 13, 2025 23:26
@jswrenn jswrenn force-pushed the generalized-splits branch 2 times, most recently from 7955ba1 to a7a8bba Compare March 17, 2025 18:55
@jswrenn jswrenn force-pushed the generalized-splits branch from a7a8bba to 66c7ab9 Compare March 17, 2025 19:07
@jswrenn jswrenn force-pushed the generalized-splits branch 8 times, most recently from 87d6676 to 2d7dde3 Compare March 19, 2025 19:46
@jswrenn jswrenn force-pushed the generalized-splits branch 5 times, most recently from ddb594e to b62dd2c Compare March 19, 2025 20:04
@jswrenn jswrenn force-pushed the generalized-splits branch 3 times, most recently from 2173b58 to 5ea000e Compare March 19, 2025 20:13
@jswrenn jswrenn force-pushed the generalized-splits branch from 5ea000e to 6a81166 Compare March 19, 2025 20:15
@jswrenn jswrenn force-pushed the generalized-splits branch 3 times, most recently from 3571b22 to 4aecf26 Compare March 24, 2025 16:13
@jswrenn jswrenn marked this pull request as ready for review March 24, 2025 16:16
@joshlf
Copy link
Member

joshlf commented Mar 24, 2025

@kupiakos I know you had an interest in this regarding #1289. Let us know if this API (specifically, the SplitAt trait and its methods) looks like it'd support your use case? (Though I'm not sure if you actually had a specific use case or were just generally interested in the problem)

@jswrenn jswrenn force-pushed the generalized-splits branch from 4aecf26 to 788d4cc Compare March 24, 2025 16:26
@joshlf joshlf changed the title Generalize split_at_unchecked Add SplitAt, derive(SplitAt) Mar 24, 2025
@jswrenn jswrenn force-pushed the generalized-splits branch from 788d4cc to 7f6dfa9 Compare March 24, 2025 18:37
@jswrenn jswrenn enabled auto-merge March 24, 2025 18:39
@jswrenn jswrenn force-pushed the generalized-splits branch 2 times, most recently from 2f5bc52 to 62b071b Compare March 24, 2025 19:04
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 96.56566% with 17 lines in your changes missing coverage. Please review.

Project coverage is 90.69%. Comparing base (079fe9c) to head (52751cd).

Files with missing lines Patch % Lines
src/lib.rs 93.75% 11 Missing ⚠️
src/util/mod.rs 96.11% 4 Missing ⚠️
src/pointer/inner.rs 99.04% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2433      +/-   ##
==========================================
+ Coverage   90.38%   90.69%   +0.31%     
==========================================
  Files          18       18              
  Lines        7287     7714     +427     
==========================================
+ Hits         6586     6996     +410     
- Misses        701      718      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This derivable trait provides generalized splitting for slice DSTs.
Splitting a DST `T` produces a pair `(T, [T::Elem])`, where `T::Elem`
is the element type of `T`'s trailing slice. We expose this
functionality as a derivable trait, so as to not violate the privacy
of the trailing slice.

Makes progress towards #1290.
@jswrenn jswrenn force-pushed the generalized-splits branch from 62b071b to 52751cd Compare March 24, 2025 19:11
@jswrenn jswrenn added this pull request to the merge queue Mar 24, 2025
Merged via the queue into main with commit fa0b99d Mar 24, 2025
88 checks passed
@jswrenn jswrenn deleted the generalized-splits branch March 24, 2025 19:42
@kupiakos
Copy link
Contributor

kupiakos commented Mar 28, 2025

actually had a specific use case

IIRC, our original use case is also solved with FromBytes::ref_from_prefix_with_elems. How we ended up implementing in 0.7 was the usual way when DSTs don't work out: a struct with fields pointing to the header and dynamic tail. It was fine because we only needed to FromBytes, not construct an owned version.

In any case, this would also make this use case when using DSTs much less fallible.

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.

4 participants