fix!(codegen): initialize function outputs and omitted arguments (1.0.x)

book/src/pous.md

@@ -58,6 +58,11 @@ An In/Out parameter must always be passed in a POU call and cannot be stored.
 
 Output parameters are used to return the result(s) of the POU call.
 They are passed by reference, but are optional.
+
+A function or method starts every call with its outputs set to their initial value, or to zero when they declare none, exactly like its local variables.
+A variable length array, a `REFERENCE TO`, or an alias output is the exception and keeps what the caller passed.
+Because the reset is written to the caller's variable at the start of the call, do not pass the same variable as an output and as an in/out of one call.
+An input or output argument that a call leaves out, or an output argument written empty (`out =>`), also takes the parameter's initial value or zero; a `VAR_IN_OUT` or `REFERENCE TO` argument must always be supplied.
 If an output parameter is not passed in a call, its value is not persisted.
 
 ### Variables

compiler/plc_ast/src/ast.rs

@@ -556,6 +556,10 @@ impl VariableBlockType {
     pub fn is_inout(&self) -> bool {
         matches!(self, VariableBlockType::InOut)
     }
+
+    pub fn is_output(&self) -> bool {
+        matches!(self, VariableBlockType::Output)
+    }
 }
 
 impl Display for VariableBlockType {

compiler/plc_lowering/src/initializer.rs

@@ -245,7 +245,11 @@ impl AstVisitor for Initializer {
         };
         let mut stmts = vec![];
         let base = Self::get_base_ident(&variable_block_type, is_stateful);
-        if !variable_block_type.is_inout() {
+        let is_vla = variable
+            .data_type_declaration
+            .get_referenced_type()
+            .is_some_and(|it| index.get_type_information_or_void(it).is_vla());
+        if !variable_block_type.is_inout() && !is_vla {
             if let Some(constructor) = variable
                 .data_type_declaration
                 .get_referenced_type()
@@ -324,10 +328,7 @@ impl AstVisitor for Initializer {
                 }
             }
         }
-        if variable_block_type.is_temp()
-            || (variable_block_type.is_local() && !is_stateful)
-            || variable_block_type.is_inout()
-        {
+        if Self::is_stack_variable(&variable_block_type, is_stateful) || variable_block_type.is_inout() {
             self.add_to_current_stack_constructor(stmts);
         } else {
             self.add_to_current_constructor(stmts);
@@ -995,9 +996,15 @@ impl Initializer {
         None
     }
 
+    /// Temps, and locals and outputs of a stateless POU, live on the stack and are initialized on
+    /// every call; an output of a function or method is reset through the caller's address.
+    fn is_stack_variable(variable_block_type: &VariableBlockType, is_stateful: bool) -> bool {
+        variable_block_type.is_temp()
+            || ((variable_block_type.is_local() || variable_block_type.is_output()) && !is_stateful)
+    }
+
     fn get_base_ident(variable_block_type: &VariableBlockType, is_stateful: bool) -> Option<&str> {
-        if variable_block_type.is_temp()
-            || (variable_block_type.is_local() && !is_stateful)
+        if Self::is_stack_variable(variable_block_type, is_stateful)
             || variable_block_type.is_inout()
             || variable_block_type.is_global()
         {

src/codegen/generators/expression_generator.rs

@@ -1350,22 +1350,16 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
         declared_parameter: Option<&VariableIndexEntry>,
     ) -> Result<BasicValueEnum<'ink>, CodegenError> {
         if argument.is_empty_statement() {
-            // Uninitialized var_output / var_in_out
+            // an explicitly empty by-ref argument gets an initialized temporary, like an omitted one
+            if let Some(parameter) = declared_parameter {
+                return self.generate_empty_expression(parameter);
+            }
             let v_type = self
                 .llvm_index
                 .find_associated_type(type_name)
                 .ok_or_else(|| Diagnostic::unknown_type(type_name, argument))?;
-
             let ptr_value = self.llvm.builder.build_alloca(v_type, "")?;
-            if let Some(p) = declared_parameter {
-                if let Some(initial_value) =
-                    self.get_initial_value(&p.initial_value, &self.get_parameter_type(p))
-                {
-                    let value = self.generate_expression(initial_value)?;
-                    self.llvm.builder.build_store(ptr_value, value)?;
-                }
-            }
-
+            self.llvm.builder.build_store(ptr_value, v_type.const_zero())?;
             return Ok(ptr_value.into());
         }
 
@@ -1622,46 +1616,40 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
             .find_effective_type_info(parameter.get_type_name())
             .is_some_and(|it| it.is_reference_to());
 
+        // a missing argument gets its declared initial value, or zero
+        let initial_value = self.get_initial_value(&parameter.initial_value, &parameter_type_name);
+
         match declaration_type {
             ArgumentType::ByVal(..) if parameter_is_reference_to => {
+                // REFERENCE TO parameters expect an address; pass the address of a zeroed temporary
                 let ptr_value = self.llvm.builder.build_alloca(parameter_type, "")?;
+                self.llvm.builder.build_store(ptr_value, parameter_type.const_zero())?;
                 Ok(ptr_value.as_basic_value_enum())
             }
             ArgumentType::ByVal(..)
                 if declaration_type.is_input() && parameter_type_info.is_aggregate_type() =>
             {
                 // Aggregate VAR_INPUT defaults are passed by reference in function signatures.
                 let ptr_value = self.llvm.builder.build_alloca(parameter_type, "")?;
-                if let Some(initial_value) =
-                    self.get_initial_value(&parameter.initial_value, &parameter_type_name)
-                {
-                    let value = self.generate_expression(initial_value)?;
-                    self.llvm.builder.build_store(ptr_value, value)?;
-                }
+                let value = match initial_value {
+                    Some(initial_value) => self.generate_expression(initial_value)?,
+                    None => parameter_type.const_zero(),
+                };
+                self.llvm.builder.build_store(ptr_value, value)?;
                 Ok(ptr_value.as_basic_value_enum())
             }
-            ArgumentType::ByVal(..) => {
-                if let Some(initial_value) =
-                    self.get_initial_value(&parameter.initial_value, &parameter_type_name)
-                {
-                    self.generate_expression(initial_value)
-                } else {
-                    let ptr_value = self.llvm.builder.build_alloca(parameter_type, "")?;
-                    let pointee = parameter_type;
-                    Ok(self.llvm.load_pointer(pointee, &ptr_value, "")?)
-                }
-            }
+            ArgumentType::ByVal(..) => match initial_value {
+                Some(initial_value) => self.generate_expression(initial_value),
+                None => Ok(parameter_type.const_zero()),
+            },
             _ => {
+                // by-ref parameters receive the address of an initialized temporary
                 let ptr_value = self.llvm.builder.build_alloca(parameter_type, "")?;
-
-                // if default value is given for an output
-                // we need to initialize the pointer value before returning
-                if let Some(initial_value) =
-                    self.get_initial_value(&parameter.initial_value, &parameter_type_name)
-                {
-                    let value = self.generate_expression(initial_value)?;
-                    self.llvm.builder.build_store(ptr_value, value)?;
-                }
+                let value = match initial_value {
+                    Some(initial_value) => self.generate_expression(initial_value)?,
+                    None => parameter_type.const_zero(),
+                };
+                self.llvm.builder.build_store(ptr_value, value)?;
                 Ok(ptr_value.as_basic_value_enum())
             }
         }
@@ -1726,9 +1714,8 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                 .expect("POU type for parameter struct must exist");
             let pointer_to_param = self.build_parameter_struct_gep(pointee, param_context);
 
-            let parameter = self
-                .index
-                .find_parameter(param_context.declaring_pou, index)
+            let parameter_entry = self.index.find_parameter(param_context.declaring_pou, index);
+            let parameter = parameter_entry
                 .and_then(|var| self.index.find_effective_type_by_name(var.get_type_name()))
                 .map(|var| var.get_type_information())
                 .unwrap_or_else(|| self.index.get_void_type().get_type_information());
@@ -1737,11 +1724,19 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                 //this is a VAR_IN_OUT assignment, so don't load the value, assign the pointer
                 //expression may be empty -> generate a local variable for it
                 let generated_exp = if expression.is_empty_statement() {
-                    let temp_type = self
-                        .llvm_index
-                        .find_associated_type(inner_type_name)
-                        .ok_or_else(|| Diagnostic::unknown_type(parameter.get_name(), expression))?;
-                    builder.build_alloca(temp_type, "empty_varinout")?.as_basic_value_enum()
+                    match parameter_entry {
+                        // the temporary starts at the parameter's default, or zero
+                        Some(parameter_entry) => self.generate_empty_expression(parameter_entry)?,
+                        None => {
+                            let temp_type = self
+                                .llvm_index
+                                .find_associated_type(inner_type_name)
+                                .ok_or_else(|| Diagnostic::unknown_type(parameter.get_name(), expression))?;
+                            let temp = builder.build_alloca(temp_type, "empty_varinout")?;
+                            builder.build_store(temp, temp_type.const_zero())?;
+                            temp.as_basic_value_enum()
+                        }
+                    }
                 } else {
                     self.generate_lvalue(expression)?.as_basic_value_enum()
                 };

src/codegen/generators/pou_generator.rs

@@ -625,6 +625,7 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
                     &function_context,
                     debug,
                 )?;
+                self.generate_initialization_of_output_params(&pou_members, &local_index)?;
             } else {
                 //Generate temp variables
                 let members = pou_members.into_iter().filter(|it| it.is_temp()).collect::<Vec<_>>();
@@ -946,6 +947,68 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
         Ok(())
     }
 
+    /// resets every by-ref output parameter of a function or method to its initial value, so the
+    /// caller never observes a stale value through an output the body does not assign
+    fn generate_initialization_of_output_params(
+        &self,
+        variables: &[&VariableIndexEntry],
+        local_llvm_index: &LlvmTypedIndex,
+    ) -> Result<(), CodegenError> {
+        let exp_gen = ExpressionCodeGenerator::new_context_free(
+            &self.llvm,
+            self.index,
+            self.annotations,
+            local_llvm_index,
+        );
+        let outputs = variables.iter().filter(|it| it.is_output() && it.get_declaration_type().is_by_ref());
+
+        for variable in outputs {
+            let Some(inner_type_name) = self
+                .index
+                .find_effective_type_info(variable.get_type_name())
+                .and_then(|it| it.get_inner_pointer_type_name())
+            else {
+                continue;
+            };
+            // reference and alias outputs are bound by the body, and a variable length array
+            // output holds the caller's bounds and data pointer, so neither is reset
+            if self
+                .index
+                .find_effective_type_info(inner_type_name)
+                .is_some_and(|it| it.is_reference_to() || it.is_alias() || it.is_vla())
+            {
+                continue;
+            }
+            let Some(pointer_slot) =
+                local_llvm_index.find_loaded_associated_variable_value(variable.get_qualified_name())
+            else {
+                continue;
+            };
+
+            let ptr_type = self.llvm.context.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC));
+            let output = self.llvm.builder.build_load(ptr_type, pointer_slot, "")?.into_pointer_value();
+            // only a resolved scalar constant is applied here; an aggregate or non-constant
+            // initializer is assigned by the lowered stack initializer, which runs with the
+            // instance in scope
+            let is_aggregate =
+                self.index.get_effective_type_or_void_by_name(inner_type_name).is_aggregate_type();
+            let initializer = variable
+                .initial_value
+                .as_ref()
+                .filter(|_| !is_aggregate)
+                .and_then(|id| self.index.get_const_expressions().get_resolved_constant_statement(id));
+            self.llvm.generate_variable_initializer(
+                self.llvm_index,
+                self.index,
+                (variable.get_qualified_name(), inner_type_name, &variable.source_location),
+                output,
+                initializer,
+                &exp_gen,
+            )?;
+        }
+        Ok(())
+    }
+
     /// initializes the variable represented by `variable` by storing into the given `variable_to_initialize` pointer using either
     /// the optional `initializer_statement` (hence code like: `variable : type := initializer_statement`), or determine the initial
     /// value with the help of the `variable`'s index entry by e.g. looking for a default value of the variable's type

