Adr 20261007 grammar fixes

src/mccode_antlr/instr/flow.py

@@ -315,12 +315,23 @@ def _build_flow_edge_records(components: tuple) -> tuple:
                 edge=GroupEdge(group_name=group_name, kind=GroupEdgeKind.PASS_THROUGH),
             ))
 
-    # Jump edges — resolve target by name if absolute_target is unset (-1)
+    records.extend(_jump_edge_records(components))
+    return tuple(records)
+
+
+def _jump_edge_records(components: tuple) -> tuple:
+    """JUMP edges, resolved as the translator does when ``absolute_target`` is unset (-1):
+    PREVIOUS[(n)], NEXT[(n)] and MYSELF relative to the jumping instance, anything else by name.
+    Targets outside the instrument give no edge."""
+    n = len(components)
     name_to_idx = {inst.name: idx for idx, inst in enumerate(components)}
-    for inst in components:
+    records = []
+    for index, inst in enumerate(components):
         for jmp in inst.jump:
             target_idx = jmp.absolute_target
-            if target_idx < 0:
+            if target_idx < 0 and (jmp.relative_target != 0 or jmp.target.lower() == 'myself'):
+                target_idx = index + jmp.relative_target
+            elif target_idx < 0:
                 target_idx = name_to_idx.get(jmp.target, -1)
             if 0 <= target_idx < n:
                 records.append(FlowEdgeRecord(
@@ -331,7 +342,6 @@ def _build_flow_edge_records(components: tuple) -> tuple:
                         absolute_target=target_idx,
                     ),
                 ))
-
     return tuple(records)
 
 

src/mccode_antlr/instr/instr.py

@@ -309,24 +309,8 @@ def _add_sequential_or_group_edge(self, prev: Instance, curr: Instance) -> None:
 
     def finalize_flow_edges(self) -> None:
         """Add JUMP edges to ``flow_edges`` (deferred because forward targets are unknown during parsing)."""
