refactor: Use VariableInfo from linter Context in generator

The `VariableInfo` in `Expression` is not really used in the parser,
except for its `expression_type` field.

As a first step of removing `VariableInfo` from the `Expression`
in the parser, we're exposing the Linter's `Context` (which
has this information) to the Instruction Generator.

There, the Instruction Generator can get the necessary information
from the Linter's Context, instead of the `Expression`.

Author: ngeor

rusty_basic/src/bin/rusty_basic.rs

@@ -1,6 +1,6 @@
-use rusty_basic::instruction_generator::generate_instructions;
+use rusty_basic::instruction_generator::{generate_instructions, unwrap_linter_context};
 use rusty_basic::interpreter::{new_default_interpreter, InterpreterTrait};
-use rusty_linter::{lint, HasUserDefinedTypes};
+use rusty_linter::{lint, Context};
 use rusty_parser::parse_main_file;
 use rusty_parser::Program;
 use std::env;
@@ -33,10 +33,10 @@ fn on_parsed(program: Program, run_options: RunOptions) {
     }
 }
 
-fn on_linted(program: Program, linter_context: impl HasUserDefinedTypes, run_options: RunOptions) {
-    // TODO propagate linter_context to instruction_generator, so that it won't read VariableInfo from Expression
-    let instruction_generator_result = generate_instructions(program);
-    let mut interpreter = new_default_interpreter(linter_context);
+fn on_linted(program: Program, linter_context: Context, run_options: RunOptions) {
+    let (linter_names, user_defined_types) = unwrap_linter_context(linter_context);
+    let instruction_generator_result = generate_instructions(program, linter_names);
+    let mut interpreter = new_default_interpreter(user_defined_types);
     run_options.set_current_dir_if_apache();
     match interpreter.interpret(instruction_generator_result) {
         Ok(_) => (),

rusty_basic/src/instruction_generator/expression.rs

@@ -109,20 +109,28 @@ impl InstructionGenerator {
         let Positioned { element: expr, pos } = expr_pos;
 
         match expr {
-            Expression::Variable(var_name, VariableInfo { shared, .. }) => {
+            Expression::Variable(var_name, ..) => {
+                let linter_var_info: &VariableInfo = self
+                    .linter_names
+                    .get_resolved_variable_info(&self.current_subprogram, &var_name);
+
                 self.push(
                     Instruction::VarPathName(RootPath {
                         name: var_name,
-                        shared,
+                        shared: linter_var_info.shared,
                     }),
                     pos,
                 );
             }
-            Expression::ArrayElement(array_name, indices, VariableInfo { shared, .. }) => {
+            Expression::ArrayElement(array_name, indices, ..) => {
+                let linter_var_info: &VariableInfo = self
+                    .linter_names
+                    .get_resolved_variable_info(&self.current_subprogram, &array_name);
+
                 self.push(
                     Instruction::VarPathName(RootPath {
                         name: array_name,
-                        shared,
+                        shared: linter_var_info.shared,
                     }),
                     pos,
                 );

rusty_basic/src/instruction_generator/instruction_generator.rs

@@ -4,25 +4,31 @@ use crate::instruction_generator::subprogram_info::{
 };
 use crate::RuntimeError;
 use rusty_common::{AtPos, CaseInsensitiveString, Position, Positioned};
-use rusty_linter::SubprogramName;
-use rusty_parser::BuiltInFunction;
+use rusty_linter::{Context, Names, SubprogramName};
 use rusty_parser::BuiltInSub;
 use rusty_parser::{
     BareName, DimVar, Expression, ExpressionPos, ExpressionType, FileHandle,
     FunctionImplementation, GlobalStatement, HasExpressionType, Name, Parameter, Program,
     QualifiedName, Statement, Statements, SubImplementation, TypeQualifier,
 };
+use rusty_parser::{BuiltInFunction, UserDefinedTypes};
 use rusty_variant::Variant;
 
+pub fn unwrap_linter_context(linter_context: Context) -> (Names, UserDefinedTypes) {
+    let (pre_linter_result, _, linter_names) = linter_context.unwrap();
+    let (_, _, user_defined_types) = pre_linter_result.unwrap();
+    (linter_names, user_defined_types)
+}
+
 /// Generates instructions for the given program.
-pub fn generate_instructions(program: Program) -> InstructionGeneratorResult {
+pub fn generate_instructions(program: Program, linter_names: Names) -> InstructionGeneratorResult {
     // pass 1: collect function/sub names -> parameter names, in order to use them in function/sub calls
     // the parameter names and types are needed
     let mut subprogram_info_collector = SubprogramInfoCollector::default();
     subprogram_info_collector.visit(&program);
     let subprogram_parameters: SubprogramInfoRepository = subprogram_info_collector.into();
     // pass 2 generate with labels still unresolved
-    let mut generator = InstructionGenerator::new(subprogram_parameters);
+    let mut generator = InstructionGenerator::new(subprogram_parameters, linter_names);
     generator.generate_unresolved(program);
     let InstructionGenerator {
         instructions,
@@ -260,15 +266,17 @@ pub struct InstructionGenerator {
     pub statement_addresses: Vec<usize>,
     pub subprogram_info_repository: SubprogramInfoRepository,
     pub current_subprogram: Option<SubprogramName>,
+    pub linter_names: Names,
 }
 
 impl InstructionGenerator {
-    fn new(subprogram_info_repository: SubprogramInfoRepository) -> Self {
+    fn new(subprogram_info_repository: SubprogramInfoRepository, linter_names: Names) -> Self {
         Self {
             instructions: vec![],
             statement_addresses: vec![],
             subprogram_info_repository,
             current_subprogram: None,
+            linter_names,
         }
     }
 

rusty_basic/src/instruction_generator/test_utils.rs

@@ -1,18 +1,20 @@
 use crate::instruction_generator::{
-    generate_instructions, Instruction, InstructionGeneratorResult, InstructionPos,
+    generate_instructions, unwrap_linter_context, Instruction, InstructionGeneratorResult,
+    InstructionPos,
 };
 use rusty_common::NoPosContainer;
-use rusty_linter::{lint, HasUserDefinedTypes};
-use rusty_parser::parse;
+use rusty_linter::lint;
+use rusty_parser::{parse, UserDefinedTypes};
 
 pub fn generate_instructions_str_with_types(
     input: &str,
-) -> (InstructionGeneratorResult, impl HasUserDefinedTypes) {
+) -> (InstructionGeneratorResult, UserDefinedTypes) {
     let program = parse(input);
-    let (linted_program, user_defined_types_holder) = lint(program).expect("Linter should succeed");
+    let (linted_program, linter_context) = lint(program).expect("Linter should succeed");
+    let (linter_names, user_defined_types) = unwrap_linter_context(linter_context);
     (
-        generate_instructions(linted_program),
-        user_defined_types_holder,
+        generate_instructions(linted_program, linter_names),
+        user_defined_types,
     )
 }
 

rusty_basic/src/interpreter/interpreter.rs

@@ -24,13 +24,7 @@ use rusty_parser::UserDefinedTypes;
 use rusty_variant::Variant;
 use std::collections::VecDeque;
 
-pub struct Interpreter<
-    TStdlib: Stdlib,
-    TStdIn: Input,
-    TStdOut: Printer,
-    TLpt1: Printer,
-    U: HasUserDefinedTypes,
-> {
+pub struct Interpreter<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer> {
     /// Offers system calls
     stdlib: TStdlib,
 
@@ -49,7 +43,7 @@ pub struct Interpreter<
     screen: Box<dyn Screen>,
 
     /// Holds the definition of user defined types
-    linter_context: U,
+    user_defined_types: UserDefinedTypes,
 
     /// Contains variables and constants, collects function/sub arguments.
     context: Context,
@@ -87,16 +81,16 @@ pub struct Interpreter<
     def_seg: Option<usize>,
 }
 
-impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer, U: HasUserDefinedTypes>
-    HasUserDefinedTypes for Interpreter<TStdlib, TStdIn, TStdOut, TLpt1, U>
+impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer> HasUserDefinedTypes
+    for Interpreter<TStdlib, TStdIn, TStdOut, TLpt1>
 {
     fn user_defined_types(&self) -> &UserDefinedTypes {
-        self.linter_context.user_defined_types()
+        &self.user_defined_types
     }
 }
 
-impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer, U: HasUserDefinedTypes>
-    InterpreterTrait for Interpreter<TStdlib, TStdIn, TStdOut, TLpt1, U>
+impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer> InterpreterTrait
+    for Interpreter<TStdlib, TStdIn, TStdOut, TLpt1>
 {
     type TStdlib = TStdlib;
     type TStdIn = TStdIn;
@@ -237,33 +231,32 @@ impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer, U: HasUse
     }
 }
 
-pub type DefaultInterpreter<U> = Interpreter<
+pub type DefaultInterpreter = Interpreter<
     DefaultStdlib,
     ReadInputSource<std::io::Stdin>,
     WritePrinter<std::io::Stdout>,
     WritePrinter<Lpt1Write>,
-    U,
 >;
 
-pub fn new_default_interpreter<U: HasUserDefinedTypes>(linter_context: U) -> DefaultInterpreter<U> {
+pub fn new_default_interpreter(user_defined_types: UserDefinedTypes) -> DefaultInterpreter {
     let stdlib = DefaultStdlib;
     let stdin = ReadInputSource::new(std::io::stdin());
     let stdout = WritePrinter::new(std::io::stdout());
     let lpt1 = WritePrinter::new(Lpt1Write {});
     let screen = CrossTermScreen::default();
-    Interpreter::new(stdlib, stdin, stdout, lpt1, screen, linter_context)
+    Interpreter::new(stdlib, stdin, stdout, lpt1, screen, user_defined_types)
 }
 
-impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer, U: HasUserDefinedTypes>
-    Interpreter<TStdlib, TStdIn, TStdOut, TLpt1, U>
+impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer>
+    Interpreter<TStdlib, TStdIn, TStdOut, TLpt1>
 {
     pub fn new<TScreen: Screen + 'static>(
         stdlib: TStdlib,
         stdin: TStdIn,
         stdout: TStdOut,
         lpt1: TLpt1,
         screen: TScreen,
-        linter_context: U,
+        user_defined_types: UserDefinedTypes,
     ) -> Self {
         Self {
             stdlib,
@@ -277,7 +270,7 @@ impl<TStdlib: Stdlib, TStdIn: Input, TStdOut: Printer, TLpt1: Printer, U: HasUse
             register_stack: vec![Registers::new()],
             stacktrace: vec![],
             file_manager: FileManager::new(),
-            linter_context,
+            user_defined_types,
             var_path_stack: VecDeque::new(),
             by_ref_stack: VecDeque::new(),
             function_result: None,

rusty_basic/src/interpreter/test_utils.rs

@@ -1,15 +1,16 @@
 use crate::instruction_generator::test_utils::generate_instructions_str_with_types;
-use crate::instruction_generator::{generate_instructions, InstructionGeneratorResult};
+use crate::instruction_generator::{
+    generate_instructions, unwrap_linter_context, InstructionGeneratorResult,
+};
 use crate::interpreter::interpreter::Interpreter;
 use crate::interpreter::interpreter_trait::InterpreterTrait;
 use crate::interpreter::read_input::ReadInputSource;
 use crate::interpreter::screen::{CrossTermScreen, HeadlessScreen};
 use crate::interpreter::write_printer::WritePrinter;
 use crate::interpreter::Stdlib;
 use crate::RuntimeErrorPos;
-use rusty_linter;
-use rusty_linter::{lint, HasUserDefinedTypes};
-use rusty_parser::parse_main_file;
+use rusty_linter::lint;
+use rusty_parser::{parse_main_file, UserDefinedTypes};
 use std::collections::HashMap;
 use std::fs::File;
 use std::io::Read;
@@ -30,15 +31,15 @@ impl<S> MockInterpreterTrait for S where
 {
 }
 
-fn mock_interpreter_for_user_defined_types<U: HasUserDefinedTypes>(
-    user_defined_types_holder: U,
+fn mock_interpreter_for_user_defined_types(
+    user_defined_types: UserDefinedTypes,
 ) -> impl MockInterpreterTrait {
     let stdlib = MockStdlib::default();
-    mock_interpreter_for_user_defined_types_stdlib(user_defined_types_holder, stdlib)
+    mock_interpreter_for_user_defined_types_stdlib(user_defined_types, stdlib)
 }
 
-fn mock_interpreter_for_user_defined_types_stdlib<U: HasUserDefinedTypes>(
-    user_defined_types_holder: U,
+fn mock_interpreter_for_user_defined_types_stdlib(
+    user_defined_types: UserDefinedTypes,
     stdlib: MockStdlib,
 ) -> impl MockInterpreterTrait {
     let stdin = ReadInputSource::new(MockStdin { stdin: vec![] });
@@ -54,7 +55,7 @@ fn mock_interpreter_for_user_defined_types_stdlib<U: HasUserDefinedTypes>(
             stdout,
             lpt1,
             HeadlessScreen {},
-            user_defined_types_holder,
+            user_defined_types,
         )
     } else {
         Interpreter::new(
@@ -63,20 +64,20 @@ fn mock_interpreter_for_user_defined_types_stdlib<U: HasUserDefinedTypes>(
             stdout,
             lpt1,
             CrossTermScreen::default(),
-            user_defined_types_holder,
+            user_defined_types,
         )
     }
 }
 
 pub fn mock_interpreter_for_input(
     input: &str,
 ) -> (InstructionGeneratorResult, impl MockInterpreterTrait) {
-    let (instruction_generator_result, user_defined_types_holder) =
+    let (instruction_generator_result, user_defined_types) =
         generate_instructions_str_with_types(input);
     // println!("{:#?}", instruction_generator_result.instructions);
     (
         instruction_generator_result,
-        mock_interpreter_for_user_defined_types(user_defined_types_holder),
+        mock_interpreter_for_user_defined_types(user_defined_types),
     )
 }
 
@@ -96,12 +97,12 @@ pub fn interpret_err(input: &str) -> RuntimeErrorPos {
 }
 
 pub fn interpret_with_raw_input(input: &str, raw_input: &str) -> impl MockInterpreterTrait {
-    let (instruction_generator_result, user_defined_types_holder) =
+    let (instruction_generator_result, user_defined_types) =
         generate_instructions_str_with_types(input);
     // for i in instructions.iter() {
     //     println!("{:?}", i.as_ref());
     // }
-    let mut interpreter = mock_interpreter_for_user_defined_types(user_defined_types_holder);
+    let mut interpreter = mock_interpreter_for_user_defined_types(user_defined_types);
     if !raw_input.is_empty() {
         interpreter.stdin().add_next_input(raw_input);
     }
@@ -112,11 +113,11 @@ pub fn interpret_with_raw_input(input: &str, raw_input: &str) -> impl MockInterp
 }
 
 pub fn interpret_with_env(input: &str, stdlib: MockStdlib) -> impl MockInterpreterTrait {
-    let (instruction_generator_result, user_defined_types_holder) =
+    let (instruction_generator_result, user_defined_types) =
         generate_instructions_str_with_types(input);
     // println!("{:#?}", instructions);
     let mut interpreter =
-        mock_interpreter_for_user_defined_types_stdlib(user_defined_types_holder, stdlib);
+        mock_interpreter_for_user_defined_types_stdlib(user_defined_types, stdlib);
     interpreter
         .interpret(instruction_generator_result)
         .map(|_| interpreter)
@@ -127,8 +128,9 @@ pub fn interpret_file(filename: &str) -> Result<impl MockInterpreterTrait, Runti
     let file_path = format!("../fixtures/{}", filename);
     let f = File::open(file_path).expect("Could not read bas file");
     let program = parse_main_file(f).unwrap();
-    let (linted_program, user_defined_types) = lint(program).unwrap();
-    let instruction_generator_result = generate_instructions(linted_program);
+    let (linted_program, linter_context) = lint(program).unwrap();
+    let (linter_names, user_defined_types) = unwrap_linter_context(linter_context);
+    let instruction_generator_result = generate_instructions(linted_program, linter_names);
     let mut interpreter = mock_interpreter_for_user_defined_types(user_defined_types);
     interpreter
         .interpret(instruction_generator_result)
@@ -142,8 +144,9 @@ pub fn interpret_file_with_raw_input(
     let file_path = format!("../fixtures/{}", filename);
     let f = File::open(file_path).expect("Could not read bas file");
     let program = parse_main_file(f).unwrap();
-    let (linted_program, user_defined_types) = lint(program).unwrap();
-    let instruction_generator_result = generate_instructions(linted_program);
+    let (linted_program, linter_context) = lint(program).unwrap();
+    let (linter_names, user_defined_types) = unwrap_linter_context(linter_context);
+    let instruction_generator_result = generate_instructions(linted_program, linter_names);
     let mut interpreter = mock_interpreter_for_user_defined_types(user_defined_types);
     if !raw_input.is_empty() {
         interpreter.stdin().add_next_input(raw_input);

rusty_linter/src/converter/common/context.rs

@@ -45,6 +45,10 @@ impl Context {
         }
     }
 
+    pub fn unwrap(self) -> (PreLinterResult, TypeResolverImpl, Names) {
+        (self.pre_linter_result, self.resolver, self.names)
+    }
+
     pub fn is_in_subprogram(&self) -> bool {
         self.names.is_in_subprogram()
     }

rusty_linter/src/converter/expr_rules/variable.rs

@@ -206,7 +206,16 @@ impl VarResolve for AssignToFunction {
         if ctx.names.is_in_function(name.bare_name()) {
             let converted_name = try_qualify(name, function_qualifier).with_err_at(&extra.pos)?;
             let expr_type = ExpressionType::BuiltIn(function_qualifier);
-            let expr = Expression::Variable(converted_name, VariableInfo::new_local(expr_type));
+            let variable_info = VariableInfo::new_local(expr_type);
+
+            // store this in the name context too to make it easier for instruction generator
+            // TODO add unit test
+            // TODO what if the name already exists?
+            ctx.names
+                .insert_compact(converted_name.bare_name().clone(), variable_info.clone());
+
+            let expr = Expression::Variable(converted_name, variable_info);
+
             Ok(expr)
         } else {
             Err(LintError::DuplicateDefinition.at_pos(extra.pos))