src/codegen/tests/directaccess_test.rs

@@ -551,9 +551,11 @@ fn direct_access_in_output_assignment_of_function() {
       store i8 %0, ptr %in, align [filtered]
       %out = alloca ptr, align [filtered]
       store ptr %1, ptr %out, align [filtered]
+      %2 = load ptr, ptr %out, align [filtered]
+      store i8 0, ptr %2, align [filtered]
       %deref = load ptr, ptr %out, align [filtered]
-      %2 = load i8, ptr %in, align [filtered]
-      %tmpVar = xor i8 %2, -1
+      %3 = load i8, ptr %in, align [filtered]
+      %tmpVar = xor i8 %3, -1
       store i8 %tmpVar, ptr %deref, align [filtered]
       ret void
     }

src/codegen/tests/fnptr.rs

@@ -300,6 +300,8 @@ fn function_pointer_method_with_all_variable_parameter_types() {
       %inout = alloca ptr, align [filtered]
       store ptr %3, ptr %inout, align [filtered]
       store i32 0, ptr %A.foo, align [filtered]
+      %4 = load ptr, ptr %out, align [filtered]
+      call void @llvm.memset.p0.i64(ptr align [filtered] %4, i8 0, i64 ptrtoint (ptr getelementptr ([81 x i8], ptr null, i32 1) to i64), i1 false)
       %A__foo_ret = load i32, ptr %A.foo, align [filtered]
       ret i32 %A__foo_ret
     }

