Update variable resolution

src/libeval/environment.rs

@@ -7,8 +7,10 @@
 
 //! Lexical environments.
 
+use std::collections::HashMap;
 use std::rc::{Rc};
 
+use locus::diagnostics::Span;
 use reader::intern_pool::{Atom};
 
 use expression::{Variable};
@@ -19,7 +21,7 @@ use expression::{Variable};
 /// visibility scopes which are nested one into another.
 pub struct Environment {
     kind: EnvironmentKind,
-    variables: Vec<Variable>,
+    variables: HashMap<Atom, EnvironmentVariable>,
     parent: Option<Rc<Environment>>,
 }
 
@@ -29,8 +31,34 @@ enum EnvironmentKind {
     Imported,
 }
 
-/// Type of a variable.
-pub enum VariableKind {
+struct EnvironmentVariable {
+    /// Kind of a variable stored in the environment.
+    kind: VariableKind,
+    /// Definition site of the variable.
+    span: Span,
+}
+
+enum VariableKind {
+    /// Run-time variable requiring storage.
+    Runtime {
+        /// Index of the variable storage location.
+        ///
+        /// For local variables it's the stack frame, for global variables it's the global table,
+        /// for imported variables it's the import table.
+        index: usize,
+    },
+}
+
+/// Reference to a variable from environment.
+pub struct VariableReference {
+    /// Kind of the variable referenced.
+    pub kind: ReferenceKind,
+    /// Location of the variable definition.
+    pub span: Span,
+}
+
+/// Kind of a referenced variable.
+pub enum ReferenceKind {
     /// Locally-bound variable, defined by a procedure.
     ///
     /// Local variables are identified by their (zero-based) index in the activation record of
@@ -51,19 +79,14 @@ pub enum VariableKind {
     Imported {
         index: usize,
     },
-    /// An unresolved variable.
-    ///
-    /// No environment contains a definition of the requested variable. It is an error to use
-    /// such variables.
-    Unresolved,
 }
 
 impl Environment {
     /// Create a new local environment with specified variables.
     pub fn new_local(variables: &[Variable], parent: &Rc<Environment>) -> Rc<Environment> {
         Rc::new(Environment {
             kind: EnvironmentKind::Local,
-            variables: variables.to_vec(),
+            variables: enumerate_runtime_variables(variables),
             parent: Some(parent.clone()),
         })
     }
@@ -78,7 +101,7 @@ impl Environment {
         assert!(match parent.kind { EnvironmentKind::Imported => true, _ => false });
         Rc::new(Environment {
             kind: EnvironmentKind::Global,
-            variables: variables.to_vec(),
+            variables: enumerate_runtime_variables(variables),
             parent: Some(parent.clone()),
         })
     }
@@ -89,35 +112,54 @@ impl Environment {
     pub fn new_imported(variables: &[Variable]) -> Rc<Environment> {
         Rc::new(Environment {
             kind: EnvironmentKind::Imported,
-            variables: variables.to_vec(),
+            variables: enumerate_runtime_variables(variables),
             parent: None,
         })
     }
 
     /// Resolve a variable in this environment.
-    pub fn resolve_variable(&self, name: Atom) -> VariableKind {
+    ///
+    /// Returns Some variable from the environment or its parents, or None if the variable
+    /// could not be found in any environment.
+    pub fn resolve_variable(&self, name: Atom) -> Option<VariableReference> {
         // First, try to resolve the name locally.
-        for (index, local) in self.variables.iter().enumerate() {
-            if name == local.name {
-                return match self.kind {
-                    EnvironmentKind::Local => VariableKind::Local { index, depth: 0 },
-                    EnvironmentKind::Global => VariableKind::Global { index },
-                    EnvironmentKind::Imported => VariableKind::Imported { index },
-                };
-            }
+        if let Some(variable) = self.variables.get(&name) {
+            let kind = match variable.kind {
+                VariableKind::Runtime { index } => {
+                    match self.kind {
+                        EnvironmentKind::Local => ReferenceKind::Local { index, depth: 0 },
+                        EnvironmentKind::Global => ReferenceKind::Global { index },
+                        EnvironmentKind::Imported => ReferenceKind::Imported { index },
+                    }
+                }
+            };
+            return Some(VariableReference { kind, span: variable.span });
         }
 
         // If that fails then look into parent environment (if it's available).
         if let Some(ref parent) = self.parent {
-            let mut variable = parent.resolve_variable(name);
-            // Bump the nesting depth for local variables.
-            if let VariableKind::Local { ref mut depth, .. } = variable {
-                *depth += 1;
+            if let Some(mut variable) = parent.resolve_variable(name) {
+                // Bump the nesting depth for local variables.
+                if let ReferenceKind::Local { ref mut depth, .. } = variable.kind {
+                    *depth += 1;
+                }
+                return Some(variable);
             }
-            return variable;
         }
 
         // The variable cannot be resolved if it is absent in all environments.
-        return VariableKind::Unresolved;
+        return None;
     }
 }
+
+fn enumerate_runtime_variables(variables: &[Variable]) -> HashMap<Atom, EnvironmentVariable> {
+    variables.iter()
+        .enumerate()
+        .map(|(index, variable)| {
+            (variable.name, EnvironmentVariable {
+                kind: VariableKind::Runtime { index },
+                span: variable.span,
+            })
+        })
+        .collect()
+}

src/libeval/expanders/lambda.rs

@@ -79,7 +79,7 @@ fn expand_arguments(datum: Option<&ScannedDatum>, diagnostic: &Handler) -> Argum
         let raw_variables: Vec<Variable> = arguments.iter()
             .filter_map(|argument| {
                 if let DatumValue::Symbol(name) = argument.value {
-                    Some(Variable { name: name, span: Some(argument.span) })
+                    Some(Variable { name: name, span: argument.span })
                 } else {
                     diagnostic.report(DiagnosticKind::err_expand_invalid_lambda,
                         argument.span);
@@ -133,7 +133,7 @@ fn deduplicate_variables(raw_variables: Vec<Variable>, diagnostic: &Handler) ->
         for previous in &variables {
             if variable.name == previous.name {
                 diagnostic.report(DiagnosticKind::err_expand_invalid_lambda,
-                    variable.span.expect("all lambda args have spans"));
+                    variable.span);
 
                 continue 'next_variable;
             }

src/libeval/expanders/set.rs

@@ -73,7 +73,7 @@ fn expand_variable(datum: Option<&ScannedDatum>, diagnostic: &Handler) -> Option
         if let DatumValue::Symbol(name) = datum.value {
             return Some(Variable {
                 name: name,
-                span: Some(datum.span),
+                span: datum.span,
             });
         }
         diagnostic.report(DiagnosticKind::err_expand_invalid_set, datum.span);

src/libeval/expression.rs

@@ -83,8 +83,8 @@ pub struct Variable {
     /// Name of the variable.
     pub name: Atom,
 
-    /// Closest known source of the reference.
-    pub span: Option<Span>,
+    /// Definition of the variable.
+    pub span: Span,
 }
 
 /// Procedure argument list.

src/libeval/meaning.rs

@@ -19,7 +19,7 @@ use reader::datum::{ScannedDatum, DatumValue};
 use reader::intern_pool::{Atom};
 
 use expression::{Expression, ExpressionKind, Literal, Variable, Arguments};
-use environment::{Environment, VariableKind};
+use environment::{Environment, ReferenceKind};
 
 pub struct Meaning {
     pub kind: MeaningKind,
@@ -273,27 +273,28 @@ fn meaning_reference(
     name: Atom, span: Span,
     environment: &Rc<Environment>) -> MeaningKind
 {
-    match environment.resolve_variable(name)  {
-        VariableKind::Local { depth, index } => {
-            if depth == 0 {
-                MeaningKind::ShallowArgumentReference(index)
-            } else {
-                MeaningKind::DeepArgumentReference(depth, index)
+    if let Some(reference) = environment.resolve_variable(name) {
+        match reference.kind {
+            ReferenceKind::Local { depth, index } => {
+                if depth == 0 {
+                    MeaningKind::ShallowArgumentReference(index)
+                } else {
+                    MeaningKind::DeepArgumentReference(depth, index)
+                }
+            }
+            ReferenceKind::Global { index } => {
+                MeaningKind::GlobalReference(index)
+            }
+            ReferenceKind::Imported { index } => {
+                MeaningKind::ImportedReference(index)
             }
         }
-        VariableKind::Global { index } => {
-            MeaningKind::GlobalReference(index)
-        }
-        VariableKind::Imported { index } => {
-            MeaningKind::ImportedReference(index)
-        }
-        VariableKind::Unresolved => {
-            // TODO: provide suggestions based on the environment
-            diagnostic.report(DiagnosticKind::err_meaning_unresolved_variable, span);
+    } else {
+        // TODO: provide suggestions based on the environment
+        diagnostic.report(DiagnosticKind::err_meaning_unresolved_variable, span);
 
-            // We cannot return an actual value or reference here, so return a poisoned value.
-            MeaningKind::Undefined
-        }
+        // We cannot return an actual value or reference here, so return a poisoned value.
+        MeaningKind::Undefined
     }
 }
 
@@ -315,39 +316,47 @@ fn meaning_assignment(
     environment: &Rc<Environment>,
     constants: &mut Vec<Value>) -> MeaningKind
 {
-    let variable_kind = environment.resolve_variable(variable.name);
+    let reference = environment.resolve_variable(variable.name);
 
     // Report the errors before computing the meaning of the assigned value
     // so that the reported diagnostics are ordered better.
-    if let VariableKind::Unresolved = variable_kind {
+    if let Some(ref reference) = reference {
+        if let ReferenceKind::Imported { .. } = reference.kind {
+            // TODO: show where the variable is imported from
+            diagnostic.report(DiagnosticKind::err_meaning_assign_to_imported_binding,
+                variable.span);
+        }
+    } else {
         // TODO: provide suggestions based on the environment
         diagnostic.report(DiagnosticKind::err_meaning_unresolved_variable,
-            variable.span.expect("BUG: unresolved variable").clone());
-    }
-    if let VariableKind::Imported { .. } = variable_kind {
-        diagnostic.report(DiagnosticKind::err_meaning_assign_to_imported_binding,
-            variable.span.expect("BUG: unresolved variable").clone());
+            variable.span);
     }
 
     let new_value = Box::new(meaning_expression(diagnostic, value, constants));
 
-    match variable_kind {
-        VariableKind::Local { depth, index } => {
-            if depth == 0 {
-                MeaningKind::ShallowArgumentSet(index, new_value)
-            } else {
-                MeaningKind::DeepArgumentSet(depth, index, new_value)
+    if let Some(ref reference) = reference {
+        match reference.kind {
+            ReferenceKind::Local { depth, index } => {
+                if depth == 0 {
+                    MeaningKind::ShallowArgumentSet(index, new_value)
+                } else {
+                    MeaningKind::DeepArgumentSet(depth, index, new_value)
+                }
+            }
+            ReferenceKind::Global { index } => {
+                MeaningKind::GlobalSet(index, new_value)
+            }
+            // We really can't assign to the imported variable, so return the meaning of the new
+            // value being computed in order to allow further passes to analyze it if necessary
+            // (and see any side-effects and errors that the new value computation may contain).
+            ReferenceKind::Imported { .. } => {
+                new_value.kind
             }
         }
-        VariableKind::Global { index } => {
-            MeaningKind::GlobalSet(index, new_value)
-        }
-        // Well... in these cases return the meaning of the new value being computed
-        // in order to allow further passes to analyze it if necessary (and see any
-        // side-effects and errors that the new value computation may contain).
-        VariableKind::Imported { .. } | VariableKind::Unresolved => {
-            new_value.kind
-        }
+    } else {
+        // The same goes for assignments to unresolved variables. Instead of replacing them with
+        // MeaningKind::Undefined return the computation of the new value.
+        new_value.kind
     }
 }
 

src/libeval/tests/meaning.rs

@@ -41,12 +41,12 @@ fn standard_scheme(pool: &InternPool) -> Box<Expander> {
 
 fn basic_scheme_environment(pool: &InternPool) -> Rc<Environment> {
     let imported_vars = [
-        Variable { name: pool.intern("car"), span: None },
-        Variable { name: pool.intern("cdr"), span: None },
-        Variable { name: pool.intern("cons"), span: None },
+        Variable { name: pool.intern("car"), span: Span::new(0, 0) },
+        Variable { name: pool.intern("cdr"), span: Span::new(0, 0) },
+        Variable { name: pool.intern("cons"), span: Span::new(0, 0) },
     ];
     let global_vars = [
-        Variable { name: pool.intern("*global*"), span: None },
+        Variable { name: pool.intern("*global*"), span: Span::new(0, 0) },
     ];
 
     let imported_env = Environment::new_imported(&imported_vars);