fix(stats): remove duplicate big-endian TDigestStub/HllSketchStub defs#3857
Merged
Conversation
… defs PR #3850's second commit (addressing roborev review 2085 by moving `PartialEq` from a derive to an explicit `impl` on `TDigestSlot`/ `HllSlot`) accidentally left the *old* stub struct + impl alongside the new one. On big-endian targets both `TDigestStub` and `HllSketchStub` were therefore defined twice — once with `#[derive(Default, Clone, PartialEq)]` and once with `#[derive(Default, Clone)]` — producing E0428, E0119, E0592, and E0034 errors and breaking the s390x build (run 25929817626). Delete the redundant `#[derive(Default, Clone, PartialEq)]` copies and keep the leaner `#[derive(Default, Clone)]` versions, matching the post-refactor intent of #3850: `PartialEq` lives on the wrapping `TDigestSlot`/`HllSlot` slot types (manual `impl PartialEq` with constant `true`), not on the inner stubs. Verified by a standalone `rustc --target s390x-unknown-linux-gnu --emit metadata` round-trip of the cfg-gated block (same shape-only verification approach #3850's commit message describes) and by running `cargo test --test tests stats -F all_features` (749 passed) on little-endian. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
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.
Summary
.0#3850's second commit (addressing roborev review 2085 by movingPartialEqfrom a derive to an explicitimplonTDigestSlot/HllSlot) accidentally left the old stub struct + impl block alongside the new one. On big-endian targets bothTDigestStubandHllSketchStubended up defined twice — once with#[derive(Default, Clone, PartialEq)]and once with#[derive(Default, Clone)]— producing E0428 (name … is defined multiple times), E0119 (conflicting impls of Default/Clone), E0592 (duplicate definitions), and E0034 (multiple applicable items in scopeonupdate/quantile/is_empty/estimate) and breaking the s390x build.workflow_dispatch-triggered only, so the regression slipped past normal CI — see run 25929817626.#[derive(Default, Clone, PartialEq)]copies and keep the leaner#[derive(Default, Clone)]versions. This matches the post-refactor intent of fix(stats): restore big-endian build by giving slot fallbacks an accessible.0#3850 —PartialEqlives on the wrappingTDigestSlot/HllSlotslot types (manualimpl PartialEqreturning a constanttrue), not on the inner stubs.Test plan
rustc --target s390x-unknown-linux-gnu --emit metadataround-trip of the cfg-gated block (same shape-only verification approach used in fix(stats): restore big-endian build by giving slot fallbacks an accessible.0#3850's commit message). All four symbols —TDigestStub,TDigestSlot,HllSketchStub,HllSlot— appear exactly once in the resulting metadata.cargo test --test tests stats -F all_features(little-endian) — 749 passed, 0 failed.cargo clippy -F all_features— clean.s390xworkflow on this branch to confirm the build now passes on the native big-endian runner.🤖 Generated with Claude Code