Centralize bytecode expression emission

src/fpy/codegen_fpybc.py

@@ -312,7 +312,7 @@ class EmitterWithNodeInfo(Emitter):
     errors raised on a directive can point at a source line."""
 
     def emit(self, node: Ast, state: CompileState) -> list[Directive | Ir]:
-        dirs = super().emit(node, state)
+        dirs = self._emit(node, state)
         # Nested emit calls run first, so an existing stamp is the more
         # specific node. (Ir instances are frozen and are skipped; the
         # directives they become carry no arguments worth locating.)
@@ -321,6 +321,9 @@ def emit(self, node: Ast, state: CompileState) -> list[Directive | Ir]:
                 dir.source_node = node
         return dirs
 
+    def _emit(self, node: Ast, state: CompileState) -> list[Directive | Ir]:
+        return super().emit(node, state)
+
 
 class GenerateFunctionBody(EmitterWithNodeInfo):
     # Flag indicating we're generating code inside a function body.
@@ -329,6 +332,25 @@ class GenerateFunctionBody(EmitterWithNodeInfo):
     # reached by absolute offset (LOAD_ABS/STORE_ABS).
     in_function = True
 
+    def _emit(self, node: Ast, state: CompileState) -> list[Directive | Ir]:
+        """Emit expressions at their contextual type before stamping directives.
+
+        Expression handlers produce the synthesized type. Constants already
+        have their contextual type and need no runtime conversion.
+        """
+        if is_instance_compat(node, AstExpr):
+            const_dirs = self.try_emit_expr_as_const(node, state)
+            if const_dirs is not None:
+                return const_dirs
+
+        dirs = super()._emit(node, state)
+        if is_instance_compat(node, AstExpr):
+            synthesized = state.synthesized_types[node]
+            contextual = state.contextual_types[node]
+            if synthesized != contextual:
+                dirs.extend(self.convert_numeric_type(synthesized, contextual))
+        return dirs
+
     def _emit_func_arg(self, arg, state: CompileState) -> list[Directive | Ir]:
         """Emit code to push a function argument onto the stack.
 
@@ -867,9 +889,6 @@ def emit_AstFor(self, node: AstFor, state: CompileState):
         assert False, node
 
     def emit_AstIndexExpr(self, node: AstIndexExpr, state: CompileState):
-        const_dirs = self.try_emit_expr_as_const(node, state)
-        if const_dirs is not None:
-            return const_dirs
         sym = state.resolved_symbols[node]
 
         assert is_instance_compat(sym, FieldAccess), sym
@@ -906,18 +925,9 @@ def emit_AstIndexExpr(self, node: AstIndexExpr, state: CompileState):
             GetFieldDirective(parent_type.max_size, parent_type.elem_type.max_size)
         )
 
-        # now convert the type if necessary
-        converted_type = state.contextual_types[node]
-        if unconverted_type != converted_type:
-            dirs.extend(self.convert_numeric_type(unconverted_type, converted_type))
-
         return dirs
 
     def emit_AstIdent(self, node: AstIdent, state: CompileState):
-        const_dirs = self.try_emit_expr_as_const(node, state)
-        if const_dirs is not None:
-            return const_dirs
-
         sym = state.resolved_symbols.get(node)
 
         assert is_instance_compat(sym, VariableSymbol), sym
@@ -932,18 +942,9 @@ def emit_AstIdent(self, node: AstIdent, state: CompileState):
         else:
             dirs = [LoadRelDirective(offset, sym.type.max_size)]
 
-        unconverted_type = state.synthesized_types[node]
-        converted_type = state.contextual_types[node]
-        if unconverted_type != converted_type:
-            dirs.extend(self.convert_numeric_type(unconverted_type, converted_type))
-
         return dirs
 
     def emit_AstGetAttr(self, node: AstGetAttr, state: CompileState):
-        const_dirs = self.try_emit_expr_as_const(node, state)
-        if const_dirs is not None:
-            return const_dirs
-
         sym = state.resolved_symbols.get(node)
 
         if is_instance_compat(sym, dict):
@@ -980,17 +981,9 @@ def emit_AstGetAttr(self, node: AstGetAttr, state: CompileState):
                 False
             ), sym  # sym should either be impossible to put on stack or should have a compile time val
 
-        converted_type = state.contextual_types[node]
-        if converted_type != unconverted_type:
-            dirs.extend(self.convert_numeric_type(unconverted_type, converted_type))
-
         return dirs
 
     def emit_AstOp(self, node: AstOp, state: CompileState):
