-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Refine documentation in interpreter.rs #6566
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
Changes from 1 commit
c35986a
2cae714
ec9cc6d
7e7428b
72b4343
e1eebd3
c4d502a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ pub type InitHook = Box<dyn FnOnce(&mut VirtualMachine)>; | |||||||||||||
| /// # Basic Usage | ||||||||||||||
| /// ```no_run | ||||||||||||||
| /// use rustpython::InterpreterConfig; | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// let interpreter = InterpreterConfig::new() | ||||||||||||||
| /// .init_stdlib() | ||||||||||||||
| /// .interpreter(); | ||||||||||||||
|
|
@@ -17,13 +17,13 @@ pub type InitHook = Box<dyn FnOnce(&mut VirtualMachine)>; | |||||||||||||
| /// ```no_run | ||||||||||||||
| /// use rustpython_vm::Settings; | ||||||||||||||
| /// use rustpython::InterpreterConfig; | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// let mut settings = Settings::default(); | ||||||||||||||
| /// settings.debug = 1; | ||||||||||||||
| /// // Add paths to allow importing Python libraries | ||||||||||||||
| /// settings.path_list.push("Lib".to_owned()); // standard library directory | ||||||||||||||
| /// settings.path_list.push("".to_owned()); // current working directory | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// let interpreter = InterpreterConfig::new() | ||||||||||||||
| /// .settings(settings) | ||||||||||||||
| /// .interpreter(); | ||||||||||||||
|
|
@@ -33,12 +33,12 @@ pub type InitHook = Box<dyn FnOnce(&mut VirtualMachine)>; | |||||||||||||
| /// ```no_run | ||||||||||||||
| /// use rustpython::InterpreterConfig; | ||||||||||||||
| /// use rustpython_vm::{VirtualMachine, PyRef, builtins::PyModule}; | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// fn make_custom_module(vm: &VirtualMachine) -> PyRef<PyModule> { | ||||||||||||||
| /// // Your module implementation | ||||||||||||||
| /// # todo!() | ||||||||||||||
| /// } | ||||||||||||||
|
Comment on lines
+37
to
+40
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still misleading about making a module.
Suggested change
|
||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// let interpreter = InterpreterConfig::new() | ||||||||||||||
| /// .init_stdlib() | ||||||||||||||
| /// .add_native_module( | ||||||||||||||
|
|
@@ -60,7 +60,7 @@ impl InterpreterConfig { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Build the interpreter with the current configuration | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// # Panics | ||||||||||||||
| /// May panic if initialization hooks encounter fatal errors | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, this is a caveat of vm init, but probably helpful |
||||||||||||||
| pub fn interpreter(self) -> Interpreter { | ||||||||||||||
|
|
@@ -73,27 +73,27 @@ impl InterpreterConfig { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Set custom settings for the interpreter | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// If called multiple times, only the last settings will be used | ||||||||||||||
| pub fn settings(mut self, settings: Settings) -> Self { | ||||||||||||||
| self.settings = Some(settings); | ||||||||||||||
| self | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Add a custom initialization hook | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// Hooks are executed in the order they are added during interpreter creation | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| pub fn init_hook(mut self, hook: InitHook) -> Self { | ||||||||||||||
| self.init_hooks.push(hook); | ||||||||||||||
| self | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Add a native module to the interpreter | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// # Arguments | ||||||||||||||
| /// * `name` - The module name that will be used for imports | ||||||||||||||
| /// * `make_module` - Function that creates the module when called | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// # Example | ||||||||||||||
| /// ```no_run | ||||||||||||||
| /// # use rustpython::InterpreterConfig; | ||||||||||||||
|
|
@@ -114,7 +114,7 @@ impl InterpreterConfig { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Initialize the Python standard library | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// This adds all standard library modules to the interpreter. | ||||||||||||||
| /// Requires the `stdlib` feature to be enabled at compile time. | ||||||||||||||
| #[cfg(feature = "stdlib")] | ||||||||||||||
|
|
@@ -123,17 +123,19 @@ impl InterpreterConfig { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Initialize the Python standard library (no-op without stdlib feature) | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// When the `stdlib` feature is not enabled, this method does nothing | ||||||||||||||
| /// and prints a warning. Enable the `stdlib` feature to use the standard library. | ||||||||||||||
| #[cfg(not(feature = "stdlib"))] | ||||||||||||||
| pub fn init_stdlib(self) -> Self { | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this useful? Users get compile error when no feature set. But after changes, it became runtime error. |
||||||||||||||
| eprintln!("Warning: stdlib feature is not enabled. Standard library will not be available."); | ||||||||||||||
| eprintln!( | ||||||||||||||
| "Warning: stdlib feature is not enabled. Standard library will not be available." | ||||||||||||||
| ); | ||||||||||||||
| self | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Convenience method to set the debug level | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// # Example | ||||||||||||||
| /// ```no_run | ||||||||||||||
| /// # use rustpython::InterpreterConfig; | ||||||||||||||
|
|
@@ -147,7 +149,7 @@ impl InterpreterConfig { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Convenience method to add a single path to the module search paths | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// # Example | ||||||||||||||
| /// ```no_run | ||||||||||||||
| /// # use rustpython::InterpreterConfig; | ||||||||||||||
|
|
@@ -157,20 +159,23 @@ impl InterpreterConfig { | |||||||||||||
| /// .interpreter(); | ||||||||||||||
| /// ``` | ||||||||||||||
| pub fn add_path(mut self, path: impl Into<String>) -> Self { | ||||||||||||||
| self.settings.get_or_insert_with(Default::default).path_list.push(path.into()); | ||||||||||||||
| self.settings | ||||||||||||||
| .get_or_insert_with(Default::default) | ||||||||||||||
| .path_list | ||||||||||||||
| .push(path.into()); | ||||||||||||||
| self | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Add multiple paths to the module search paths at once | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// # Example | ||||||||||||||
| /// ```no_run | ||||||||||||||
| /// # use rustpython::InterpreterConfig; | ||||||||||||||
| /// let interpreter = InterpreterConfig::new() | ||||||||||||||
| /// .add_paths(vec!["Lib", ".", "custom_modules"]) | ||||||||||||||
| /// .interpreter(); | ||||||||||||||
| /// ``` | ||||||||||||||
| pub fn add_paths<I, S>(mut self, paths: I) -> Self | ||||||||||||||
| pub fn add_paths<I, S>(mut self, paths: I) -> Self | ||||||||||||||
| where | ||||||||||||||
| I: IntoIterator<Item = S>, | ||||||||||||||
| S: Into<String>, | ||||||||||||||
|
|
@@ -182,7 +187,7 @@ impl InterpreterConfig { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Initialize the standard library modules | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// This function sets up both native modules and handles frozen/dynamic stdlib loading | ||||||||||||||
| #[cfg(feature = "stdlib")] | ||||||||||||||
| pub fn init_stdlib(vm: &mut VirtualMachine) { | ||||||||||||||
|
|
@@ -196,13 +201,13 @@ pub fn init_stdlib(vm: &mut VirtualMachine) { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Setup frozen standard library | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// Used when the stdlib is compiled into the binary | ||||||||||||||
| #[cfg(all(feature = "stdlib", feature = "freeze-stdlib"))] | ||||||||||||||
| fn setup_frozen_stdlib(vm: &mut VirtualMachine) { | ||||||||||||||
| vm.add_frozen(rustpython_pylib::FROZEN_STDLIB); | ||||||||||||||
|
|
||||||||||||||
| // FIXME: Remove this hack once sys._stdlib_dir is properly implemented | ||||||||||||||
| // FIXME: Remove this hack once sys._stdlib_dir is properly implemented | ||||||||||||||
| // or _frozen_importlib doesn't depend on it anymore. | ||||||||||||||
| // The assert ensures _stdlib_dir doesn't already exist before we set it | ||||||||||||||
| assert!(vm.sys_module.get_attr("_stdlib_dir", vm).is_err()); | ||||||||||||||
|
|
@@ -216,7 +221,7 @@ fn setup_frozen_stdlib(vm: &mut VirtualMachine) { | |||||||||||||
| } | ||||||||||||||
|
Comment on lines
+207
to
+221
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for all _stdlib_dir references
rg -n "_stdlib_dir" --type=rust -C3Repository: RustPython/RustPython Length of output: 897 🏁 Script executed: # Look for where setup_frozen_stdlib is called to understand initialization order
rg -n "setup_frozen_stdlib" --type=rust -B2 -A2Repository: RustPython/RustPython Length of output: 624 🏁 Script executed: # Check the VirtualMachine initialization and sys_module setup
rg -n "sys_module" src/interpreter.rs -B1 -A1 | head -50Repository: RustPython/RustPython Length of output: 259 🏁 Script executed: # Look at the full context around line 197 to see initialization order
sed -n '180,210p' src/interpreter.rsRepository: RustPython/RustPython Length of output: 1046 🏁 Script executed: # Check if sys_module is created before or after setup_frozen_stdlib
rg -n "sys_module.*=" src/interpreter.rs | head -20Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Look for any module initialization that might touch _stdlib_dir
rg -n "def.*_stdlib_dir|_stdlib_dir.*=" --type=pythonRepository: RustPython/RustPython Length of output: 95 🏁 Script executed: # Search for where sys module is initially created/initialized
rg -n "sys_module" src/interpreter.rs | head -30Repository: RustPython/RustPython Length of output: 155 🏁 Script executed: # Look for VirtualMachine::new to understand initialization order
ast-grep --pattern 'impl VirtualMachine {
$$$
pub fn new($_) {
$$$
}
$$$
}'Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Search for any code path that could set _stdlib_dir before setup_frozen_stdlib
rg -n "set_attr.*_stdlib_dir|_stdlib_dir" src/Repository: RustPython/RustPython Length of output: 401 🏁 Script executed: # Look for VirtualMachine creation and sys_module initialization
rg -n "sys_module\s*=" src/interpreter.rs -B2 -A2Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Find where init_stdlib is called to understand the full initialization sequence
rg -n "init_stdlib" --type=rust -B3 -A3Repository: RustPython/RustPython Length of output: 5257 🏁 Script executed: # Check if there's any Python-level code or module that might set _stdlib_dir
fd --type f -e py | xargs rg "_stdlib_dir" 2>/dev/null || echo "No Python files found with _stdlib_dir"Repository: RustPython/RustPython Length of output: 103 🏁 Script executed: # Look for VirtualMachine::new and sys module initialization
rg -n "fn new\(" src/interpreter.rs crates/vm/src/vm/mod.rs -A 50 | grep -A 50 "impl.*VirtualMachine" | head -100Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Search in vm crate for sys_module creation
rg -n "sys_module\s*:" crates/vm/src/ -B2 -A2Repository: RustPython/RustPython Length of output: 720 🏁 Script executed: # Look at the actual flow: when is sys module initialized relative to init_stdlib
grep -n "pub fn interpreter\|init_stdlib\|init_hook" src/interpreter.rs | head -20Repository: RustPython/RustPython Length of output: 628 🏁 Script executed: # Look at the Interpreter creation flow to see when sys_module is created
rg -n "pub fn interpreter" src/interpreter.rs -A 20Repository: RustPython/RustPython Length of output: 814 🏁 Script executed: # Find where VirtualMachine is constructed
rg -n "VirtualMachine\s*{" crates/vm/src/vm/mod.rs -B5 -A20 | head -80Repository: RustPython/RustPython Length of output: 164 🏁 Script executed: # Look for sys module initialization in new()
rg -n "sys_module.*PyModule\|fn new\(" crates/vm/src/vm/mod.rs -A 30 | head -100Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Find Interpreter::with_init to see when sys_module is created
rg -n "with_init\|pub fn with_init" crates/vm/src/ --type=rust -A 30 | head -100Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Look for sys module initialization/creation
rg -n "PyModule::new\(.*sys\|sys.*PyModule" crates/vm/src/ --type=rust -B2 -A2Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Check if sys module is set up before or after hooks are called
rg -n "pub fn with_init" crates/vm/src/ -A 50Repository: RustPython/RustPython Length of output: 3824 🏁 Script executed: # Look at VirtualMachine::new to see when sys_module is created
rg -n "impl VirtualMachine" crates/vm/src/vm/mod.rs -A 100 | grep -A 50 "pub fn new"Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Find the VirtualMachine::new implementation
sed -n '500,700p' crates/vm/src/vm/mod.rs | head -150Repository: RustPython/RustPython Length of output: 5630 🏁 Script executed: # Look for the initialize method to see what it does
rg -n "pub fn initialize\(" crates/vm/src/vm/mod.rs -A 30Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Find VirtualMachine::new
rg -n "fn new\(" crates/vm/src/vm/mod.rs -A 40Repository: RustPython/RustPython Length of output: 1734 🏁 Script executed: # Find the initialize method
rg -n "fn initialize\(" crates/vm/src/vm/mod.rs -A 50Repository: RustPython/RustPython Length of output: 2578 🏁 Script executed: # Look at stdlib::sys::init_module to see if it sets _stdlib_dir
rg -n "pub fn init_module\|_stdlib_dir" crates/stdlib/src/sys.rs -B2 -A2Repository: RustPython/RustPython Length of output: 129 🏁 Script executed: # Check what attributes stdlib::sys::init_module actually sets
rg -n "def __module_def\|init_module" crates/stdlib/src/sys.rs -A 80 | head -150Repository: RustPython/RustPython Length of output: 129 🏁 Script executed: # Find where sys module is defined
fd "sys.rs" --type fRepository: RustPython/RustPython Length of output: 92 🏁 Script executed: # Search for sys module definition
rg -n "stdlib.*sys\|pub fn init_module.*sys" --type=rust | head -20Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Look at initialization order more carefully
sed -n '280,320p' crates/vm/src/vm/mod.rsRepository: RustPython/RustPython Length of output: 1877 🏁 Script executed: # Check stdlib::sys::init_module to see what it does
rg -n "pub fn init_module" crates/vm/src/stdlib/sys.rs -A 100Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Search for _stdlib_dir in sys.rs
rg -n "_stdlib_dir" crates/vm/src/stdlib/sys.rsRepository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Look at the full sys.rs file to understand initialization
head -100 crates/vm/src/stdlib/sys.rsRepository: RustPython/RustPython Length of output: 3318 🏁 Script executed: # Check if init_module sets _stdlib_dir or any similar attribute
cat crates/vm/src/stdlib/sys.rs | head -200Repository: RustPython/RustPython Length of output: 6292 🏁 Script executed: # Look at the full sys module to see if _stdlib_dir is anywhere
wc -l crates/vm/src/stdlib/sys.rsRepository: RustPython/RustPython Length of output: 97 🏁 Script executed: # Search the entire file for _stdlib_dir
grep -n "_stdlib_dir" crates/vm/src/stdlib/sys.rsRepository: RustPython/RustPython Length of output: 47 🏁 Script executed: # Get a sense of the file structure
tail -100 crates/vm/src/stdlib/sys.rsRepository: RustPython/RustPython Length of output: 2953 🏁 Script executed: # Check if any other code sets _stdlib_dir before setup_frozen_stdlib
rg -n "set_attr.*_stdlib_dir|_stdlib_dir.*set" --type=rustRepository: RustPython/RustPython Length of output: 164 🏁 Script executed: # Check the initialization sequence in detail - get the full setup_frozen_stdlib and init_stdlib flow
sed -n '190,225p' src/interpreter.rsRepository: RustPython/RustPython Length of output: 1265 🏁 Script executed: # Verify the timing: check if any init_hook before setup_frozen_stdlib could set _stdlib_dir
rg -n "init_hook\|setup_frozen_stdlib\|init_stdlib" src/interpreter.rs | head -30Repository: RustPython/RustPython Length of output: 47 The The sys module is created as an empty struct in However, the current if vm.sys_module.get_attr("_stdlib_dir", vm).is_ok() {
panic!("sys._stdlib_dir already exists; stdlib initialization called in unexpected order");
}This provides better debugging information if the initialization invariant is ever violated. 🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| /// Setup dynamic standard library loading from filesystem | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// Used when the stdlib is loaded from disk at runtime | ||||||||||||||
| #[cfg(all(feature = "stdlib", not(feature = "freeze-stdlib")))] | ||||||||||||||
| fn setup_dynamic_stdlib(vm: &mut VirtualMachine) { | ||||||||||||||
|
|
@@ -233,7 +238,7 @@ fn setup_dynamic_stdlib(vm: &mut VirtualMachine) { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Collect standard library paths from build-time configuration | ||||||||||||||
| /// | ||||||||||||||
| /// | ||||||||||||||
| /// Checks BUILDTIME_RUSTPYTHONPATH environment variable or uses default pylib path | ||||||||||||||
| #[cfg(all(feature = "stdlib", not(feature = "freeze-stdlib")))] | ||||||||||||||
| fn collect_stdlib_paths() -> Vec<String> { | ||||||||||||||
|
|
@@ -281,9 +286,7 @@ mod tests { | |||||||||||||
|
|
||||||||||||||
| #[test] | ||||||||||||||
| fn test_add_multiple_paths_sequential() { | ||||||||||||||
| let config = InterpreterConfig::new() | ||||||||||||||
| .add_path("path1") | ||||||||||||||
| .add_path("path2"); | ||||||||||||||
| let config = InterpreterConfig::new().add_path("path1").add_path("path2"); | ||||||||||||||
| let settings = config.settings.unwrap(); | ||||||||||||||
| assert_eq!(settings.path_list.len(), 2); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
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.
Is there benefits to add
no_runhere?no_runmeans the code can be outdated easy without doctestUh oh!
There was an error while loading. Please reload this page.
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.
The original code used compile_fail, which indicates the example is expected to fail compilation. However the code is valid and should compile successfully it just can't run in a doc test environment so I changed it to no_run for clarity.
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.
Only one test had compile_fail, and the others were possible to be run on doc test environment.