Skip to content

Fix numeric warning error start offsets - #9305

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:split/numeric-warning-main-offset-20261022
Oct 22, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:split/numeric-warning-main-offset-20261022

Conversation

@youknowdot

Copy link
Copy Markdown
Contributor

Follow-up to #9296, based independently on tested main 0b0a360. Use the existing numeric literal's last-character position when a parser-collected numeric SyntaxWarning becomes SyntaxError in noninteractive compilation. Interactive compilation already uses that position.

For example, the offset for compile('0b1and 2', ..., 'exec') with SyntaxWarning configured as an error changes from 1 to 3, matching CPython 3.15.0. The same applies to AST compilation and Unicode character columns.

This changes only the collector's offset argument. It does not change warning order or callback arguments, exception conversion/stringification, original-filename handling, or interactive behavior. It is not the broader numeric metadata patch prepared on #9282: end positions, error text/newline policy, lone-CR normalization and SyntaxError args remain outside this PR. The separate #9282 work is not included or superseded.

Validation:

  • Built default+capi with locked/offline dependencies and incremental compilation disabled; source-bound independent binary.
  • Compared 18 fixed, compile-only cases with final CPython 3.15.0 (d220409a13b77de7a6c30abe16ea90db01528edc) and a historical Order parser warnings for valid source while preserving recovery boundaries #9296 binary whose compile.rs is byte-identical to the main base. That historical binary is not represented as a fresh main build.
  • Cases cover ordinary/AST exec, eval and single modes, Unicode before the numeric literal, and LF/CRLF/lone-CR sources. Start positions now agree in 16/18 cases. Two lone-CR cases retain the pre-existing normalization gap.
  • Every recorded baseline-to-candidate field was compared: only SyntaxError.offset changed in all 18 cases. Full metadata equality with CPython is not claimed; end/text/args differences remain.
  • VM parser-disabled, nonthreading/threading and test-target checks passed; strict all-target threading Clippy passed. The first Clippy attempt was stopped by the disk-reserve guard; after reclaiming disposable build outputs, the unchanged candidate passed the same guarded command.
  • Existing canonical test.test_grammar.TokenTests.test_end_of_numerical_literals passed: 1 test, no skips, expected failures or unexpected successes. This is regression coverage; it does not assert offsets.

No Lib changes, expectedFailure removal or bespoke behavior tests. The canonical numeric warning method does not currently assert this offset; the finite observation matrix checks it directly without executing generated code. Full workspace/CAPI test suites were not run.

AI assistance was used for implementation and validation.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 0b0a360 and 1ba423d.

📒 Files selected for processing: 1
  • crates/vm/src/vm/compile.rs
+-------------------------------------------------+
| We don't ship debt; we refinance it into tests. |
+-------------------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)
✨ 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.

@youknowone
youknowone marked this pull request as ready for review October 22, 2026 14:24
@youknowone
youknowone merged commit 94f92e4 into RustPython:main Oct 22, 2026
20 of 21 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

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-22T14:29:54.377893Z 1ba423d Draft marked ready
ℹ️ 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.

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