refactor: self-review

Author: volsa

compiler/plc_ast/src/ast.rs

@@ -1485,7 +1485,8 @@ impl AstNode {
         matches!(self.stmt, AstStatement::Literal(AstLiteral::Real(_), ..))
     }
 
-    pub fn get_name_of_lhs_of_assignment(&self) -> Option<&str> {
+    /// Returns the identifier of the left-hand side if this is an assignment statement
+    pub fn get_assignment_identifier(&self) -> Option<&str> {
         match &self.stmt {
             AstStatement::Assignment(Assignment { left, .. })
             | AstStatement::OutputAssignment(Assignment { left, .. })

src/builtins.rs

@@ -672,7 +672,7 @@ fn annotate_comparison_function(
             .has_nature(TypeNature::Elementary, annotator.index)
     }) {
         // we are trying to call this function with a non-elementary type, so we redirect back to the resolver
-        annotator.annotate_call_statement(operator, Some(parameters), &ctx);
+        annotator.annotate_arguments(operator, parameters, &ctx);
         return;
     }
 
@@ -741,7 +741,7 @@ fn annotate_arithmetic_function(
             .has_nature(TypeNature::Num, annotator.index)
     }) {
         // we are trying to call this function with a non-numerical type, so we redirect back to the resolver
-        annotator.annotate_call_statement(operator, Some(parameters), &ctx);
+        annotator.annotate_arguments(operator, parameters, &ctx);
         return;
     }
 

src/codegen/generators/expression_generator.rs

@@ -68,15 +68,25 @@ pub struct ExpressionCodeGenerator<'a, 'b> {
 /// context information to generate a parameter
 #[derive(Debug)]
 struct CallParameterAssignment<'a, 'b> {
-    /// the assignmentstatement in the call-argument list (a:=3)
+    /// The named argument node of the function call (e.g. `foo(in := 3)`)
     assignment: &'b AstNode,
-    /// the name of the function we're calling
+
+    /// The name of the POU being called
     function_name: &'b str,
-    /// the position of the argument in the POU's argument's list
-    index: u32,
+
+    /// The position of the parameter in the POUs variable declaration list.
+    /// See also [`StatementAnnotation::Argument::position`].
+    position: u32,
+
+    /// The depth between where the parameter is declared versus where it is being called from.
+    /// See also [`StatementAnnotation::Argument::depth`].
     depth: u32,
-    arg_pou: &'b str,
-    /// a pointer to the struct instance that carries the call's arguments
+
+    /// The name of the POU where the parameter is declared
+    /// See also [`StatementAnnotation::Argument::pou`].
+    pou_parameter: &'b str,
+
+    /// The pointer to the struct instance that carries the call's arguments
     parameter_struct: PointerValue<'a>,
 }
 
@@ -713,9 +723,9 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                 self.assign_output_value(&CallParameterAssignment {
                     assignment: assignment_statement,
                     function_name,
-                    index: *position as u32,
+                    position: *position as u32,
                     depth: *depth as u32,
-                    arg_pou: pou,
+                    pou_parameter: pou,
                     parameter_struct,
                 })?
             }
@@ -843,13 +853,28 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
         Ok(())
     }
 
+    fn build_parameter_struct_gep(&self, context: &CallParameterAssignment<'ink, 'b>) -> PointerValue<'ink> {
+        unsafe {
+            let mut gep_index = vec![0; (context.depth + 1) as usize];
+            gep_index.push(context.position);
+
+            let i32_type = self.llvm.context.i32_type();
+            let gep_index = gep_index
+                .into_iter()
+                .map(|value| i32_type.const_int(value as u64, false))
+                .collect::<Vec<_>>();
+
+            self.llvm.builder.build_gep(context.parameter_struct, &gep_index, "").unwrap()
+        }
+    }
+
     fn generate_output_assignment(&self, context: &CallParameterAssignment) -> Result<(), CodegenError> {
         let &CallParameterAssignment {
             assignment: expr,
             function_name,
-            index,
+            position: index,
             depth: _,
-            arg_pou,
+            pou_parameter: arg_pou,
             parameter_struct,
         } = context;
 
@@ -901,18 +926,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                 let assigned_output = self.generate_lvalue(expr)?;
                 let assigned_output_type =
                     self.annotations.get_type_or_void(expr, self.index).get_type_information();
-                let output = unsafe {
-                    let mut gep_index = vec![0; (context.depth + 1) as usize];
-                    gep_index.push(context.index);
-
-                    let i32_type = self.llvm.context.i32_type();
-                    let gep_index = gep_index
-                        .into_iter()
-                        .map(|value| i32_type.const_int(value as u64, false))
-                        .collect::<Vec<_>>();
-
-                    builder.build_gep(parameter_struct, &gep_index, "").unwrap()
-                };
+                let output = self.build_parameter_struct_gep(context);
 
                 let output_value_type = self.index.get_type_information_or_void(parameter.get_type_name());
 
@@ -948,10 +962,10 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
             self.generate_output_assignment(&CallParameterAssignment {
                 assignment: right,
                 function_name,
-                index: param_context.index,
+                position: param_context.position,
                 depth: param_context.depth,
                 parameter_struct,
-                arg_pou: param_context.arg_pou,
+                pou_parameter: param_context.pou_parameter,
             })?
         };
 
