Add loop-detection test for symbol table.

Author: youngman-g

assembler/src/asmlib/driver/tests.rs

@@ -7,7 +7,7 @@ use super::super::ast::{
 };
 use super::super::eval::{make_empty_rc_block_for_test, SymbolLookup, SymbolValue};
 use super::super::span::*;
-use super::super::symbol::{SymbolContext, SymbolName};
+use super::super::symbol::SymbolName;
 use super::super::symtab::LookupOperation;
 use super::{assemble_nonempty_valid_input, assemble_source};
 use super::{assemble_pass1, Binary, BinaryChunk};
@@ -29,12 +29,11 @@ fn assemble_check_symbols(input: &str, target_address: Address, expected: &[(&st
             canonical: name.to_string(),
         };
         let span = span(0..(name.len()));
-        let context = SymbolContext::from((Script::Normal, span));
         let here = HereValue::Address(target_address);
         let mut op = LookupOperation::default();
         let mut rc_block =
             make_empty_rc_block_for_test(Address::from(Unsigned18Bit::from(0o020_000u16)));
-        match symtab.lookup_with_op(&sym, span, &here, &mut rc_block, &context, &mut op) {
+        match symtab.lookup_with_op(&sym, span, &here, &mut rc_block, &mut op) {
             Ok(got) => {
                 if got != *expected_value {
                     panic!("incorrect value for symbol {name}; expected {expected_value:?}, got {got:?}");

assembler/src/asmlib/eval.rs

@@ -19,7 +19,7 @@ use super::ast::{
 };
 use super::listing::{Listing, ListingLine};
 use super::span::*;
-use super::symbol::{SymbolContext, SymbolName, SymbolOrHere};
+use super::symbol::{SymbolName, SymbolOrHere};
 use super::types::{AssemblerFailure, BlockIdentifier, MachineLimitExceededFailure};
 
 #[derive(Debug, PartialEq, Eq)]
@@ -144,7 +144,6 @@ pub(crate) trait SymbolLookup {
         span: Span, // TODO: use &Span?
         target_address: &HereValue,
         rc_allocator: &mut R,
-        context: &SymbolContext,
         op: &mut Self::Operation<'_>,
     ) -> Result<SymbolValue, SymbolLookupFailure>;
 }
@@ -419,8 +418,7 @@ fn symbol_name_lookup<R: RcAllocator>(
     rc_allocator: &mut R,
     op: &mut LookupOperation,
 ) -> Result<Unsigned36Bit, SymbolLookupFailure> {
-    let context = SymbolContext::from((elevation, span));
-    match symtab.lookup_with_op(name, span, target_address, rc_allocator, &context, op) {
+    match symtab.lookup_with_op(name, span, target_address, rc_allocator, op) {
         Ok(SymbolValue::Final(value)) => Ok(value),
         Err(e) => Err(e),
     }

assembler/src/asmlib/symtab.rs

@@ -76,6 +76,64 @@ pub(crate) struct LookupOperation {
     deps_in_order: Vec<SymbolName>,
 }
 
+fn final_lookup_helper_body<R: RcAllocator>(
+    t: &mut SymbolTable,
+    name: &SymbolName,
+    span: Span,
+    target_address: &HereValue,
+    rc_allocator: &mut R,
+    op: &mut LookupOperation,
+) -> Result<SymbolValue, SymbolLookupFailure> {
+    if let Some(def) = t.get_clone(name) {
+        match def {
+            SymbolDefinition::DefaultAssigned(value, _) => Ok(SymbolValue::Final(value)),
+            SymbolDefinition::Origin(addr) => Ok(SymbolValue::Final(addr.into())),
+            SymbolDefinition::Tag {
+                block_id,
+                block_offset,
+                span: _,
+            } => match t.finalise_origin(block_id, rc_allocator, Some(op)) {
+                Ok(address) => {
+                    let computed_address: Address = address.index_by(block_offset);
+                    Ok(SymbolValue::Final(computed_address.into()))
+                }
+                Err(e) => {
+                    panic!("failed to finalise origin of {block_id}: {e}");
+                }
+            },
+            SymbolDefinition::TagOverride(address) => Ok(SymbolValue::Final(address.into())),
+            SymbolDefinition::Equality(expression) => {
+                // The target address does not matter below
+                // since assignments are not allowed to use
+                // '#' on the right-hand-side of the
+                // assignment.
+                expression
+                    .evaluate(target_address, t, rc_allocator, op)
+                    .map(SymbolValue::Final)
+            }
+            SymbolDefinition::Undefined(context_union) => {
+                match t.get_default_value(name, &span, &context_union, rc_allocator) {
+                    Ok(value) => {
+                        match t.define(
+                            span,
+                            name.clone(),
+                            SymbolDefinition::DefaultAssigned(value, context_union),
+                        ) {
+                            Err(e) => {
+                                panic!("got a bad symbol definition error ({e}) on a previously undefined symbol, which should be impossible");
+                            }
+                            Ok(()) => Ok(SymbolValue::Final(value)),
+                        }
+                    }
+                    Err(e) => Err(e),
+                }
+            }
+        }
+    } else {
+        panic!("final symbol lookup of symbol '{name}' happened in an evaluation context which was not previously returned by SourceFile::global_symbol_references(): {op:?}");
+    }
+}
+
 impl SymbolTable {
     pub(crate) fn new<I>(block_sizes: I) -> SymbolTable
     where
@@ -159,93 +217,6 @@ impl SymbolTable {
             });
     }
 
-    fn final_lookup_helper<R: RcAllocator>(
-        &mut self,
-        name: &SymbolName,
-        span: Span,
-        target_address: &HereValue,
-        rc_allocator: &mut R,
-        _context: &SymbolContext,
-        op: &mut LookupOperation,
-    ) -> Result<SymbolValue, SymbolLookupFailure> {
-        fn body<R: RcAllocator>(
-            t: &mut SymbolTable,
-            name: &SymbolName,
-            span: Span,
-            target_address: &HereValue,
-            rc_allocator: &mut R,
-            op: &mut LookupOperation,
-        ) -> Result<SymbolValue, SymbolLookupFailure> {
-            if let Some(def) = t.get_clone(name) {
-                match def {
-                    SymbolDefinition::DefaultAssigned(value, _) => Ok(SymbolValue::Final(value)),
-                    SymbolDefinition::Origin(addr) => Ok(SymbolValue::Final(addr.into())),
-                    SymbolDefinition::Tag {
-                        block_id,
-                        block_offset,
-                        span: _,
-                    } => match t.finalise_origin(block_id, rc_allocator, Some(op)) {
-                        Ok(address) => {
-                            let computed_address: Address = address.index_by(block_offset);
-                            Ok(SymbolValue::Final(computed_address.into()))
-                        }
-                        Err(e) => {
-                            panic!("failed to finalise origin of {block_id}: {e}");
-                        }
-                    },
-                    SymbolDefinition::TagOverride(address) => {
-                        Ok(SymbolValue::Final(address.into()))
-                    }
-                    SymbolDefinition::Equality(expression) => {
-                        // The target address does not matter below
-                        // since assignments are not allowed to use
-                        // '#' on the right-hand-side of the
-                        // assignment.
-                        expression
-                            .evaluate(target_address, t, rc_allocator, op)
-                            .map(SymbolValue::Final)
-                    }
-                    SymbolDefinition::Undefined(context_union) => {
-                        match t.get_default_value(name, &span, &context_union, rc_allocator) {
-                            Ok(value) => {
-                                match t.define(
-                                    span,
-                                    name.clone(),
-                                    SymbolDefinition::DefaultAssigned(value, context_union),
-                                ) {
-                                    Err(e) => {
-                                        panic!("got a bad symbol definition error ({e}) on a previously undefined symbol, which should be impossible");
-                                    }
-                                    Ok(()) => Ok(SymbolValue::Final(value)),
-                                }
-                            }
-                            Err(e) => Err(e),
-                        }
-                    }
-                }
-            } else {
-                panic!("final symbol lookup of symbol '{name}' happened in an evaluation context which was not previously returned by SourceFile::global_symbol_references(): {op:?}");
-            }
-        }
-
-        op.deps_in_order.push(name.clone());
-        if !op.depends_on.insert(name.clone()) {
-            // `name` was already in `depends_on`; in other words,
-            // we have a loop.
-            return Err(SymbolLookupFailure::from((
-                name.clone(),
-                span,
-                SymbolLookupFailureKind::Loop {
-                    deps_in_order: op.deps_in_order.to_vec(),
-                },
-            )));
-        }
-        let result = body(self, name, span, target_address, rc_allocator, op);
-        op.deps_in_order.pop();
-        op.depends_on.remove(name);
-        result
-    }
-
     pub(crate) fn finalise_origin<R: RcAllocator>(
         &mut self,
         block_id: BlockIdentifier,
@@ -498,10 +469,24 @@ impl SymbolLookup for SymbolTable {
         span: Span,
         target_address: &HereValue,
         rc_allocator: &mut R,
-        context: &SymbolContext,
         op: &mut Self::Operation<'_>,
     ) -> Result<SymbolValue, SymbolLookupFailure> {
-        self.final_lookup_helper(name, span, target_address, rc_allocator, context, op)
+        op.deps_in_order.push(name.clone());
+        if !op.depends_on.insert(name.clone()) {
+            // `name` was already in `depends_on`; in other words,
+            // we have a loop.
+            return Err(SymbolLookupFailure::from((
+                name.clone(),
+                span,
+                SymbolLookupFailureKind::Loop {
+                    deps_in_order: op.deps_in_order.to_vec(),
+                },
+            )));
+        }
+        let result = final_lookup_helper_body(self, name, span, target_address, rc_allocator, op);
+        op.deps_in_order.pop();
+        op.depends_on.remove(name);
+        result
     }
 }
 
@@ -723,3 +708,70 @@ impl SymbolDefinition {
         }
     }
 }
+
+#[test]
+fn test_loop_detection() {
+    use super::ast::*;
+    use super::eval::RcBlock;
+    use base::charset::Script;
+    let blocks = [(span(0..10), None, u18!(2))];
+    let mut symtab = SymbolTable::new(blocks);
+
+    // define "A"
+    symtab
+        .define(
+            span(0..1),
+            SymbolName::from("A"),
+            SymbolDefinition::Equality(EqualityValue::from(vec![CommaDelimitedInstruction {
+                leading_commas: None,
+                instruction: UntaggedProgramInstruction {
+                    span: span(0..1),
+                    holdbit: HoldBit::Unspecified,
+                    inst: InstructionFragment::from(ArithmeticExpression::from(
+                        SymbolOrLiteral::Symbol(Script::Normal, SymbolName::from("B"), span(2..3)),
+                    )),
+                },
+                trailing_commas: None,
+            }])),
+        )
+        .expect("definition of A should be correct");
+    // define "B"
+    symtab
+        .define(
+            span(4..5),
+            SymbolName::from("B"),
+            SymbolDefinition::Equality(EqualityValue::from(vec![CommaDelimitedInstruction {
+                leading_commas: None,
+                instruction: UntaggedProgramInstruction {
+                    span: span(4..5),
+                    holdbit: HoldBit::Unspecified,
+                    inst: InstructionFragment::from(ArithmeticExpression::from(
+                        SymbolOrLiteral::Symbol(Script::Normal, SymbolName::from("A"), span(6..7)),
+                    )),
+                },
+                trailing_commas: None,
+            }])),
+        )
+        .expect("definition of B should be correct");
+
+    let mut rc_block = RcBlock::default();
+
+    // An attempt to look up A should fail because we detect a loop.
+    let mut op = Default::default();
+    let r = symtab.lookup_with_op(
+        &SymbolName::from("A"),
+        span(0..1),
+        &HereValue::NotAllowed,
+        &mut rc_block,
+        &mut op,
+    );
+    dbg!(&r);
+    assert!(matches!(
+        r,
+        Err(SymbolLookupFailure {
+            target: _,
+            kind: SymbolLookupFailureKind::Loop { .. }
+        })
+    ));
+    assert!(false);
+}