src/codegen/tests/function_tests.rs

@@ -544,6 +544,8 @@ fn function_output_should_be_cast_if_needed() {
       %result = alloca ptr, align [filtered]
       store ptr %2, ptr %result, align [filtered]
       store i16 0, ptr %libFunction, align [filtered]
+      %3 = load ptr, ptr %result, align [filtered]
+      store float 0.000000e+00, ptr %3, align [filtered]
       %libFunction_ret = load i16, ptr %libFunction, align [filtered]
       ret i16 %libFunction_ret
     }

src/codegen/tests/oop_tests.rs

@@ -2524,6 +2524,8 @@ fn fb_extension_with_output() {
       %outputValue = alloca ptr, align [filtered]
       store ptr %3, ptr %outputValue, align [filtered]
       store i16 0, ptr %foo.met1, align [filtered]
+      %4 = load ptr, ptr %outputValue, align [filtered]
+      store i16 0, ptr %4, align [filtered]
       %foo__met1_ret = load i16, ptr %foo.met1, align [filtered]
       ret i16 %foo__met1_ret
     }
@@ -2541,6 +2543,7 @@ fn fb_extension_with_output() {
       %1 = load ptr, ptr %met1, align [filtered]
       %deref3 = load ptr, ptr %this, align [filtered]
       %2 = alloca i16, align [filtered]
+      store i16 0, ptr %2, align [filtered]
       %fnptr_call = call i16 %1(ptr %deref3, i16 0, i16 0, ptr %2)
       ret void
     }