-        from .flow import FlowEdgeRecord, JumpEdge
-        components = self.components
-        n = len(components)
-        name_to_idx = {inst.name: idx for idx, inst in enumerate(components)}
-        for inst in components:
-            for jmp in inst.jump:
-                target_idx = jmp.absolute_target
-                if target_idx < 0:
-                    target_idx = name_to_idx.get(jmp.target, -1)
-                if 0 <= target_idx < n:
-                    self.flow_edges += (FlowEdgeRecord(
-                        src=inst.name, dst=components[target_idx].name,
-                        edge=JumpEdge(
-                            condition=jmp.condition,
-                            iterate=jmp.iterate,
-                            absolute_target=target_idx,
-                        ),
-                    ),)
+        from .flow import _jump_edge_records
+        self.flow_edges += _jump_edge_records(self.components)
 
     def build_flow_graph(self):
         """Rebuild ``flow_edges`` from scratch and return the derived :class:`networkx.MultiDiGraph`.

src/mccode_antlr/instr/visitor.py

@@ -11,6 +11,12 @@
 logger = logging.getLogger(__name__)
 
 
+def raise_myself_error(filename, ctx):
+    line = None if ctx.start is None else ctx.start.line
+    raise RuntimeError(f'{filename}: {line} -- MYSELF can not be used here, it is only available '
+                       f'after the component parameters (WHEN, AT, ROTATED, JUMP, ...)')
+
+
 def literal_string(ctx):
     start_token, stop_token = ctx.start, ctx.stop
     stream = start_token.getInputStream()
@@ -113,7 +119,11 @@ def visitComponent_instance(self, ctx: McInstrParser.Component_instanceContext):
         # Construct a new instance, possibly copying values from an existing instance:
         instance = Instance.from_instance(name, comp, at, rotate) if is_ref else Instance(name, comp, at, rotate)
         if ctx.instance_parameters() is not None:
-            for param_name, param_value in self.visit(ctx.instance_parameters()):
+            # MYSELF is not valid in the instance's own parameters, only from WHEN onwards
+            self.current_instance_name = None
+            parameters = self.visit(ctx.instance_parameters())
+            self.current_instance_name = name
+            for param_name, param_value in parameters:
                 instance.set_parameter(param_name, param_value, overwrite=is_ref)
         if ctx.Removable() is not None:
             instance.REMOVABLE()
@@ -153,7 +163,12 @@ def visitInstanceNameIdentifier(self, ctx: McInstrParser.InstanceNameIdentifierC
         return str(ctx.Identifier())
 
     def visitComponentTypeCopy(self, ctx: McInstrParser.ComponentTypeCopyContext):
-        return self.visit(ctx.component_ref())
+        source = self.visit(ctx.component_ref())
+        if source is None:
+            line = None if ctx.start is None else ctx.start.line
+            raise RuntimeError(f'{self.filename}: {line} -- COPY of an undefined component instance '
+                               f'{literal_string(ctx.component_ref())}')
+        return source
 
     def visitComponentTypeIdentifier(self, ctx: McInstrParser.ComponentTypeIdentifierContext):
         return self.parent.get_component(str(ctx.Identifier()), current_instance_name=self.current_instance_name)
@@ -225,14 +240,14 @@ def visitJump(self, ctx: McInstrParser.JumpContext):
 
     def visitJumpPrevious(self, ctx: McInstrParser.JumpPreviousContext):
         i = ctx.IntegerLiteral()
-        return ("PREVIOUS", -1) if i is None else (f"PREVIOUS_{i}", -int(i))
+        return ("PREVIOUS", -1) if i is None else (f"PREVIOUS_{i}", -int(str(i)))
 
     def visitJumpMyself(self, ctx: McInstrParser.JumpMyselfContext):
         return "MYSELF", 0
 
     def visitJumpNext(self, ctx: McInstrParser.JumpNextContext):
         i = ctx.IntegerLiteral()
-        return ("NEXT", 1) if i is None else (f"NEXT_{i}", int(i))
+        return ("NEXT", 1) if i is None else (f"NEXT_{i}", int(str(i)))
 
     def visitJumpIdentifier(self, ctx: McInstrParser.JumpIdentifierContext):
         return str(ctx.Identifier()), 0
@@ -248,6 +263,7 @@ def visitComponent_ref(self, ctx: McInstrParser.Component_refContext):
                     return state.last_component(remaining, removable_ok=True)
                 remaining -= len(state.components)
             logger.error(f'Too large PREVIOUS count {count} for the reachable component instances')
+            return None
         name = str(ctx.Identifier())
         for state in (self.state, *self._enclosing_states()):
             if any(inst.name == name for inst in state.components):
@@ -266,7 +282,15 @@ def visitCoords(self, ctx: McInstrParser.CoordsContext):
 
     def visitReference(self, ctx: McInstrParser.ReferenceContext):
         # ABSOLUTE or RELATIVE ABSOLUTE -> None
-        return self.visit(ctx.component_ref()) if ctx.Absolute() is None else None
+        if ctx.Absolute() is not None:
+            return None
+        relative = self.visit(ctx.component_ref())
+        # An unreachable PREVIOUS falls back to ABSOLUTE, as in the classic code generator
+        if relative is None and ctx.component_ref().Previous() is None:
+            line = None if ctx.start is None else ctx.start.line
+            raise RuntimeError(f'{self.filename}: {line} -- reference to undefined component instance '
+                               f'{literal_string(ctx.component_ref())}')
+        return relative
 
     def visitDependency(self, ctx: McInstrParser.DependencyContext):
         # store the flag without its surrounding quotes
@@ -338,7 +362,9 @@ def visitExpressionPrevious(self, ctx: McInstrParser.ExpressionPreviousContext):
 
     def visitExpressionMyself(self, ctx: McInstrParser.ExpressionMyselfContext):
         # The even-worse expression use of MYSELF to refer to the current being-constructed component's name
-        return Expr.string(self.current_instance.name)
+        if self.current_instance_name is None:
+            raise_myself_error(self.filename, ctx)
+        return Expr.string(self.current_instance_name)
 
     def multi_block(self, part: str, ctx: McInstrParser.Multi_blockContext):
         """Common visitor for {part} unparsed_block? ((INHERIT identifier)|(EXTEND unparsed_block))*
@@ -407,8 +433,7 @@ def visitExpressionPrevious(self, ctx: McInstrParser.ExpressionPreviousContext):
         raise RuntimeError('PREVIOUS keyword used in expression before any components defined')
 
     def visitExpressionMyself(self, ctx: McInstrParser.ExpressionMyselfContext):
