Repository navigation
Match obsolete comparison diagnostics - #9074
youknowdot wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
| if let Some(error) = parse_error | ||
| && let Some(range) = self.invalid_legacy_operator(error) | ||
| { | ||
| if error.location.start() > range.start() { |
There was a problem hiding this comment.
is this the right place instead of patching parser?
There was a problem hiding this comment.
The parser is the cleaner place for this diagnostic. I used the existing BarrySource adapter to keep the extraction independent of a parser release, but the review uncovered a real weakness: for 2 <<> 3, the textual match overlaps the << token and incorrectly emits the obsolete-comparison hint. I confirmed that with compile-only CPython 3.14.8/native checks. A comparison-context check in the pinned parser is the appropriate fix; the compiler override must also stop replacing that parser diagnostic. I am checking the narrow dependency path rather than importing the broader parser changes. This draft is not ready in its current form.
There was a problem hiding this comment.
The parser is still the cleaner owner of this grammar-context diagnostic. The current head, dd0c42f, fixes the concrete 2 <<> 3 overlap by matching complete adjacent tokens instead of searching the source text, and its exact-head CI passes. That does not resolve your architecture question.
The implementation still lives in BarrySource::diagnostic: it combines the token range with the parser's first error location, and the compiler/symbol-table paths call it before first_parse_error. The registry Ruff parser remains at 0.16.10. Moving the diagnostic into the comparison production would therefore also require removing or narrowing this compiler override, so the adapter does not replace the parser's result.
I've corrected the stale PR description and kept this boundary explicit. The PR remains a draft pending that design decision; I haven't made an unrelated parser rewrite. This clarification is AI-assisted with OpenAI Codex (model version unavailable), based on the current source and hosted results.
5b0fac6 to
7b25dc1
Compare
youknowdot
changed the title
7b25dc1 to
1c36965
Compare
1c36965 to
0596dee
Compare
0596dee to
aa02e48
Compare
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_mmap.py (TODO: 19) dependencies: dependent tests: (2 tests)
Legend:
|
Use the existing Barry adapter token spans for ordinary obsolete-comparison diagnostics as well. Preserve parser errors for left-shift suffixes, malformed comparisons, comments and strings, with final Python 3.15 message and span regressions. Assisted-by: OpenAI Codex:model identifier unavailable
aa02e48 to
dd0c42f
Compare
Current status
Current head:
dd0c42ff332013dff6584e896e70638176008307, targetingmain. The current source uses Python 3.15.0, not the older 3.14.8 pin described by earlier revisions. Exact-head hosted checks are complete: 27 succeeded and one conditional benchmark check was skipped; CI succeeded. The prior CI-pause and intermediate-head descriptions are superseded.This remains a draft because the parser-versus-compiler architecture question is still open. A passing CI run and the token-overlap correction do not settle that design decision.
Scope
2 <> 3, with the complete operator span.<>a generic syntax error and preserve Barry-mode behavior.<and>tokens rather than a textual substring, so2 <<> 3does not incorrectly match the suffix of<<.extra_tests/snippets/builtin_compile.py, including overlap controls. Existing Rust tests retain structural parsing checks.Only
crates/compiler/src/lib.rsand that existing snippet change. NoLib/files or parser dependency are changed. The source still uses registryrustpython-ruff_python_parser0.16.10.Remaining architecture question
The current implementation lives in the existing compiler
BarrySourceadapter. It inspects parser tokens and the first parse-error position, then returns its diagnostic beforefirst_parse_errorin the compiler and symbol-table paths. This fixes the documented textual-token overlap, but still infers comparison context outside the parser.A parser-owned diagnostic would let the grammar's comparison production determine context directly. Such a migration would also need to remove or narrow the compiler override so it does not replace the parser's diagnostic. No parser migration is claimed by this patch; maintainer review of that boundary remains necessary.
Validation and history
Exact-head hosted CI is the current validation evidence. This status refresh adds no code and claims no new local runtime or build execution.
The earlier 3.14.8 backport/source-extraction revisions had focused ASCII diagnostic, compiler and Barry-mode checks, normal hooks and scoped Clippy. Those results belong to their historical revisions, not a fresh run of this head. The change originated from the CPython obsolete-operator diagnostic update 66f8dbe0.
AI assistance
Maintainer-requested Python compatibility maintenance. OpenAI Codex assisted with source analysis, code and validation work in the existing commits, and this status/review clarification. Model version unavailable. No additional human manual testing is claimed by this update.