Skip to content

Retain buffer exports for regex scanners - #9455

Merged
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:re_scanner_buffer
Oct 29, 2026
Merged

Retain buffer exports for regex scanners#9455
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:re_scanner_buffer

Conversation

@1ndahous3

@1ndahous3 1ndahous3 commented Oct 29, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Keep the original buffer export alive for the lifetime of a regex scanner or finditer iterator. Resizing the underlying bytearray or releasing its memoryview now raises BufferError while the scanner is active.
  • Reuse that export for subsequent matches and expose the scanner's owned references to cyclic GC.

Performance

Scanning a bytearray containing 500 matches, measured against main at 8c6c94b07:

Platform main This patch Speedup
Windows 11 x64 252 µs 199 µs 1.26×
Debian x64 235 µs 190 µs 1.24×

AI assistance

Written with Codex (GPT-6), reviewed by a human before submission.

Summary by CodeRabbit

  • Bug Fixes
    • Improved regular-expression scanner handling for byte patterns, including consistent use of buffered input during matching and searching.
    • Scanner creation now validates that subjects for non-byte patterns are strings, so invalid subjects are rejected earlier.
    • finditer now uses the same scanner creation path as other scanner operations.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T06:39:11.377336Z b8a33ab PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between b85582d and b8a33ab.

Files ignored due to path filters: 1
  • Lib/test/test_re.py is excluded by !Lib/**
Files selected for processing: 1
  • crates/vm/src/stdlib/_sre.rs

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

Walkthrough

Pattern.scanner now creates byte-pattern buffers and validates other subjects as strings. Scanner match and search pass the stored buffer through with_sre_str!. finditer now obtains its scanner through Pattern.scanner.

Changes: SRE scanner

Layer / File(s) Summary
Buffer-aware scanner setup and operations
crates/vm/src/stdlib/_sre.rs		 with_sre_str! accepts an optional buffer and uses contiguous_or_collect when a buffer is present. Pattern.scanner creates a buffer for byte patterns and validates other subjects as strings. SreScanner stores the optional buffer, which its match and search methods pass to the macro. finditer now calls Pattern.scanner.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: Minimal · up to b8a33

No actionable merge-blocking issue is established for retaining scanner buffer exports. The change is mergeable subject to normal build and test checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 "Retain buffer exports for regex scanners" matches the main objective of PR #9455, which keeps the original buffer export alive for the lifetime of a regex scanner or finditer iterator. The …
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.
Full details: Title check

Explanation

The title "Retain buffer exports for regex scanners" matches the main objective of PR #9455, which keeps the original buffer export alive for the lifetime of a regex scanner or finditer iterator. The raw summary confirms the changes in crates/vm/src/stdlib/_sre.rs: the scanner stores a buffer, scanner match and search pass that buffer through with_sre_str!, and finditer obtains its scanner through Pattern.scanner. The title is specific and concise. It omits the validation change for non-bytes subjects, which is a secondary detail.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions

github-actions Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/re
[ ] test: cpython/Lib/test/test_re.py (TODO: 3)
[x] test: cpython/Lib/test/re_tests.py

dependencies:

  • re

dependent tests: (86 tests)

  • re: test_android test_ast test_asyncio test_binascii test_builtin test_bytes test_clinic test_cmd_line test_ctypes test_dict test_dis test_docxmlrpc test_dtrace test_email test_embed test_faulthandler test_filecmp test_fileinput test_fnmatch test_format test_frame test_free_threading test_fstring test_functools test_future_stmt test_genericalias test_glob test_hashlib test_http_cookiejar test_httplib test_httpservers test_imaplib test_importlib test_inspect test_ipaddress test_launcher test_logging test_mailbox test_mmap test_optparse test_ordered_dict test_pdb test_pprint test_profiling test_pydoc test_pyexpat test_pyrepl test_re test_regrtest test_remote_pdb test_runpy test_set test_site test_smtplib test_socket test_ssl test_strftime test_string test_strptime test_strtod test_structseq test_symtable test_syntax test_sys test_sysconfig test_tarfile test_tempfile test_tkinter test_tokenize test_tools test_traceback test_turtle test_types test_typing test_unittest test_unparse test_venv test_webbrowser test_winapi test_with test_wsgiref test_xmlrpc test_zipfile test_zipimport test_zoneinfo test_zstd

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

youknowone
youknowone approved these changes Oct 29, 2026
View reviewed changes

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

👍

@youknowone
youknowone merged commit eccb9ad into RustPython:main Oct 29, 2026
21 checks passed
@1ndahous3
1ndahous3 deleted the re_scanner_buffer branch October 29, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@1ndahous3 @youknowone