-        const_dirs = self.try_emit_expr_as_const(node, state)
-        if const_dirs is not None:
-            return const_dirs
-
         dirs = FPYBC_OP_IMPLS[state.op_cases[node]](self, node, state)
 
         # The VM operates on 64-bit values, so after the op we have a 64-bit result.
@@ -1003,18 +996,9 @@ def emit_AstOp(self, node: AstOp, state: CompileState):
         ):
             dirs.extend(self.convert_numeric_type(intermediate_type, synthesized_type))
 
-        # and convert the result of the op into the desired result of this expr
-        converted_type = state.contextual_types[node]
-        if synthesized_type != converted_type:
-            dirs.extend(self.convert_numeric_type(synthesized_type, converted_type))
-
         return dirs
 
     def emit_AstFuncCall(self, node: AstFuncCall, state: CompileState):
-        const_dirs = self.try_emit_expr_as_const(node, state)
-        if const_dirs is not None:
-            return const_dirs
-
         node_args = node.args if node.args is not None else []
         func = state.resolved_symbols[node.func]
         dirs = []
@@ -1090,8 +1074,7 @@ def emit_AstFuncCall(self, node: AstFuncCall, state: CompileState):
             for arg_node in node_args:
                 dirs.extend(self._emit_func_arg(arg_node, state))
         elif is_instance_compat(func, CastSymbol):
-            # just putting the arg value on the stack should be good enough, the
-            # conversion will happen below
+            # Semantics coerces the argument to the explicit cast's target.
             dirs.extend(self.emit(node_args[0], state))
         elif is_instance_compat(func, FunctionSymbol):
             # script-defined function
@@ -1107,12 +1090,6 @@ def emit_AstFuncCall(self, node: AstFuncCall, state: CompileState):
         else:
             assert False, func
 
-        # perform type conversion if called for
-        unconverted_type = state.synthesized_types[node]
-        converted_type = state.contextual_types[node]
-        if unconverted_type != converted_type:
-            dirs.extend(self.convert_numeric_type(unconverted_type, converted_type))
-
         return dirs
 
     def _compute_field_access_offset(
@@ -1233,9 +1210,7 @@ def emit_AstAssign(self, node: AstAssign, state: CompileState):
         return dirs
 
     def emit_AstLiteral(self, node: AstLiteral, state: CompileState):
-        const_dirs = self.try_emit_expr_as_const(node, state)
-        assert const_dirs is not None
-        return const_dirs
+        assert False, "literals must have a compile-time constant value"
 
     def emit_AstAssert(self, node: AstAssert, state: CompileState):
         dirs = self.emit(node.condition, state)

test/fpy/test_codegen_fpybc.py

@@ -0,0 +1,36 @@
+from pathlib import Path
+
+from fpy.bytecode.directives import (
+    IntegerZeroExtend32To64Directive,
+    PushValDirective,
+    UnsignedIntToFloatDirective,
+)
+from fpy.compiler import analyze_ast, analysis_to_fpybc_directives, text_to_ast
+from fpy.state import get_base_compile_state
+
+
+def test_expression_directives_keep_their_source_nodes():
+    dictionary = Path(__file__).with_name("RefTopologyDictionary.json")
+    state = analyze_ast(
+        text_to_ast("x: U32 = 1\ny: F64 = x + 1\nz: F64 = 2 + 2\n"),
+        get_base_compile_state(str(dictionary)),
+    )
+    expr = state.main_block.stmts[1].rhs
+    folded_expr = state.main_block.stmts[2].rhs
+    directives, _ = analysis_to_fpybc_directives(state)
+
+    # Nested operand coercion and outer result coercion must each retain
+    # the expression responsible for the instruction's diagnostic location.
+    extensions = [
+        d for d in directives if isinstance(d, IntegerZeroExtend32To64Directive)
+    ]
+    conversions = [d for d in directives if isinstance(d, UnsignedIntToFloatDirective)]
+    assert len(extensions) == len(conversions) == 1
+    assert extensions[0].source_node is expr.lhs
+    assert conversions[0].source_node is expr
+
+    # A folded expression emits one push at its already-coerced type.
+    folded = [d for d in directives if d.source_node is folded_expr]
+    assert len(folded) == 1
+    assert isinstance(folded[0], PushValDirective)
+    assert folded[0].val == state.const_expr_values[folded_expr].serialize()