@@ -2726,14 +2729,16 @@ fn function_with_output_used_in_main_by_extension() {
       %outputValue = alloca ptr, align [filtered]
       store ptr %3, ptr %outputValue, align [filtered]
       store i16 0, ptr %foo.met1, align [filtered]
+      %4 = load ptr, ptr %outputValue, align [filtered]
+      store i16 0, ptr %4, align [filtered]
       %deref = load ptr, ptr %outputValue, align [filtered]
       %load_mandatoryInput = load i16, ptr %mandatoryInput, align [filtered]
-      %4 = sext i16 %load_mandatoryInput to i32
+      %5 = sext i16 %load_mandatoryInput to i32
       %load_optionalInput = load i16, ptr %optionalInput, align [filtered]
-      %5 = sext i16 %load_optionalInput to i32
-      %tmpVar = add i32 %4, %5
-      %6 = trunc i32 %tmpVar to i16
-      store i16 %6, ptr %deref, align [filtered]
+      %6 = sext i16 %load_optionalInput to i32
+      %tmpVar = add i32 %5, %6
+      %7 = trunc i32 %tmpVar to i16
+      store i16 %7, ptr %deref, align [filtered]
       %foo__met1_ret = load i16, ptr %foo.met1, align [filtered]
       ret i16 %foo__met1_ret
     }