Relax iterator Debug bounds on ExactlyOneError - #1132
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1132 +/- ##
==========================================
- Coverage 94.38% 93.77% -0.62%
==========================================
Files 48 52 +4
Lines 6665 6491 -174
==========================================
- Hits 6291 6087 -204
- Misses 374 404 +30 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| #[test] | ||
| fn exactly_one_without_debug_iterator() { | ||
| fn expect_one(iter: impl Iterator<Item = i32>) -> i32 { |
There was a problem hiding this comment.
Please remove expect_one and just evaluate the result at the call site.
There was a problem hiding this comment.
Removed expect_one in 1e61443. The assertion is now at the call site, using &mut dyn Iterator<Item = i32> so the test still catches the unwanted Debug bound.
| #[test] | ||
| fn at_most_one_without_debug_iterator() { | ||
| fn expect_at_most_one(iter: impl Iterator<Item = i32>) -> Option<i32> { | ||
| iter.at_most_one().expect("at most one item") |
There was a problem hiding this comment.
Please make expect_at_most_one accept the two arguments and pull the assert_eq into it.
There was a problem hiding this comment.
Moved assert_eq! into the two-argument expect_at_most_one helper in 1e61443.
| } | ||
|
|
||
| #[test] | ||
| fn exactly_one_error_debug_preserves_items() { |
There was a problem hiding this comment.
I am a bit surprised to see this, because Debug::fmt takes &self, which inhibits calling next. Or am I missing something?
There was a problem hiding this comment.
fmt only borrows &self; the call counter was unnecessary here. Removed it in 1e61443 while retaining the buffered-field formatting and item-recovery assertions.
| Ok(iter.exactly_one()?) | ||
| } | ||
|
|
||
| fn expect_at_most_one( |
There was a problem hiding this comment.
Do we really need the helper functions expect_at_most_one and expect_one?
There was a problem hiding this comment.
Removed both helpers in 1e61443. The test now returns Result and exercises ? directly on Box<dyn Iterator<Item = i32>>; the failure cases still convert to Box<dyn Error>. The 127 tests in these two modules pass, as do the 32 core tests separately with no default features and with use_alloc. Restoring either old trait bound makes the corresponding regression checks fail.
|
Sorry for my back and forth, but please remove the newly introduced tests. Server-Checks should detect if we mistakenly re-introduce the Debug bound |
|
Removed the four added tests in b723eec. Both test files now match upstream; the production change is unchanged. The remaining two test modules pass 123 tests, and the 30 core tests pass separately without default features and with |
|
Thanks for this |
1681e07
2 participants
ExactlyOneErrorcurrently requires the input iterator to implementDebug, soexactly_one().expect(...),at_most_one().expect(...), and?intoBox<dyn Error>fail for an opaqueimpl Iterator<Item = i32>.Remove that unnecessary bound from
DebugandError. Keep the buffered first/second item diagnostics and mark the omitted inner iterator with...Item: Debugis still required. Existing debug output therefore loses the inner iterator; iteration andDisplayare unchanged.Fixes #1052.
At the maintainer's request, the four newly introduced tests were removed; both test files now match upstream. The production fix is unchanged. The removed regressions had previously demonstrated the original trait-bound failures on upstream and passed with the fix.
Validated after test removal on Rust 1.97.1, macOS ARM64:
test_coreandtest_std.use_alloc.The earlier broader validation and baseline comparison remain historical evidence. Strict Clippy still has the previously identified unchanged
question_markwarnings, and hosted semver/project-coverage failures remain unresolved. After the requested test removal, hosted patch coverage also reports one uncovered changed line (0% patch coverage); this check is failing. No warnings or checks were suppressed. MSRV, Miri and the full platform matrix were not rerun locally for this test-only follow-up.AI assistance: OpenAI Codex assisted with the implementation, tests and description; a separate agent reviewed the patch and verification evidence.