| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 761696a8-8425-4c7b-8461-59f1a801b483 📥 CommitsReviewing files that changed from the base of the PR and between dd2cc4d and 10a1df3. 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 Walkthrough WalkthroughDeque capacity checks now run after insertion. Zero-capacity deques discard inserted items, while bounded deques evict from the opposite end only after exceeding capacity. Tests cover mutation, construction, rotation, concatenation, extension, and insertion behavior. ChangesDeque capacity enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to 10a1d This localized fix makes zero-length deques discard appended items as expected, with regression coverage and reported checks passing; no actionable merge-blocking risk remains beyond normal review. Suggested reviewers: shaharnaveh, youknowone 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands. Warning ⚠️ This pull request shows signs of AI-generated slop (description_diff_mismatch). It has been flagged by CodeRabbit slop detection and should be reviewed carefully. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you! and welcome to RustPython project
Sorry, something went wrong.
append and appendleft trimmed before pushing, and the test they used,
maxlen == len, holds for an empty deque whose bound is zero. The pop then
had nothing to remove and the item stayed:
>>> d = deque(maxlen=0)
>>> d.append(1)
>>> list(d)
[1]
Both now push first and trim after, which is the order CPython uses, so a
bound of zero drops what just arrived. extend, extendleft, insert, rotate,
the operators and the constructor were already right.
Assisted-by: Claude Code:claude-opus-5
| Back | FazBrowse Home | New Git URL |
Summary
deque(maxlen=0) keeps everything that is appended to it:
A container that reports a bound of zero grows without one, so the idiom of using a zero length deque as a sink holds on to every object handed to it.
append and appendleft in crates/vm/src/stdlib/_collections.rs trim before pushing, on self.maxlen == Some(deque.len()). For an empty deque with maxlen zero that comparison is true, pop_front on an empty deque does nothing, and the push goes through anyway. CPython appends and then trims while the deque is longer than its bound, so the same two lines are enough here.
The rest of the deque already handles the case. I ran every mutating entry point with maxlen=0 against CPython 3.14 and only these two disagreed:
Why the suite is green
Lib/test/test_deque.py has test_maxlen_zero, and it exercises the constructor, extend and extendleft, which are the three paths that were already correct. It never calls append on a deque built with maxlen=0.
Test Plan
Built in a Debian container on rustc 1.98.0.
The three clippy jobs and the WASM check are red for the reason in #8564, unrelated to this change.
Summary by CodeRabbit
Bug Fixes
Tests