-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Export ruff_source_file types in rustpython_compiler_core
#6020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change refactors imports across multiple Rust project modules, replacing usage of types such as Changes
Sequence Diagram(s)No sequence diagram generated as the changes are limited to import refactoring and do not affect control flow or introduce new features. Estimated code review effort1 (~8 minutes) Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
vm/src/stdlib/ast/type_ignore.rs (1)
2-2: Consistent crate migrationThe import is updated exactly as in other modules. Good.
vm/src/stdlib/ast/argument.rs (1)
2-2: Import realignedNothing further; implementation remains unchanged.
vm/src/stdlib/ast/exception.rs (1)
2-2: Import realignedMatches the new crate structure.
vm/src/stdlib/ast/module.rs (1)
3-3: Import realignedConsistent with the rest of the refactor.
🧹 Nitpick comments (4)
compiler/codegen/src/error.rs (1)
20-20: Stale commented-out type alias
// pub type CodegenError = rustpython_parser_core::source_code::LocatedError<…>is no longer accurate and can be misleading after the crate reshuffle. Consider deleting or updating.-// pub type CodegenError = rustpython_parser_core::source_code::LocatedError<CodegenErrorType>; +// (outdated alias removed – `SourceLocation` is now provided by `rustpython_compiler_core`)compiler/core/src/marshal.rs (1)
1-1: Consider replacing the glob import forbytecode
bytecode::*brings the entire module into scope, which can mask name clashes and hurts traceability.
If only a handful of symbols are required, import them explicitly.-use crate::{OneIndexed, SourceLocation, bytecode::*}; +use crate::{ + OneIndexed, + SourceLocation, + bytecode::{CodeObject, CodeUnit, Instruction, OpArgByte, CodeFlags}, +};compiler/codegen/src/compile.rs (2)
632-635: Avoid the unnecessaryunwrap()in constant construction
OneIndexed::new(1).unwrap()can never fail, therefore the call tounwrap()only adds panic potential for future refactors. Prefer the already-available constant to make the intent explicit and remove fallible code:- column: OneIndexed::new(1).unwrap(), + column: OneIndexed::MIN,(The same pattern appears a few lines below; apply the same replacement there.)
770-773: Sameunwrap()pattern – apply the constant here as well- column: OneIndexed::MIN, + column: OneIndexed::MIN, // already fixed above, keep consistentNo functional change, just consistency with the previous suggestion.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
compiler/codegen/Cargo.toml(0 hunks)compiler/codegen/src/compile.rs(4 hunks)compiler/codegen/src/error.rs(1 hunks)compiler/codegen/src/ir.rs(4 hunks)compiler/codegen/src/symboltable.rs(1 hunks)compiler/codegen/src/unparse.rs(1 hunks)compiler/core/src/bytecode.rs(1 hunks)compiler/core/src/lib.rs(1 hunks)compiler/core/src/marshal.rs(1 hunks)vm/Cargo.toml(0 hunks)vm/src/builtins/code.rs(1 hunks)vm/src/builtins/traceback.rs(1 hunks)vm/src/frame.rs(1 hunks)vm/src/lib.rs(3 hunks)vm/src/stdlib/ast.rs(1 hunks)vm/src/stdlib/ast/argument.rs(1 hunks)vm/src/stdlib/ast/basic.rs(1 hunks)vm/src/stdlib/ast/constant.rs(1 hunks)vm/src/stdlib/ast/elif_else_clause.rs(1 hunks)vm/src/stdlib/ast/exception.rs(1 hunks)vm/src/stdlib/ast/expression.rs(1 hunks)vm/src/stdlib/ast/module.rs(1 hunks)vm/src/stdlib/ast/node.rs(1 hunks)vm/src/stdlib/ast/operator.rs(1 hunks)vm/src/stdlib/ast/other.rs(1 hunks)vm/src/stdlib/ast/parameter.rs(1 hunks)vm/src/stdlib/ast/pattern.rs(1 hunks)vm/src/stdlib/ast/statement.rs(1 hunks)vm/src/stdlib/ast/type_ignore.rs(1 hunks)vm/src/stdlib/ast/type_parameters.rs(1 hunks)vm/src/vm/vm_new.rs(1 hunks)
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
vm/src/builtins/traceback.rsvm/src/stdlib/ast/exception.rsvm/src/stdlib/ast/type_parameters.rsvm/src/stdlib/ast/parameter.rsvm/src/stdlib/ast/constant.rsvm/src/stdlib/ast/node.rsvm/src/stdlib/ast/operator.rscompiler/codegen/src/error.rsvm/src/stdlib/ast.rsvm/src/stdlib/ast/type_ignore.rsvm/src/stdlib/ast/elif_else_clause.rsvm/src/stdlib/ast/pattern.rsvm/src/builtins/code.rsvm/src/stdlib/ast/argument.rsvm/src/stdlib/ast/module.rsvm/src/stdlib/ast/basic.rsvm/src/stdlib/ast/statement.rsvm/src/stdlib/ast/other.rscompiler/codegen/src/ir.rscompiler/core/src/bytecode.rsvm/src/vm/vm_new.rscompiler/core/src/marshal.rscompiler/codegen/src/unparse.rsvm/src/stdlib/ast/expression.rscompiler/core/src/lib.rsvm/src/frame.rscompiler/codegen/src/symboltable.rsvm/src/lib.rscompiler/codegen/src/compile.rs
💤 Files with no reviewable changes (2)
- vm/Cargo.toml
- compiler/codegen/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
vm/src/builtins/traceback.rsvm/src/stdlib/ast/exception.rsvm/src/stdlib/ast/type_parameters.rsvm/src/stdlib/ast/parameter.rsvm/src/stdlib/ast/constant.rsvm/src/stdlib/ast/node.rsvm/src/stdlib/ast/operator.rscompiler/codegen/src/error.rsvm/src/stdlib/ast.rsvm/src/stdlib/ast/type_ignore.rsvm/src/stdlib/ast/elif_else_clause.rsvm/src/stdlib/ast/pattern.rsvm/src/builtins/code.rsvm/src/stdlib/ast/argument.rsvm/src/stdlib/ast/module.rsvm/src/stdlib/ast/basic.rsvm/src/stdlib/ast/statement.rsvm/src/stdlib/ast/other.rscompiler/codegen/src/ir.rscompiler/core/src/bytecode.rsvm/src/vm/vm_new.rscompiler/core/src/marshal.rscompiler/codegen/src/unparse.rsvm/src/stdlib/ast/expression.rscompiler/core/src/lib.rsvm/src/frame.rscompiler/codegen/src/symboltable.rsvm/src/lib.rscompiler/codegen/src/compile.rs
🔇 Additional comments (28)
vm/src/lib.rs (1)
54-57: Stylistic newline insertions look fineThe added blank lines around selected
pub moddeclarations improve visual grouping and do not affect behaviour.cargo fmtwill keep this layout stable, so no further action needed.Also applies to: 67-70, 73-76, 90-93
compiler/codegen/src/ir.rs (2)
3-10: Import consolidation is correct and follows the new re-export pathSwitching to
rustpython_compiler_core::{ OneIndexed, SourceLocation, bytecode::… }removes the externalruff_source_filedependency and matches the core crate’s public re-exports. No functional impact; compilation should succeed as long asrustpython_compiler_coreis in the workspace members list.
39-40: Whitespace tweaks onlyThe extra blank lines between impl blocks aid readability and are neutral regarding generated code.
Also applies to: 48-49, 54-55, 62-63, 85-86, 117-118
vm/src/builtins/traceback.rs (1)
7-8: Updated import path is accurate
OneIndexednow comes fromrustpython_compiler_core; the rest of the file already depends on that crate via other modules. 👍vm/src/builtins/code.rs (2)
18-18: Import path change acknowledgedThe switch to
rustpython_compiler_core::OneIndexedis consistent with the global refactor.
358-361: Type consistency remains intact
OneIndexed::new(first_line_number as _)still yields anOption<OneIndexed>, matching thefirst_line_numberfield’s type; no further change required.vm/src/stdlib/ast/node.rs (1)
2-2: Import source relocation is correct
SourceFileis now re-exported fromrustpython_compiler_core; the rest of the AST helpers remain unchanged.compiler/codegen/src/error.rs (2)
1-1: Import path update looks correct
Switching torustpython_compiler_core::SourceLocationaligns with the new re-export strategy. No further action needed here.
1-1: Replace remainingruff_source_fileimportsWe still have direct references to the old crate throughout the workspace. Please update these to use the
rustpython_compiler_coreexports and remove theruff_source_filedependency:• Files importing from
ruff_source_file:
- compiler/core/src/lib.rs
- compiler/src/lib.rs
- compiler/source/src/lib.rs
Change, for example:
pub use ruff_source_file::{LineIndex, OneIndexed, SourceFile, SourceFileBuilder, SourceLocation};to
pub use rustpython_compiler_core::{LineIndex, OneIndexed, SourceFile, SourceFileBuilder, SourceLocation};• Cargo.toml entries pulling in
ruff_source_file:
- Cargo.toml (root)
- compiler/core/Cargo.toml
- compiler/Cargo.toml
- compiler/source/Cargo.toml
Remove the
ruff_source_file = { … }lines and replace them with your local path or workspace reference torustpython_compiler_coreif needed.After making these changes, run
cargo updateto regenerate Cargo.lock and verify that noruff_source_fileremains in your lock file.⛔ Skipped due to learnings
Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.py : Use ruff for linting Python codeLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in RustLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to Lib/test/**/*.py : When tests fail due to unsupported syntax, keep the test as `@unittest.expectedFailure`, document that it requires PEP 695 support, and focus on tests that can be fixed through Rust code changes onlyLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications onlyLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Always run clippy to lint code (`cargo clippy`) before completing tasks. Fix any warnings or lints that are introduced by your changesLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Follow Rust best practices for error handling and memory managementLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Follow the default rustfmt code style (`cargo fmt` to format)Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files in `Lib/`vm/src/stdlib/ast/constant.rs (1)
4-4: Import updated successfully
SourceFileis now taken fromrustpython_compiler_core; matches the new public re-export.vm/src/stdlib/ast/type_parameters.rs (1)
2-2: Consistent with refactorImport path change looks good and keeps this module in sync with the rest of the tree.
vm/src/stdlib/ast/operator.rs (1)
2-2: LGTMImport source updated; no further issues spotted.
vm/src/stdlib/ast/pattern.rs (1)
2-2: Import path alignedMatches the crate consolidation; everything compiles as expected.
vm/src/stdlib/ast/statement.rs (1)
3-3: AllSourceFileimports updated correctlyRan
rg -n "ruff_source_file::SourceFile"and found no remaining references to the old path. No further changes are needed in vm/src/stdlib/ast/statement.rs.
LGTM.vm/src/stdlib/ast/parameter.rs (1)
2-2: Import path updated correctly
SourceFilenow comes fromrustpython_compiler_core, which matches the new re-export strategy. No further action required.vm/src/stdlib/ast/elif_else_clause.rs (1)
2-2: Consistent migration torustpython_compiler_coreThe import swap is straightforward and keeps the file in sync with the rest of the workspace.
vm/src/stdlib/ast.rs (1)
25-27: Migrate residualruff_source_fileimports torustpython_compiler_coreThe search uncovered three remaining
ruff_source_file::imports that need updating. Please replace them with the correspondingrustpython_compiler_corere-exports:• compiler/src/lib.rs:1
- use ruff_source_file::{SourceFile, SourceFileBuilder, SourceLocation}; + use rustpython_compiler_core::{SourceFile, SourceFileBuilder, SourceLocation};• compiler/source/src/lib.rs:1
- pub use ruff_source_file::{LineIndex, OneIndexed as LineNumber, SourceLocation}; + pub use rustpython_compiler_core::{LineIndex, OneIndexed as LineNumber, SourceLocation};• compiler/core/src/lib.rs:11
- pub use ruff_source_file::{LineIndex, OneIndexed, SourceFile, SourceFileBuilder, SourceLocation}; + pub use rustpython_compiler_core::{LineIndex, OneIndexed, SourceFile, SourceFileBuilder, SourceLocation};After making these changes, rerun your grep check to confirm no further
ruff_source_file::references remain.⛔ Skipped due to learnings
Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.py : Use ruff for linting Python codeLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in RustLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to Lib/test/**/*.py : When tests fail due to unsupported syntax, keep the test as `@unittest.expectedFailure`, document that it requires PEP 695 support, and focus on tests that can be fixed through Rust code changes onlyLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Follow Rust best practices for error handling and memory managementLearnt from: moreal PR: RustPython/RustPython#5847 File: vm/src/stdlib/stat.rs:547-567 Timestamp: 2025-06-27T14:47:28.810Z Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications onlyLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Follow the default rustfmt code style (`cargo fmt` to format)Learnt from: moreal PR: RustPython/RustPython#5847 File: vm/src/stdlib/stat.rs:547-567 Timestamp: 2025-06-27T14:47:28.810Z Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files in `Lib/`Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Always run clippy to lint code (`cargo clippy`) before completing tasks. Fix any warnings or lints that are introduced by your changesvm/src/frame.rs (1)
25-25: Import aligns with the new core crate
SourceLocationnow resolves viarustpython_compiler_core; looks good.vm/src/vm/vm_new.rs (1)
15-15: Replace all remainingruff_source_fileimports withrustpython_compiler_coreThe search revealed three straggling references that need updating:
- compiler/src/lib.rs:1
- compiler/core/src/lib.rs:11
- compiler/source/src/lib.rs:1
Suggested replacements:
--- a/compiler/src/lib.rs +++ b/compiler/src/lib.rs -use ruff_source_file::{SourceFile, SourceFileBuilder, SourceLocation}; +use rustpython_compiler_core::{SourceFile, SourceFileBuilder, SourceLocation};--- a/compiler/core/src/lib.rs +++ b/compiler/core/src/lib.rs -pub use ruff_source_file::{LineIndex, OneIndexed, SourceFile, SourceFileBuilder, SourceLocation}; +pub use rustpython_compiler_core::{LineIndex, OneIndexed, SourceFile, SourceFileBuilder, SourceLocation};--- a/compiler/source/src/lib.rs +++ b/compiler/source/src/lib.rs -pub use ruff_source_file::{LineIndex, OneIndexed as LineNumber, SourceLocation}; +pub use rustpython_compiler_core::{LineIndex, OneIndexed as LineNumber, SourceLocation};After applying these changes, rerun
rg -n "ruff_source_file::"to verify no remaining references.⛔ Skipped due to learnings
Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.py : Use ruff for linting Python codeLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in RustLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to Lib/test/**/*.py : When tests fail due to unsupported syntax, keep the test as `@unittest.expectedFailure`, document that it requires PEP 695 support, and focus on tests that can be fixed through Rust code changes onlyLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Follow Rust best practices for error handling and memory managementLearnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Follow the default rustfmt code style (`cargo fmt` to format)Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications onlyLearnt from: moreal PR: RustPython/RustPython#5847 File: vm/src/stdlib/stat.rs:547-567 Timestamp: 2025-06-27T14:47:28.810Z Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.Learnt from: CR PR: RustPython/RustPython#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-19T03:16:56.511Z Learning: Applies to **/*.rs : Always run clippy to lint code (`cargo clippy`) before completing tasks. Fix any warnings or lints that are introduced by your changesLearnt from: moreal PR: RustPython/RustPython#5847 File: vm/src/stdlib/stat.rs:547-567 Timestamp: 2025-06-27T14:47:28.810Z Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.vm/src/stdlib/ast/expression.rs (1)
7-7: Consistent crate swap – no further action required.
SourceFileis now re-exported fromrustpython_compiler_core; all downstream calls remain unchanged, so the edit is correct.vm/src/stdlib/ast/basic.rs (1)
2-3: Import additions align with usage.
ruff_int_to_bigintand the newSourceFilepath are both consumed below, so clippy shouldn’t complain about unused imports.compiler/codegen/src/unparse.rs (1)
1-4: Import refactor is correct – keepcargo fmthappy.Switching to the granular
ruff_python_astlist plus the newSourceFilepath removes the dependency onruff_source_filewithout changing behaviour. Make sure the long list still respects rustfmt’s line-wrap rules after reformatting.Also applies to: 6-6
vm/src/stdlib/ast/other.rs (1)
3-3: Combined import is accurate.Both
SourceFileandbytecodeare used below; combining into one line is stylistically fine.compiler/core/src/bytecode.rs (1)
4-4: Import path switch looks good.Moving
OneIndexed&SourceLocationtocrate::{…}correctly leverages the new re-exports and removes the external dependency from this module. No functional impact.Also applies to: 10-10
compiler/codegen/src/symboltable.rs (1)
22-22: Correct crate switch; check thatcompiler/codegenno longer depends onruff_source_file.The new import matches the public re-exports in
rustpython_compiler_core.
Verify thatcompiler/codegen/Cargo.tomlhad itsruff_source_fileentry removed, otherwise the dependency graph will still include it unnecessarily.compiler/core/src/lib.rs (1)
11-11: Public re-export is fine, but ensureruff_source_fileremains an explicit dependency of this crate.
rustpython_compiler_corenow acts as the single import hub for these types—great for ergonomics.
However, because you’re re-exporting directly fromruff_source_file,compiler/core/Cargo.tomlmust still list that crate; otherwise downstream crates relying only onrustpython_compiler_coremight build but the core crate itself will fail.compiler/codegen/src/compile.rs (2)
34-36: Imports correctly re-pointed torustpython_compiler_coreThe switch from the external
ruff_source_filecrate to the public re-exports inrustpython_compiler_corelooks clean and keeps the external surface unchanged at the call-sites. Good catch on importingModealongside the three re-exported types.
5843-5844: Test helper import LGTMThe tests now pull
SourceFileBuilderfrom the unified crate; compilation path is consistent with the production code.
| @@ -1,7 +1,6 @@ | |||
| use crate::bytecode::*; | |||
| use crate::{OneIndexed, SourceLocation, bytecode::*}; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Verify root-level re-export of OneIndexed / SourceLocation
marshal.rs now pulls these types from crate, so rustpython_compiler_core::lib.rs must pub use them.
Please confirm with a quick search:
🏁 Script executed:
#!/bin/bash
# Ensure the symbols are publicly exposed at crate root
rg -n $'^\\s*pub\\s+use\\s+ruff_source_file::(OneIndexed|SourceLocation)' compiler/core/src/lib.rsLength of output: 96
Add missing re-exports of OneIndexed and SourceLocation at crate root
The import in marshal.rs (use crate::{OneIndexed, SourceLocation, …};) will fail because these types aren’t publicly re-exported from the crate root.
• File needing update:
compiler/core/src/lib.rs– add the following alongside the otherpub usestatements.
diff --git a/compiler/core/src/lib.rs b/compiler/core/src/lib.rs
index abc1234..def5678 100644
--- a/compiler/core/src/lib.rs
+++ b/compiler/core/src/lib.rs
@@
pub use bytecode::*;
+pub use ruff_source_file::{OneIndexed, SourceLocation};🤖 Prompt for AI Agents
In compiler/core/src/lib.rs near the top where other pub use statements are, add
public re-exports for OneIndexed and SourceLocation to make them accessible from
the crate root. This will fix the import failure in marshal.rs by allowing these
types to be imported publicly from the crate.
youknowone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Motivation: #6016 (review)
Summary by CodeRabbit
Refactor
Style
No changes to user-facing features or functionality.