FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Drop what a deque with maxlen of zero is handed by luantaraschi · Pull Request #8567 · RustPython/RustPython · GitHub

Drop what a deque with maxlen of zero is handed - #8567

Merged
youknowone merged 1 commit into
RustPython:mainfrom
luantaraschi:fix/deque-maxlen-zero-append
Aug 22, 2026
Merged

Drop what a deque with maxlen of zero is handed#8567
youknowone merged 1 commit into
RustPython:mainfrom
luantaraschi:fix/deque-maxlen-zero-append

Conversation

luantaraschi commented Aug 21, 2026
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

deque(maxlen=0) keeps everything that is appended to it:

>>> d = deque(maxlen=0)
>>> d.append(1)
>>> d.append(2)
>>> list(d)
[1, 2]                    # CPython: []
>>> d.maxlen
0

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:

before CPython
append, appendleft keeps the item empty
extend, extendleft, += empty empty
insert IndexError: deque already at its maximum size same
*, *=, +, rotate, copy, constructor empty empty

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.

  • extra_tests/snippets/stdlib_collections_deque.py gains 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 at assert 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 clippy with the flags CI uses, clean, and cargo fmt --check clean. ruff format --check and ruff check --select I clean 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

    • Corrected bounded deque behavior when adding items at capacity.
    • Ensured zero-capacity deques remain empty across append, extension, concatenation, rotation, and construction operations.
    • Fixed item eviction and insertion handling for bounded deques.
  • Tests

    • Added coverage for zero-capacity and full-capacity deque scenarios.

coderabbitai Bot commented Aug 21, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 761696a8-8425-4c7b-8461-59f1a801b483

📥 Commits

Reviewing files that changed from the base of the PR and between dd2cc4d and 10a1df3.

📒 Files selected for processing (2)
  • crates/vm/src/stdlib/_collections.rs
  • extra_tests/snippets/stdlib_collections_deque.py

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Deque 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.

Changes

Deque capacity enforcement

Layer / File(s) Summary
Capacity enforcement logic
crates/vm/src/stdlib/_collections.rs
The deque adds centralized overflow detection. append and appendleft insert items before trimming excess items, including discarding inserts when maxlen is zero.
Capacity behavior validation
extra_tests/snippets/stdlib_collections_deque.py
Tests cover zero-capacity deque operations and verify opposite-end eviction for bounded deques after capacity is reached.

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: discarding items handed to a deque with maxlen set to zero.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @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.

youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
youknowone force-pushed the fix/deque-maxlen-zero-append branch from 10a1df3 to e0c0f5a Compare August 21, 2026 18:13
youknowone merged commit f9e65c0 into RustPython:main Aug 22, 2026
27 checks passed
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL