wip: 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

@@ -1532,7 +1532,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 +2274,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 +2306,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.
+    // Should we move this to the resolver.rs file?
+    /// 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,32 @@ 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 for the given POU by its location, if it exists.
+    ///
+    /// A parameter is defined as a `VAR_INPUT`, `VAR_OUTPUT` or `VAR_IN_OUT` variable.
+    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 of a POU, including those defined in super-classes,
+    /// in intuitive order: base class parameters first, then 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 +1767,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())
     }

src/index/tests/index_tests.rs

@@ -2541,3 +2541,67 @@ fn enum_ensure_a_combination_of_variables_can_be_assigned_in_function() {
     assert!(index.find_local_member("fn", "blue").is_some());
     assert_eq!(index.find_local_member("fn", "blue").unwrap().data_type_name, "EnumType");
 }
+
+#[test]
+fn declared_parameters() {
+    let (_, index) = index(
+        r#"
+        FUNCTION_BLOCK FbA
+            VAR
+                localA: DINT;
+            END_VAR
+
+            VAR_INPUT
+                inA: DINT;
+            END_VAR
+
+            VAR_OUTPUT
+                outA: DINT;
+            END_VAR
+
+            VAR_IN_OUT
+                inoutA: DINT;
+            END_VAR
+
+            METHOD methA
+            END_METHOD
+        END_FUNCTION_BLOCK
+
+        FUNCTION_BLOCK FbB EXTENDS FbA
+            VAR
+                localB: DINT;
+            END_VAR
+
+            VAR_INPUT
+                inB: DINT;
+            END_VAR
+
+            VAR_OUTPUT
+                outB: DINT;
+            END_VAR
+
+            VAR_IN_OUT
+                inoutB: DINT;
+            END_VAR
+
+            METHOD methB
+                VAR_INPUT
+                    inB_meth: DINT;
+                END_VAR
+            END_METHOD
+        END_FUNCTION_BLOCK
+    "#,
+    );
+
+    let members = index.get_declared_parameters("FbA").iter().map(|var| &var.name).collect::<Vec<_>>();
+    assert_eq!(members, vec!["inA", "outA", "inoutA"]);
+
+    let members = index.get_declared_parameters("FbB").iter().map(|var| &var.name).collect::<Vec<_>>();
+    assert_eq!(members, vec!["inA", "outA", "inoutA", "inB", "outB", "inoutB",]);
+
+    let members = index.get_declared_parameters("methA").iter().map(|var| &var.name).collect::<Vec<_>>();
+    assert!(members.is_empty());
+
+    let members = index.get_declared_parameters("FbB.methB").iter().map(|var| &var.name).collect::<Vec<_>>();
+    assert_eq!(members, vec!["inB_meth"]);
+}