-        # The even-worse expression use of MYSELF to refer to the current being-constructed component's name
-        return Expr.string(self.current_instance.name)
+        raise_myself_error('instrument parameters', ctx)
 
 
 

src/mccode_antlr/translators/c_decls.py

@@ -119,10 +119,15 @@ def component_share_declarations():
             "/* ************************************************************************** */",
             "/*             SHARE user declarations for all components                     */",
             "/* ************************************************************************** */"]
+        emitted = set()  # a class INHERITing another holds the very same SHARE blocks
         for comp in sharers:
+            blocks = [share for share in comp.share if share not in emitted]
+            if not blocks:
+                continue
+            emitted.update(blocks)
             lines.append(f"/* Shared user declarations for all components types '{comp.name}'. */")
             # TODO FIXME to_c includes the `#line` preprocessor directive -- which was removed from McCode3??
-            lines.extend([share.to_c(line_directives) for share in comp.share])
+            lines.extend([share.to_c(line_directives) for share in blocks])
         lines.extend([
             "/* ************************************************************************** */",
             "/*             End of SHARE user declarations for all components              */",

src/mccode_antlr/translators/c_header.py

@@ -1,3 +1,4 @@
+import re
 import logging
 from textwrap import dedent
 from .c_listener import CDeclarator
@@ -7,6 +8,19 @@
 logger = logging.getLogger(__name__)
 
 
+# C arithmetic type words; a typedef of one (other than these) is not recognised
+_NUMERIC_WORDS = {'char', 'short', 'int', 'long', 'float', 'double', 'signed', 'unsigned',
+                  '_Bool', 'bool', 'const', 'volatile', 'MCNUM', 'size_t'}
+
+
+def is_numeric_scalar(declaration: CDeclarator) -> bool:
+    """Whether a USERVAR can be read as a double, by particle_getvar and particle_getuservar_byid"""
+    if declaration.is_pointer or declaration.is_array or not declaration.dtype:
+        return False
+    return all(w in _NUMERIC_WORDS or re.fullmatch(r'u?int(_least|_fast)?\d+_t', w)
+               for w in declaration.dtype.split())
+
+
 def header_pre_runtime(
         source: Instr,
         flavor: Flavor,
@@ -56,9 +70,10 @@ def jump_line(instance, jump):
     uservar_string += '\n'.join([f'  {x};' for x in uservars])
 
     members = list(accessible_struct_members(flavor)) + [x.name for x in uservars]
-    # Shouldn't we exclude array-valued names here?
+    # Only numeric scalars can be returned as a double; for the rest *suc reports failure
+    readable = list(accessible_struct_members(flavor)) + [x.name for x in uservars if is_numeric_scalar(x)]
     getvar = '\n'.join(
-        [f'  if(!str_comp("{x}",name)){{rval=*((double*)(&(p->{x})));s=0;}}' for x in members]
+        [f'  if(!str_comp("{x}",name)){{rval=(double)(p->{x});s=0;}}' for x in readable]
     )
 
     # Array valued names seem OK here, since a user can type-cast correctly
@@ -84,8 +99,10 @@ def jump_line(instance, jump):
         [f'  p->{x} = p0->{x};' for x in restorable_struct_members(flavor)]
     )
 
+    # ids count all user vars, readable or not
     getuservar_byid = '\n'.join(
-        [f'  case {i}: {{rval=*( (double *)(&(p->{x.name})) ); s=0; break;}}' for i, x in enumerate(uservars)])
+        [f'  case {i}: {{rval=(double)(p->{x.name}); s=0; break;}}'
+         for i, x in enumerate(uservars) if is_numeric_scalar(x)])
 
     uservar_init = ''
     for x in uservars:

src/mccode_antlr/translators/c_initialise.py

@@ -144,8 +144,9 @@ def parameter_line(default):
                 # hack-in an allocator for the fixed-length allocated array
                 c_type = par.value.mccode_c_type.translate(str.maketrans('', '', ' *'))  # strip the trailing ' *'
                 pl.append(f'  {fullname} = calloc(sizeof({c_type}), {len(p.value.value)});')
-                for i, v in enumerate(p.value.value):
-                    pl.append(f'  {fullname}[{i}] = {v};')
+                # print each element as an expression, so instrument parameters get their prefix
+                for i, e in enumerate(p.value._exprs):
+                    pl.append(f'  {fullname}[{i}] = {Expr(e):p};')
             elif p.value.is_id:
                 pl.append(f'  {fullname} = {value};')
             else:

src/mccode_antlr/translators/target.py

@@ -360,17 +360,22 @@ def can_skip(comp):
         self.ok_to_skip = can_skip_transform
 
     def set_jump_absolute_targets(self):
+        names = [c.name for c in self.source.components]
         for index, jump in [(i, j) for i, c in enumerate(self.source.components) for j in c.jump]:
             # jump is a dataclass with 'target', 'index', 'iterate', 'condition', and 'actual_target_index'
             if jump.absolute_target > -1:
                 # Somewhere/something already set the actual target index -- we don't have a choice but to trust it
                 continue
             if jump.relative_target == 0 and jump.target.lower() != 'myself':
                 # the target is another named component:
-                jump.absolute_target = [i for i, c in enumerate(self.source.components) if jump.target == c.name][0]
-                jump.relative_target = jump.absolute_target - index
+                target = names.index(jump.target) if jump.target in names else -1
             else:
-                jump.absolute_target = index + jump.relative_target
+                target = index + jump.relative_target
+            if not 0 <= target < len(names):
+                raise RuntimeError(f'JUMP at component {names[index]}: target {jump.target} '
+                                   f'is not a component of this instrument')
+            jump.absolute_target = target
+            jump.relative_target = target - index
 
     def enter_trace(self):
         pass

tests/instr/test_flow.py

@@ -179,6 +179,38 @@ def test_jump_iterate_edge(self):
         self.assertEqual(len(jump_edges), 1)
         self.assertTrue(jump_edges[0].iterate)
 
+    def test_relative_jump_edges(self):
+        instr = self._parse("""
+            COMPONENT a = Arm() AT (0,0,0) ABSOLUTE
+            COMPONENT b = Arm() AT (0,0,1) RELATIVE a
+            COMPONENT c = Arm() AT (0,0,2) RELATIVE a JUMP PREVIOUS(2) ITERATE (3) JUMP NEXT(2) WHEN (1)
+            COMPONENT d = Arm() AT (0,0,3) RELATIVE a JUMP MYSELF WHEN (0) JUMP PREVIOUS WHEN (0)
+            COMPONENT e = Arm() AT (0,0,4) RELATIVE a
+        """)
+        for G in (build_particle_flow_graph(instr), instr.build_flow_graph()):
+            for u, v, target in (('c', 'a', 0), ('c', 'e', 4), ('d', 'd', 3), ('d', 'c', 2)):
+                jump_edges = [e for e in _flows(G, u, v) if isinstance(e, JumpEdge)]
+                self.assertEqual(len(jump_edges), 1, f'{u} -> {v}')
+                self.assertEqual(jump_edges[0].absolute_target, target)
+
+    def test_parsed_relative_jump_edges(self):
+        # finalize_flow_edges runs at the end of parsing
+        instr = self._parse("""
+            COMPONENT a = Arm() AT (0,0,0) ABSOLUTE
+            COMPONENT b = Arm() AT (0,0,1) RELATIVE a JUMP NEXT WHEN (1)
+            COMPONENT c = Arm() AT (0,0,2) RELATIVE a JUMP PREVIOUS(2) WHEN (1)
+        """)
+        jumps = {(r.src, r.dst) for r in instr.flow_edges if isinstance(r.edge, JumpEdge)}
+        self.assertEqual(jumps, {('b', 'c'), ('c', 'a')})
+
+    def test_out_of_range_jump_has_no_edge(self):
+        instr = self._parse("""
+            COMPONENT a = Arm() AT (0,0,0) ABSOLUTE
+            COMPONENT b = Arm() AT (0,0,1) RELATIVE a JUMP NEXT(2) WHEN (1)
+        """)
+        G = build_particle_flow_graph(instr)
+        self.assertFalse(any(isinstance(d['flow'], JumpEdge) for _, _, d in G.edges(data=True)))
+
     # ------------------------------------------------------------------
     # Serialisation round-trip
     # ------------------------------------------------------------------