Skip to content

Normalize PList suffix slice bounds - #337

Open
xiehuanyi wants to merge 1 commit into
tobgu:masterfrom
xiehuanyi:fix-plist-suffix-slices
Open

xiehuanyi wants to merge 1 commit into
tobgu:masterfrom
xiehuanyi:fix-plist-suffix-slices

Conversation

@xiehuanyi

Copy link
Copy Markdown

plist([1, 2, 3])[-1:] currently raises IndexError, although PList supports sequence slicing. The optimized suffix-slice path passes the negative start directly to _drop, instead of counting from the end and clipping it as a built-in list does. That path also accepts floating-point starts and rejects objects that implement __index__.

Normalize suffix-slice starts while retaining the existing shared tail. Stop dropping once the list is empty, so an oversized positive start does not keep traversing the empty-list sentinel.

Validation on Python 3.12:

  • Regression cases before the fix: 19 failed, 16 passed.
  • pytest tests/list_test.py: 73 passed, including comparisons with built-in list slicing and shared-tail checks.
  • pytest: 571 passed, 102 skipped (optional C-extension cases are unavailable in this local environment).
  • git diff --check: passed.

Implementation and regression tests prepared with Codex; the patch and test results were reviewed against the existing PList code.

Copilot AI balanced review requested due to automatic review settings October 11, 2026 07:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@carlitogwapito

Copy link
Copy Markdown

One small error-path test to add alongside this: test-only commit.

It compares plist with a built-in list when __index__ itself raises. Both should preserve the same exception object, call the provider once, and leave the input unchanged. The new case passes here; the base raises a different TypeError before calling the provider. The built-in oracle and an ordinary shared-tail control pass on both versions.

Focused checks used Windows / CPython 3.14.4 with the pure-Python backend. The full suite, C-extension tests, and other interpreter/platform combinations weren't run.

Optional suggestion: keep this beside the successful __index__ and wrong-type cases, since a provider's own failure is a separate path from an invalid index value.

@xiehuanyi

Copy link
Copy Markdown
Author

Thanks for the focused __index__ error-path test. Checking exception identity, a single invocation, and unchanged input is useful additional coverage. I have reviewed the proposed test, but have not applied the optional commit or run it locally.

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.

3 participants