@@ -1304,9 +1318,9 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
             let parameter = self.generate_call_struct_argument_assignment(&CallParameterAssignment {
                 assignment: argument,
                 function_name: pou_name,
-                index: *position as u32,
+                position: *position as u32,
                 depth: *depth as u32,
-                arg_pou: pou,
+                pou_parameter: pou,
                 parameter_struct,
             })?;
             if let Some(parameter) = parameter {
@@ -1398,34 +1412,22 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
     ) -> Result<Option<BasicValueEnum<'ink>>, CodegenError> {
         let builder = &self.llvm.builder;
         // let function_name = param_context.function_name;
-        let index = param_context.index;
-        let parameter_struct = param_context.parameter_struct;
+        let index = param_context.position;
         let expression = param_context.assignment;
 
-        if let Some(parameter) = self.index.get_declared_parameter(param_context.arg_pou, index) {
+        if let Some(parameter) = self.index.get_declared_parameter(param_context.pou_parameter, index) {
             // this happens before the pou call
             // before the call statement we may only consider inputs and inouts
             // after the call we need to copy the output values to the correct assigned variables
             if matches!(parameter.get_variable_type(), VariableType::Output) {
                 return Ok(None);
             }
 
-            let pointer_to_param = unsafe {
-                let mut gep_index = vec![0; (param_context.depth + 1) as usize];
-                gep_index.push(param_context.index);
-
-                let i32_type = self.llvm.context.i32_type();
-                let gep_index = gep_index
-                    .into_iter()
-                    .map(|value| i32_type.const_int(value as u64, false))
-                    .collect::<Vec<_>>();
-
-                builder.build_gep(parameter_struct, &gep_index, "").unwrap()
-            };
+            let pointer_to_param = self.build_parameter_struct_gep(param_context);
 
             let parameter = self
                 .index
-                .find_parameter(param_context.arg_pou, index)
+                .find_parameter(param_context.pou_parameter, index)
                 .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());
@@ -1484,9 +1486,9 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                 self.generate_call_struct_argument_assignment(&CallParameterAssignment {
                     assignment: right,
                     function_name,
-                    index: param_context.index,
+                    position: param_context.position,
                     depth: param_context.depth,
-                    arg_pou: param_context.arg_pou,
+                    pou_parameter: param_context.pou_parameter,
                     parameter_struct,
                 })?;
             };
@@ -1532,7 +1534,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                     let member_location = self
                         .index
                         .find_fully_qualified_variable(qualified_name)
-                        .map(VariableIndexEntry::get_location_in_parent)
+                        .map(VariableIndexEntry::get_position)
                         .ok_or_else(|| Diagnostic::unresolved_reference(qualified_name, offset))?;
                     let gep: PointerValue<'_> =
                         self.llvm.get_member_pointer_from_struct(*qualifier, member_location, name)?;
@@ -2274,7 +2276,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                                 Diagnostic::unresolved_reference(qualified_name, data.left.as_ref())
                             })?;
 
-                        let index_in_parent = member.get_location_in_parent();
+                        let index_in_parent = member.get_position();
                         let value = self.generate_expression(data.right.as_ref())?;
 
                         uninitialized_members.remove(member);
@@ -2306,7 +2308,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                         Diagnostic::cannot_generate_initializer(member.get_qualified_name(), assignments)
                     })?;
 
-                member_values.push((member.get_location_in_parent(), initial_value));
+                member_values.push((member.get_position(), initial_value));
             }
             let struct_type = self.llvm_index.get_associated_type(struct_name)?.into_struct_type();
             if member_values.len() == struct_type.count_fields() as usize {

src/index.rs

@@ -199,7 +199,7 @@ impl VariableIndexEntry {
         self.data_type_name.as_str()
     }
 
-    pub fn get_location_in_parent(&self) -> u32 {
+    pub fn get_position(&self) -> u32 {
         self.location_in_parent
     }
 
@@ -1551,30 +1551,31 @@ impl Index {
             })
     }
 
-    fn truly_find_local_member(&self, pou: &str, name: &str) -> Option<&VariableIndexEntry> {
-        self.type_index.find_type(pou).and_then(|it| it.find_member(name))
-    }
+    // XXX: This is super specific and currently only being used in the context of argument annotations. Do we
+    // want to move this to the resolver?
+    /// Finds a member in the specified POU, traversing the inheritance chain if necessary. Returns the
+    /// [`VariableIndexEntry`] along with the inheritance depth from the given POU to where the member
+    /// was declared.
+    pub fn find_pou_member_and_depth(&self, pou: &str, name: &str) -> Option<(&VariableIndexEntry, usize)> {
+        fn find<'a>(index: &'a Index, pou: &str, name: &str) -> Option<&'a VariableIndexEntry> {
+            index.type_index.find_type(pou).and_then(|pou| pou.find_member(name))
+        }
 
