Retire the Arrange trait in favour of inherent methods - #843
Merged
Conversation
`Arrange` existed to let two overlapping impls coexist on `Collection`: one for `(K, V)` collections arranged by key, and one for `K` collections arranged by the whole record. As inherent methods they would collide, since `K` instantiated at a pair covers the first impl's shape. The second impl was sugar for `.map(|k| (k, ()))` before arranging, and it is worth deleting for its own sake. Which of the two impls a call site selected was decided by the builder type's `Input`, three type arguments away from the choice, so the same turbofish could arrange by key or by record depending on a parameter that named neither. Three call sites in `graspan1` and `spines` were reaching the record-arranging impl while spelling `ValBatcher`/`ValBuilder`/`ValSpine`. They now say `.map(|k| (k, ()))` and keep the spine they had; six others move to `arrange_by_self`. With one impl left there is no overlap, so `arrange` and `arrange_named` become inherent methods on `Collection<(K, V)>` with unchanged signatures. Call sites are untouched: inherent resolution matches what the trait provided. Only the `use ...::Arrange;` imports go away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Arrangewas an extension trait whose only implementors wereCollectionitself, in the same crate. It existed to let two overlapping impls coexist: one for(K, V)collections arranged by key, and one forKcollections arranged by the whole record. As inherent methods those would collide, sinceKinstantiated at a pair covers the first impl's shape.The record-arranging impl was sugar for
.map(|k| (k, ()))before arranging, and it is worth deleting for its own sake. Which of the two impls a call site selected was decided by the builder type'sInput, three type arguments away from the choice, so the same turbofish could arrange by key or by record depending on a parameter that named neither.That is not hypothetical. Three call sites — one in
graspan1, two inspines— were reaching the record-arranging impl while spellingValBatcher/ValBuilder/ValSpine. They now say.map(|k| (k, ()))explicitly and keep exactly the spine they had. Six others, which spelled theKey*types, move toarrange_by_self.With one impl left there is no overlap, so
arrangeandarrange_namedbecome inherent methods onCollection<(K, V)>with unchanged signatures. Call sites are untouched, since inherent resolution matches what the trait provided; only theuse ...::Arrange;imports go away.+27 −60 across seven files. Whole workspace builds warning-free and the test suite passes.
🤖 Generated with Claude Code