Drop what a deque with maxlen of zero is handed - #8567
Conversation
|
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: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughDeque 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 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: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Warning |
youknowone
left a comment
There was a problem hiding this comment.
Thank you! and welcome to RustPython project
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
10a1df3 to
e0c0f5a
Compare
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.
appendandappendleftincrates/vm/src/stdlib/_collections.rstrim before pushing, onself.maxlen == Some(deque.len()). For an empty deque withmaxlenzero that comparison is true,pop_fronton 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=0against CPython 3.14 and only these two disagreed:append,appendleftextend,extendleft,+=insertIndexError: deque already at its maximum size*,*=,+,rotate,copy, constructorWhy the suite is green
Lib/test/test_deque.pyhastest_maxlen_zero, and it exercises the constructor,extendandextendleft, which are the three paths that were already correct. It never callsappendon a deque built withmaxlen=0.Test Plan
Built in a Debian container on rustc 1.98.0.
extra_tests/snippets/stdlib_collections_deque.pygains the zero bound cases, plus the neighbouring behaviour it would be easy to break: a bounded deque still drops from the far end and only once it is full. On a build without the change the file stops atassert list(d) == []right after the first append.pytest test_snippets.py -k collections_deque, both legs green, so the file holds under CPython 3.14.7 as well.cargo run --release -- -m test test_deque: 81 tests, 3 skipped, SUCCESS.cargo clippywith the flags CI uses, clean, andcargo fmt --checkclean.ruff format --checkandruff check --select Iclean on the snippet.cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: no failures.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