-    // TODO: Own type?
-    /// Given some POU name and one of its members' name, returns the member and the position including its
-    /// inheritance level. For example If we have `A { localVarA }, B extends A { localVarB }`, then
-    /// `find_member_with_path("B", "localVarA")` will return `(localVarA, 0, 1)`
-    pub fn find_member_with_path(&self, pou: &str, name: &str) -> Option<(String, &VariableIndexEntry, u32)> {
         // Check if the POU has the member locally
-        if let Some(entry) = self.truly_find_local_member(pou, name) {
-            return Some((pou.to_string(), entry, 0));
+        if let Some(entry) = find(self, pou, name) {
+            return Some((entry, 0));
         }
 
         // ..and if not walk the inheritance chain and re-try
-        let mut level = 1;
+        let mut depth = 1;
         let mut current_pou = pou;
 
-        while let Some(parent) = self.find_pou(current_pou).and_then(|it| it.get_super_class()) {
-            if let Some(entry) = self.truly_find_local_member(parent, name) {
-                return Some((parent.to_string(), entry, level));
+        while let Some(parent) = self.find_pou(current_pou).and_then(PouIndexEntry::get_super_class) {
+            if let Some(entry) = find(self, parent, name) {
+                return Some((entry, depth));
             }
 
-            level += 1;
+            depth += 1;
             current_pou = parent;
         }
 
@@ -1731,21 +1732,30 @@ impl Index {
         self.get_pou_types().get(&pou_name.to_lowercase())
     }
 
-    pub fn get_declared_parameters(&self, pou_name: &str) -> Vec<&VariableIndexEntry> {
-        self.get_pou_members(pou_name)
-            .iter()
-            .filter(|it| it.is_parameter() && !it.is_variadic())
-            .collect::<Vec<_>>()
+    /// Returns the parameter (INPUT, OUTPUT or IN_OUT) for the given POU by its location, if it exists.
+    pub fn get_declared_parameter(&self, pou_name: &str, index: u32) -> Option<&VariableIndexEntry> {
+        self.type_index.find_pou_type(pou_name).and_then(|it| it.find_declared_parameter_by_location(index))
     }
 
-    /// Returns all declared parameters of a POU, including those defined in super-classes
-    pub fn get_declared_parameters_2nd(&self, pou: &str) -> Vec<&VariableIndexEntry> {
-        let mut pou = pou;
-        let mut parameters = self.get_declared_parameters(pou);
+    /// Returns all declared parameters (INPUT, OUTPUT or IN_OUT) of a POU, including those defined in parent
+    /// POUs. The returned list is ordered by the inheritance chain, from base to derived.
+    pub fn get_declared_parameters(&self, pou: &str) -> Vec<&VariableIndexEntry> {
+        // Collect all POU names in the inheritance chain from base to derived
+        let mut chain = Vec::new();
+        let mut current = Some(pou);
+        let mut parameters = Vec::new();
 
-        while let Some(parent) = self.find_pou(pou).and_then(PouIndexEntry::get_super_class) {
-            parameters.extend(self.get_declared_parameters(parent));
-            pou = parent;
+        // Walk the inheritance chain and collect its POU names; only has an effect on function block calls
+        while let Some(pou_name) = current {
+            chain.push(pou_name);
+            current = self.find_pou(pou_name).and_then(PouIndexEntry::get_super_class);
+        }
+
+        // Then, reverse the chain to start at the root and collect its parameters
+        for &name in chain.iter().rev() {
+            parameters.extend(
+                self.get_pou_members(name).iter().filter(|var| var.is_parameter() && !var.is_variadic()),
+            );
         }
 
         parameters
@@ -1755,13 +1765,6 @@ impl Index {
         self.get_pou_members(pou_name).iter().any(|member| member.is_parameter() && member.is_variadic())
     }
 
-    /// returns some if the current index is a VAR_INPUT, VAR_IN_OUT or VAR_OUTPUT that is not a variadic argument
-    /// In other words it returns some if the member variable at `index` of the given container is a possible parameter in
-    /// the call to it
-    pub fn get_declared_parameter(&self, pou_name: &str, index: u32) -> Option<&VariableIndexEntry> {
-        self.type_index.find_pou_type(pou_name).and_then(|it| it.find_declared_parameter_by_location(index))
-    }
-
     pub fn get_variadic_member(&self, pou_name: &str) -> Option<&VariableIndexEntry> {
         self.type_index.find_pou_type(pou_name).and_then(|it| it.find_variadic_member())
     }