Fix relative imports in VM and properly promote compiler errorsg

Author: JonesBeach

src/bytecode_vm/compiler/compiler.rs

@@ -17,9 +17,15 @@ mod stmt;
 
 /// A Python bytecode compiler.
 pub struct Compiler {
+    /// Track the filename so we can associate it with later `CodeObjects`.
     filename: String,
+
+    /// Track the module name so we can associate it with later `CodeObjects`.
     module_name: ModuleName,
 
+    /// We must know the package name for relative imports to work.
+    package: ModuleName,
+
     /// Keep a reference to the code object being constructed so we can associate things with it,
     /// (variable names, constants, etc.).
     code_stack: Vec<CodeObject>,
@@ -29,10 +35,11 @@ pub struct Compiler {
 }
 
 impl Compiler {
-    pub fn new(module_name: &ModuleName, filename: &str) -> Self {
+    pub fn new(module_name: &ModuleName, package: &ModuleName, filename: &str) -> Self {
         Self {
             filename: filename.to_string(),
             module_name: module_name.clone(),
+            package: package.clone(),
             code_stack: vec![],
             line_number: 0,
         }
@@ -965,7 +972,8 @@ from .outer import foo
 from .outer import foo
 "#;
         let module_name = ModuleName::from_segments(&["pkg", "mod"]);
-        let code = compile_at_module(text, module_name.clone());
+        let pkg = ModuleName::from_segments(&["pkg"]);
+        let code = compile_at_pkg(text, module_name.clone(), pkg);
 
         let expected = CodeObject {
             module_name,
@@ -994,7 +1002,8 @@ from .outer import foo
 from .outer.inner import foo
 "#;
         let module_name = ModuleName::from_segments(&["pkg", "mod"]);
-        let code = compile_at_module(text, module_name.clone());
+        let pkg = ModuleName::from_segments(&["pkg"]);
+        let code = compile_at_pkg(text, module_name.clone(), pkg);
 
         let expected = CodeObject {
             module_name,
@@ -1023,7 +1032,8 @@ from .outer.inner import foo
 from ..outer import foo
 "#;
         let module_name = ModuleName::from_segments(&["pkg", "mod"]);
-        let err = compile_err_at_module(text, module_name);
+        let pkg = ModuleName::from_segments(&["pkg"]);
+        let err = compile_err_at_pkg(text, module_name, pkg);
 
         match err {
             CompilerError::ImportError(msg) => assert_eq!(

src/bytecode_vm/compiler/compiler/stmt.rs

@@ -3,7 +3,7 @@ use crate::{
         compiler::{opcode::UnsignedOffset, CodeObject, Constant, Opcode},
         Compiler, CompilerError, CompilerResult,
     },
-    domain::{resolve_import_path, FromImportPath, FunctionType, Identifier, ModuleName},
+    domain::{resolve_import_path, FromImportPath, FunctionType, Identifier},
     parser::types::{
         Ast, ConditionalAst, Expr, FromImportMode, LoopIndex, Params, RegularImport, Statement,
         StatementKind,
@@ -334,9 +334,7 @@ impl Compiler {
         import_path: &FromImportPath,
         mode: &FromImportMode,
     ) -> CompilerResult<()> {
-        // TODO pass the real package here, we're not detecting __init__ here at the moment
-        let package = self.module_name.parent().unwrap_or(ModuleName::empty());
-        let module_name = resolve_import_path(import_path, &package)
+        let module_name = resolve_import_path(import_path, &self.package)
             .map_err(|e| CompilerError::import_error(e.message()))?;
 
         let index = self.get_or_set_nonlocal_index(&module_name.as_str())?;

src/bytecode_vm/compiler/error.rs

@@ -1,4 +1,4 @@
-use std::fmt::{Display, Error, Formatter};
+use crate::bytecode_vm::{runtime::types::Exception, VirtualMachine};
 
 #[derive(Clone, PartialEq, Debug)]
 pub enum CompilerError {
@@ -12,15 +12,13 @@ impl CompilerError {
     pub fn import_error(msg: impl Into<String>) -> Self {
         Self::ImportError(msg.into())
     }
-}
 
-impl Display for CompilerError {
-    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+    pub fn into_exception(self, vm: &mut VirtualMachine) -> Exception {
         match self {
-            Self::Unsupported(msg) => write!(f, "Unsupported feature: {msg}"),
-            Self::SyntaxError(msg) => write!(f, "Syntax error: {msg}"),
-            Self::Internal(msg) => write!(f, "Internal error: {msg}"),
-            Self::ImportError(msg) => write!(f, "ImportError: {msg}"),
+            CompilerError::SyntaxError(msg) => Exception::syntax_error(vm.intern_string(&msg)),
+            CompilerError::ImportError(msg) => Exception::import_error(vm.intern_string(&msg)),
+            CompilerError::Unsupported(msg) => Exception::syntax_error(vm.intern_string(&msg)),
+            CompilerError::Internal(msg) => Exception::syntax_error(vm.intern_string(&msg)),
         }
     }
 }

src/bytecode_vm/compiler/test_utils.rs

@@ -12,7 +12,11 @@ use crate::{
 };
 
 fn init() -> Compiler {
-    Compiler::new(&ModuleName::main(), "compiler_unit_test")
+    Compiler::new(
+        &ModuleName::main(),
+        &ModuleName::empty(),
+        "compiler_unit_test",
+    )
 }
 
 fn init_ctx(text: &str) -> VmContext {
@@ -34,9 +38,10 @@ pub fn compile(text: &str) -> CodeObject {
         .expect("Failed to compile test program!")
 }
 
-pub fn compile_at_module(text: &str, module_name: ModuleName) -> CodeObject {
+pub fn compile_at_pkg(text: &str, module: ModuleName, pkg: ModuleName) -> CodeObject {
     let mut ctx = init_ctx(text);
-    ctx.set_module_name(module_name);
+    ctx.set_module(module);
+    ctx.set_pkg(pkg);
     ctx.compile().expect("Failed to compile test program!")
 }
 
@@ -47,9 +52,10 @@ pub fn compile_err(text: &str) -> CompilerError {
     }
 }
 
-pub fn compile_err_at_module(text: &str, module_name: ModuleName) -> CompilerError {
+pub fn compile_err_at_pkg(text: &str, module: ModuleName, pkg: ModuleName) -> CompilerError {
     let mut ctx = init_ctx(text);
-    ctx.set_module_name(module_name);
+    ctx.set_module(module);
+    ctx.set_pkg(pkg);
     match ctx.compile() {
         Ok(_) => panic!("Expected an CompilerError!"),
         Err(e) => e,

src/bytecode_vm/context.rs

@@ -2,8 +2,7 @@
 use crate::domain::Source;
 use crate::{
     bytecode_vm::{
-        compiler::CodeObject, runtime::types::Exception, Compiler, CompilerError, Runtime,
-        VirtualMachine, VmResult, VmValue,
+        compiler::CodeObject, Compiler, CompilerError, Runtime, VirtualMachine, VmResult, VmValue,
     },
     core::{Container, Interpreter},
     domain::{MemphisResult, MemphisValue, ModuleName, ModuleOrigin, Text},
@@ -15,6 +14,7 @@ use crate::{
 pub struct VmContext {
     lexer: Lexer,
     module_name: ModuleName,
+    package: ModuleName,
     path_str: String,
     vm: VirtualMachine,
 }
@@ -23,12 +23,19 @@ impl VmContext {
     pub fn new(text: Text, origin: ModuleOrigin) -> Self {
         let state = Container::new(MemphisState::init(origin.clone()));
         let runtime = Container::new(Runtime::new());
-        Self::from_state(ModuleName::main(), text, origin, state, runtime)
+        Self::from_state(
+            ModuleName::main(),
+            ModuleName::empty(),
+            text,
+            origin,
+            state,
+            runtime,
+        )
     }
 
-    /// Initialize a context from a [`Source`] and existing treewalk state.
     pub fn from_state(
         module_name: ModuleName,
+        package: ModuleName,
         text: Text,
         origin: ModuleOrigin,
         state: Container<MemphisState>,
@@ -37,16 +44,17 @@ impl VmContext {
         Self {
             lexer: Lexer::new(&text),
             module_name,
+            package,
             path_str: origin.path_str(),
             vm: VirtualMachine::new(state, runtime),
         }
     }
 
     pub fn run_inner(&mut self) -> VmResult<VmValue> {
-        // TODO use a real syntax error here
-        let code = self
-            .compile()
-            .map_err(|_e| self.vm.raise(Exception::syntax_error()))?;
+        let code = self.compile().map_err(|e| {
+            let exc = e.into_exception(&mut self.vm);
+            self.vm.raise(exc)
+        })?;
         self.vm.execute(code)
     }
 
@@ -57,7 +65,7 @@ impl VmContext {
             .map_err(|e| CompilerError::SyntaxError(e.to_string()))?;
         ast.rewrite_last_expr_to_return();
 
-        let mut compiler = Compiler::new(&self.module_name, &self.path_str);
+        let mut compiler = Compiler::new(&self.module_name, &self.package, &self.path_str);
         compiler.compile(&ast)
     }
 
@@ -75,10 +83,15 @@ impl VmContext {
     }
 
     #[cfg(test)]
-    pub fn set_module_name(&mut self, name: ModuleName) {
+    pub fn set_module(&mut self, name: ModuleName) {
         self.module_name = name;
     }
 
+    #[cfg(test)]
+    pub fn set_pkg(&mut self, name: ModuleName) {
+        self.package = name;
+    }
+
     #[cfg(any(test, feature = "wasm"))]
     pub fn from_text(text: Text) -> Self {
         Self::new(text, ModuleOrigin::Stdin)

src/bytecode_vm/interpreter.rs

@@ -1283,6 +1283,12 @@ b = f.bar()
         assert_read_eq!(ctx, "x", int!(2));
     }
 
+    #[test]
+    fn relative_import_from_package() {
+        let ctx = run_path("src/fixtures/imports/pkg_test/test_app.py");
+        assert_read_eq!(ctx, "a", int!(111));
+    }
+
     #[test]
     fn regular_import_error() {
         let text = r#"

src/bytecode_vm/runtime/modules/asyncio.rs

@@ -48,8 +48,8 @@ fn expect_float_or_raise(vm: &mut VirtualMachine, value: &VmValue) -> VmResult<f
     match value.as_float() {
         Some(i) => Ok(i),
         None => {
-            let msg = VmValue::String("Expected a float".to_string());
-            Exception::type_error(vm.heapify(msg)).raise(vm)
+            let msg = vm.intern_string("Expected a float");
+            Exception::type_error(msg).raise(vm)
         }
     }
 }
@@ -61,8 +61,8 @@ fn expect_coroutine_or_raise(
     match value.as_coroutine() {
         Some(i) => Ok(i.clone()),
         None => {
-            let msg = VmValue::String("Expected a coroutine".to_string());
-            Exception::type_error(vm.heapify(msg)).raise(vm)
+            let msg = vm.intern_string("Expected a coroutine");
+            Exception::type_error(msg).raise(vm)
         }
     }
 }