diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index f8038b39f2..067ebca90a 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -1484,6 +1484,16 @@ impl AstNode { pub fn is_real(&self) -> bool { matches!(self.stmt, AstStatement::Literal(AstLiteral::Real(_), ..)) } + + /// 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, .. }) + | AstStatement::RefAssignment(Assignment { left, .. }) => left.get_flat_reference_name(), + _ => None, + } + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] diff --git a/src/builtins.rs b/src/builtins.rs index 9a23752eb4..9eb9cb9dda 100644 --- a/src/builtins.rs +++ b/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; } diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs index 7dfbe04c9f..1eed9468ed 100644 --- a/src/codegen/generators/expression_generator.rs +++ b/src/codegen/generators/expression_generator.rs @@ -68,13 +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, - /// a pointer to the struct instance that carries the call's arguments + + /// 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, + + /// 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>, } @@ -702,14 +714,18 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { let is_output = pou_info.get(index).is_some_and(|param| param.get_variable_type().is_output()); if assignment_statement.is_output_assignment() || (implicit && is_output) { + let Some(StatementAnnotation::Argument { position, depth, pou, .. }) = + self.annotations.get_hint(assignment_statement) + else { + panic!() + }; + self.assign_output_value(&CallParameterAssignment { assignment: assignment_statement, function_name, - index: self - .annotations - .get_hint(assignment_statement) - .and_then(StatementAnnotation::get_location_in_parent) - .expect("arguments must have a type hint"), + position: *position as u32, + depth: *depth as u32, + pou_parameter: pou, parameter_struct, })? } @@ -720,11 +736,9 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { fn assign_output_value(&self, param_context: &CallParameterAssignment) -> Result<(), CodegenError> { match ¶m_context.assignment.stmt { - AstStatement::OutputAssignment(assignment) => self.generate_explicit_output_assignment( - param_context.parameter_struct, - param_context.function_name, - assignment, - ), + AstStatement::OutputAssignment(assignment) => { + self.generate_explicit_output_assignment(param_context, assignment) + } _ => self.generate_output_assignment(param_context), } @@ -839,8 +853,36 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { Ok(()) } + fn build_parameter_struct_gep(&self, context: &CallParameterAssignment<'ink, 'b>) -> PointerValue<'ink> { + unsafe { + // Assuming we have an inheritance hierarchy of `A <- B <- C` and a call `objC(inA := 1, ...)` + // then in order to access `inA` in `C` we have to generate some IR of form `objC.__B.__A.inA`. + // Because the parent structs are always the first member of these function blocks / classes, we + // can simply generate a GEP with indices `0` and repeat that `depth` times followed by the + // actual position of the parameter. So `objC.__B.__A.inA` becomes `GEP objC, 0, 0, 0, ` + 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::>(); + + 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, parameter_struct } = context; + let &CallParameterAssignment { + assignment: expr, + function_name, + position: index, + depth: _, + pou_parameter: arg_pou, + parameter_struct, + } = context; + let builder = &self.llvm.builder; // We don't want to generate any code if the right side of an assignment is empty, e.g. `foo(out =>)` @@ -848,7 +890,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { return Ok(()); } - let parameter = self.index.get_declared_parameter(function_name, index).expect("must exist"); + let parameter = self.index.get_declared_parameter(arg_pou, index).expect("must exist"); match expr.get_stmt() { AstStatement::ReferenceExpr(_) if expr.has_direct_access() => { @@ -889,12 +931,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 = builder.build_struct_gep(parameter_struct, index, "").map_err(|_| { - Diagnostic::codegen_error( - format!("Cannot build generate parameter: {parameter:#?}"), - ¶meter.source_location, - ) - })?; + let output = self.build_parameter_struct_gep(context); let output_value_type = self.index.get_type_information_or_void(parameter.get_type_name()); @@ -919,24 +956,21 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { fn generate_explicit_output_assignment( &self, - parameter_struct: PointerValue<'ink>, - function_name: &str, + param_context: &CallParameterAssignment, assignment: &Assignment, ) -> Result<(), CodegenError> { + let parameter_struct = param_context.parameter_struct; + let function_name = param_context.function_name; let Assignment { left, right } = assignment; - if let Some(StatementAnnotation::Variable { qualified_name, .. }) = self.annotations.get(left) { - let parameter = self - .index - .find_fully_qualified_variable(qualified_name) - .ok_or_else(|| Diagnostic::unresolved_reference(qualified_name, left.as_ref()))?; - let index = parameter.get_location_in_parent(); - + if let Some(StatementAnnotation::Variable { .. }) = self.annotations.get(left) { self.generate_output_assignment(&CallParameterAssignment { assignment: right, function_name, - index, + position: param_context.position, + depth: param_context.depth, parameter_struct, + pou_parameter: param_context.pou_parameter, })? }; @@ -1280,14 +1314,18 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { }) .unwrap_or_else(|| vec![parameter_struct.as_basic_value_enum().into()]); for argument in arguments.iter() { + let Some(StatementAnnotation::Argument { position, depth, pou, .. }) = + self.annotations.get_hint(argument) + else { + panic!() + }; + let parameter = self.generate_call_struct_argument_assignment(&CallParameterAssignment { assignment: argument, function_name: pou_name, - index: self - .annotations - .get_hint(argument) - .and_then(StatementAnnotation::get_location_in_parent) - .expect("arguments must have a type hint"), + position: *position as u32, + depth: *depth as u32, + pou_parameter: pou, parameter_struct, })?; if let Some(parameter) = parameter { @@ -1378,11 +1416,11 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { param_context: &CallParameterAssignment, ) -> Result>, 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 function_name = param_context.function_name; + let index = param_context.position; let expression = param_context.assignment; - if let Some(parameter) = self.index.get_declared_parameter(function_name, 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 @@ -1390,16 +1428,11 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { return Ok(None); } - let pointer_to_param = builder.build_struct_gep(parameter_struct, index, "").map_err(|_| { - Diagnostic::codegen_error( - format!("Cannot build generate parameter: {expression:#?}"), - expression, - ) - })?; + let pointer_to_param = self.build_parameter_struct_gep(param_context); let parameter = self .index - .find_parameter(function_name, 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()); @@ -1444,21 +1477,23 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { .index .find_fully_qualified_variable(qualified_name) .ok_or_else(|| Diagnostic::unresolved_reference(qualified_name, left))?; - let index = parameter.get_location_in_parent(); // don't generate param assignments for empty statements, with the exception // of VAR_IN_OUT params - they need an address to point to let is_auto_deref = self .index .find_effective_type_by_name(parameter.get_type_name()) - .map(|var| var.get_type_information()) - .unwrap_or(self.index.get_void_type().get_type_information()) + .map(DataType::get_type_information) + .unwrap_or_else(|| self.index.get_void_type().get_type_information()) .is_auto_deref(); + if !right.is_empty_statement() || is_auto_deref { self.generate_call_struct_argument_assignment(&CallParameterAssignment { assignment: right, function_name, - index, + position: param_context.position, + depth: param_context.depth, + pou_parameter: param_context.pou_parameter, parameter_struct, })?; }; @@ -1504,7 +1539,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)?; @@ -2246,7 +2281,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); @@ -2278,7 +2313,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 { diff --git a/src/codegen/tests/directaccess_test.rs b/src/codegen/tests/directaccess_test.rs index b7d3c99c76..63e9658ac0 100644 --- a/src/codegen/tests/directaccess_test.rs +++ b/src/codegen/tests/directaccess_test.rs @@ -169,7 +169,7 @@ fn direct_acess_in_output_assignment_implicit_explicit_and_mixed() { %0 = bitcast %FOO* %f to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %0, i8* align 1 getelementptr inbounds (%FOO, %FOO* @__FOO__init, i32 0, i32 0), i64 ptrtoint (%FOO* getelementptr (%FOO, %FOO* null, i32 1) to i64), i1 false) store i32 0, i32* %main, align 4 - %1 = getelementptr inbounds %FOO, %FOO* %f, i32 0, i32 0 + %1 = getelementptr %FOO, %FOO* %f, i32 0, i32 0 %load_error_bits = load i8, i8* %error_bits, align 1 %shift = lshr i8 %load_error_bits, 0 %2 = and i8 %shift, 1 @@ -182,7 +182,7 @@ fn direct_acess_in_output_assignment_implicit_explicit_and_mixed() { %value = shl i8 %5, 0 %or = or i8 %erase, %value store i8 %or, i8* %error_bits, align 1 - %6 = getelementptr inbounds %FOO, %FOO* %f, i32 0, i32 0 + %6 = getelementptr %FOO, %FOO* %f, i32 0, i32 0 %load_error_bits1 = load i8, i8* %error_bits, align 1 %shift2 = lshr i8 %load_error_bits1, 0 %7 = and i8 %shift2, 1 @@ -195,7 +195,7 @@ fn direct_acess_in_output_assignment_implicit_explicit_and_mixed() { %value4 = shl i8 %10, 0 %or5 = or i8 %erase3, %value4 store i8 %or5, i8* %error_bits, align 1 - %11 = getelementptr inbounds %FOO, %FOO* %f, i32 0, i32 0 + %11 = getelementptr %FOO, %FOO* %f, i32 0, i32 0 %load_error_bits6 = load i8, i8* %error_bits, align 1 %shift7 = lshr i8 %load_error_bits6, 0 %12 = and i8 %shift7, 1 @@ -208,7 +208,7 @@ fn direct_acess_in_output_assignment_implicit_explicit_and_mixed() { %value9 = shl i8 %15, 0 %or10 = or i8 %erase8, %value9 store i8 %or10, i8* %error_bits, align 1 - %16 = getelementptr inbounds %FOO, %FOO* %f, i32 0, i32 0 + %16 = getelementptr %FOO, %FOO* %f, i32 0, i32 0 %load_error_bits11 = load i8, i8* %error_bits, align 1 %shift12 = lshr i8 %load_error_bits11, 0 %17 = and i8 %shift12, 1 diff --git a/src/codegen/tests/fnptr.rs b/src/codegen/tests/fnptr.rs index fd962c8d07..cd81ee9c59 100644 --- a/src/codegen/tests/fnptr.rs +++ b/src/codegen/tests/fnptr.rs @@ -412,13 +412,13 @@ fn function_block_body() { store i32 0, i32* %localOut, align 4 store i64 0, i64* %localInout, align 8 %1 = load void (%A*)*, void (%A*)** %bodyPtr, align 8 - %2 = getelementptr inbounds %A, %A* %instanceA, i32 0, i32 1 + %2 = getelementptr %A, %A* %instanceA, i32 0, i32 1 %load_localIn = load i16, i16* %localIn, align 2 store i16 %load_localIn, i16* %2, align 2 - %3 = getelementptr inbounds %A, %A* %instanceA, i32 0, i32 3 + %3 = getelementptr %A, %A* %instanceA, i32 0, i32 3 store i64* %localInout, i64** %3, align 8 call void %1(%A* %instanceA) - %4 = getelementptr inbounds %A, %A* %instanceA, i32 0, i32 2 + %4 = getelementptr %A, %A* %instanceA, i32 0, i32 2 %5 = load i32, i32* %4, align 4 store i32 %5, i32* %localOut, align 4 ret void diff --git a/src/codegen/tests/parameters_tests.rs b/src/codegen/tests/parameters_tests.rs index b2639216a0..93dc3e92c2 100644 --- a/src/codegen/tests/parameters_tests.rs +++ b/src/codegen/tests/parameters_tests.rs @@ -649,10 +649,11 @@ fn parameters_behind_function_block_pointer_are_assigned_to() { fn var_in_out_params_can_be_out_of_order() { let res = codegen( "PROGRAM mainProg - VAR - fb : fb_t; - out1, out2 : BOOL; - END_VAR + VAR + fb : fb_t; + out1, out2 : BOOL; + END_VAR + fb(myOtherInOut := out1, myInOut := out2); fb(myInOut := out1, myOtherInOut := out2); @@ -661,21 +662,25 @@ fn var_in_out_params_can_be_out_of_order() { END_PROGRAM FUNCTION_BLOCK fb_t - VAR - myVar : BOOL; - END_VAR - VAR_INPUT - myInput : USINT; - END_VAR - VAR_IN_OUT - myInOut : BOOL; - END_VAR - VAR_OUTPUT - myOut : BOOL; - END_VAR - VAR_IN_OUT - myOtherInOut : BOOL; - END_VAR + VAR + myVar : BOOL; + END_VAR + + VAR_INPUT + myInput : USINT; + END_VAR + + VAR_IN_OUT + myInOut : BOOL; + END_VAR + + VAR_OUTPUT + myOut : BOOL; + END_VAR + + VAR_IN_OUT + myOtherInOut : BOOL; + END_VAR END_FUNCTION_BLOCK ACTIONS @@ -1058,11 +1063,11 @@ fn by_value_fb_arg_aggregates_are_memcopied() { %2 = bitcast %FOO* %fb to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %2, i8* align 1 getelementptr inbounds (%FOO, %FOO* @__FOO__init, i32 0, i32 0, i32 0), i64 ptrtoint (%FOO* getelementptr (%FOO, %FOO* null, i32 1) to i64), i1 false) store i32 0, i32* %main, align 4 - %3 = getelementptr inbounds %FOO, %FOO* %fb, i32 0, i32 0 + %3 = getelementptr %FOO, %FOO* %fb, i32 0, i32 0 %4 = bitcast [65537 x i8]* %3 to i8* %5 = bitcast [65537 x i8]* %str to i8* call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 1 %4, i8* align 1 %5, i32 65536, i1 false) - %6 = getelementptr inbounds %FOO, %FOO* %fb, i32 0, i32 1 + %6 = getelementptr %FOO, %FOO* %fb, i32 0, i32 1 %7 = bitcast [1024 x i32]* %6 to i8* %8 = bitcast [1024 x i32]* %arr to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %7, i8* align 1 %8, i64 ptrtoint ([1024 x i32]* getelementptr ([1024 x i32], [1024 x i32]* null, i32 1) to i64), i1 false) @@ -1162,23 +1167,23 @@ fn var_output_aggregate_types_are_memcopied() { %out5 = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 4 %station = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 5 call void @FB(%FB* %station) - %1 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 0 + %1 = getelementptr %FB, %FB* %station, i32 0, i32 0 %2 = bitcast %OUT_TYPE* %out to i8* %3 = bitcast %OUT_TYPE* %1 to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %2, i8* align 1 %3, i64 ptrtoint (%OUT_TYPE* getelementptr (%OUT_TYPE, %OUT_TYPE* null, i32 1) to i64), i1 false) - %4 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 1 + %4 = getelementptr %FB, %FB* %station, i32 0, i32 1 %5 = bitcast [11 x i32]* %out2 to i8* %6 = bitcast [11 x i32]* %4 to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %5, i8* align 1 %6, i64 ptrtoint ([11 x i32]* getelementptr ([11 x i32], [11 x i32]* null, i32 1) to i64), i1 false) - %7 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 2 + %7 = getelementptr %FB, %FB* %station, i32 0, i32 2 %8 = bitcast [11 x %OUT_TYPE]* %out3 to i8* %9 = bitcast [11 x %OUT_TYPE]* %7 to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %8, i8* align 1 %9, i64 ptrtoint ([11 x %OUT_TYPE]* getelementptr ([11 x %OUT_TYPE], [11 x %OUT_TYPE]* null, i32 1) to i64), i1 false) - %10 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 3 + %10 = getelementptr %FB, %FB* %station, i32 0, i32 3 %11 = bitcast [81 x i8]* %out4 to i8* %12 = bitcast [81 x i8]* %10 to i8* call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 1 %11, i8* align 1 %12, i32 80, i1 false) - %13 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 4 + %13 = getelementptr %FB, %FB* %station, i32 0, i32 4 %14 = bitcast [81 x i16]* %out5 to i8* %15 = bitcast [81 x i16]* %13 to i8* call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 2 %14, i8* align 2 %15, i32 160, i1 false) @@ -1527,7 +1532,7 @@ fn function_block_with_array_of_array_parameter_stride_calculation() { entry: %processor = getelementptr inbounds %main, %main* %0, i32 0, i32 0 %data = getelementptr inbounds %main, %main* %0, i32 0, i32 1 - %1 = getelementptr inbounds %MatrixProcessor, %MatrixProcessor* %processor, i32 0, i32 0 + %1 = getelementptr %MatrixProcessor, %MatrixProcessor* %processor, i32 0, i32 0 store [2 x [4 x float]]* %data, [2 x [4 x float]]** %1, align 8 call void @MatrixProcessor(%MatrixProcessor* %processor) ret void diff --git a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_input_param.snap b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_input_param.snap index 99a636d224..b330662581 100644 --- a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_input_param.snap +++ b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_input_param.snap @@ -25,7 +25,7 @@ entry: define void @main(%main* %0) { entry: %fb = getelementptr inbounds %main, %main* %0, i32 0, i32 0 - %1 = getelementptr inbounds %fb_t, %fb_t* %fb, i32 0, i32 0 + %1 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 0 store i32 1, i32* %1, align 4 call void @fb_t(%fb_t* %fb) ret void diff --git a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_output_param.snap b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_output_param.snap index d45989586b..dc39af0167 100644 --- a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_output_param.snap +++ b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__fb_accepts_empty_statement_as_output_param.snap @@ -27,7 +27,7 @@ entry: %fb = getelementptr inbounds %main, %main* %0, i32 0, i32 0 %x = getelementptr inbounds %main, %main* %0, i32 0, i32 1 call void @fb_t(%fb_t* %fb) - %1 = getelementptr inbounds %fb_t, %fb_t* %fb, i32 0, i32 0 + %1 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 0 %2 = load i32, i32* %1, align 4 store i32 %2, i32* %x, align 4 ret void diff --git a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__parameters_behind_function_block_pointer_are_assigned_to.snap b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__parameters_behind_function_block_pointer_are_assigned_to.snap index 51152f767a..cce83bd150 100644 --- a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__parameters_behind_function_block_pointer_are_assigned_to.snap +++ b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__parameters_behind_function_block_pointer_are_assigned_to.snap @@ -19,7 +19,7 @@ entry: %FileOpen = getelementptr inbounds %main, %main* %0, i32 0, i32 1 store %file_t* %file, %file_t** %FileOpen, align 8 %deref = load %file_t*, %file_t** %FileOpen, align 8 - %1 = getelementptr inbounds %file_t, %file_t* %deref, i32 0, i32 1 + %1 = getelementptr %file_t, %file_t* %deref, i32 0, i32 1 store i8 1, i8* %1, align 1 call void @file_t(%file_t* %deref) ret void diff --git a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__var_in_out_params_can_be_out_of_order.snap b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__var_in_out_params_can_be_out_of_order.snap index 7bbe91b29a..2ed3f57bf2 100644 --- a/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__var_in_out_params_can_be_out_of_order.snap +++ b/src/codegen/tests/snapshots/rusty__codegen__tests__parameters_tests__var_in_out_params_can_be_out_of_order.snap @@ -18,21 +18,25 @@ entry: %fb = getelementptr inbounds %mainProg, %mainProg* %0, i32 0, i32 0 %out1 = getelementptr inbounds %mainProg, %mainProg* %0, i32 0, i32 1 %out2 = getelementptr inbounds %mainProg, %mainProg* %0, i32 0, i32 2 - %1 = getelementptr inbounds %fb_t, %fb_t* %fb, i32 0, i32 4 + %1 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 4 store i8* %out1, i8** %1, align 8 - %2 = getelementptr inbounds %fb_t, %fb_t* %fb, i32 0, i32 2 + %2 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 2 store i8* %out2, i8** %2, align 8 call void @fb_t(%fb_t* %fb) - %3 = getelementptr inbounds %fb_t, %fb_t* %fb, i32 0, i32 2 + %3 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 2 store i8* %out1, i8** %3, align 8 - %4 = getelementptr inbounds %fb_t, %fb_t* %fb, i32 0, i32 4 + %4 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 4 store i8* %out2, i8** %4, align 8 call void @fb_t(%fb_t* %fb) - %load_out2 = load i8, i8* %out2, align 1 - %load_out1 = load i8, i8* %out1, align 1 + %5 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 4 + store i8* %out2, i8** %5, align 8 + %6 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 2 + store i8* %out1, i8** %6, align 8 call void @fb_t__foo(%fb_t* %fb) - %load_out21 = load i8, i8* %out2, align 1 - %load_out12 = load i8, i8* %out1, align 1 + %7 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 2 + store i8* %out2, i8** %7, align 8 + %8 = getelementptr %fb_t, %fb_t* %fb, i32 0, i32 4 + store i8* %out1, i8** %8, align 8 call void @fb_t__foo(%fb_t* %fb) ret void } diff --git a/src/index.rs b/src/index.rs index e9a5b41fe9..2fdf889317 100644 --- a/src/index.rs +++ b/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,6 +1551,37 @@ impl Index { }) } + // 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)) + } + + // Check if the POU has the member locally + if let Some(entry) = find(self, pou, name) { + return Some((entry, 0)); + } + + // ..and if not walk the inheritance chain and re-try + let mut depth = 1; + let mut current_pou = pou; + + 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)); + } + + depth += 1; + current_pou = parent; + } + + None + } + /// Searches for method names in the given container, if not found, attempts to search for it in super class pub fn find_method(&self, container_name: &str, method_name: &str) -> Option<&PouIndexEntry> { self.find_method_recursive(container_name, method_name, &mut FxHashSet::default()) @@ -1701,22 +1732,37 @@ 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::>() + /// 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)) } - pub fn has_variadic_parameter(&self, pou_name: &str) -> bool { - self.get_pou_members(pou_name).iter().any(|member| member.is_parameter() && member.is_variadic()) + /// 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(); + + // 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 } - /// 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 has_variadic_parameter(&self, pou_name: &str) -> bool { + self.get_pou_members(pou_name).iter().any(|member| member.is_parameter() && member.is_variadic()) } pub fn get_variadic_member(&self, pou_name: &str) -> Option<&VariableIndexEntry> { diff --git a/src/index/tests/index_tests.rs b/src/index/tests/index_tests.rs index 9545126540..4df9527490 100644 --- a/src/index/tests/index_tests.rs +++ b/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::>(); + assert_eq!(members, vec!["inA", "outA", "inoutA"]); + + let members = index.get_declared_parameters("FbB").iter().map(|var| &var.name).collect::>(); + assert_eq!(members, vec!["inA", "outA", "inoutA", "inB", "outB", "inoutB",]); + + let members = index.get_declared_parameters("methA").iter().map(|var| &var.name).collect::>(); + assert!(members.is_empty()); + + let members = index.get_declared_parameters("FbB.methB").iter().map(|var| &var.name).collect::>(); + assert_eq!(members, vec!["inB_meth"]); +} diff --git a/src/resolver.rs b/src/resolver.rs index 330d3d7c18..15f3e1b03e 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -292,22 +292,68 @@ impl TypeAnnotator<'_> { )) } - pub fn annotate_call_statement( - &mut self, - operator: &AstNode, - parameters_stmt: Option<&AstNode>, - ctx: &VisitorContext, - ) { - let parameters = if let Some(parameters) = parameters_stmt { - self.visit_statement(ctx, parameters); - flatten_expression_list(parameters) + pub fn annotate_arguments(&mut self, operator: &AstNode, arguments_node: &AstNode, ctx: &VisitorContext) { + self.visit_statement(ctx, arguments_node); + let arguments = flatten_expression_list(arguments_node); + + let pou_name = { + let name = self.get_call_name(operator); + let implementation = self.index.find_implementation_by_name(&name); + implementation.map(|it| it.get_type_name().to_string()).unwrap_or(name) + }; + + let generics = if arguments.iter().any(|arg| arg.is_assignment() | arg.is_output_assignment()) { + self.annotate_arguments_named(&pou_name, arguments) } else { - vec![] + self.annotate_arguments_positional(&pou_name, operator, arguments) }; - let mut generics_candidates: FxHashMap> = FxHashMap::default(); - let mut params = vec![]; - let mut parameters = if self.annotation_map.get(operator).is_some_and(|opt| opt.is_fnptr()) { + self.update_generic_call_statement(generics, &pou_name, operator, arguments_node, ctx.to_owned()); + } + + fn annotate_arguments_named( + &mut self, + pou_name: &str, + arguments: Vec<&AstNode>, + ) -> FxHashMap> { + let mut generics_candidates = FxHashMap::>::default(); + + for argument in arguments { + let Some(var_name) = argument.get_assignment_identifier() else { + continue; + }; + + let Some((parameter, depth)) = self.index.find_pou_member_and_depth(pou_name, var_name) else { + continue; + }; + + if let Some((key, candidate)) = self.get_generic_candidate(parameter.get_type_name(), argument) { + generics_candidates.entry(key.to_string()).or_default().push(candidate.to_string()); + continue; + } + + self.annotate_argument( + parameter.get_qualifier().expect("parameter must have a qualifier"), + argument, + parameter.get_type_name(), + depth, + parameter.get_position() as usize, + ); + } + + generics_candidates + } + + fn annotate_arguments_positional( + &mut self, + pou_name: &str, + operator: &AstNode, + arguments: Vec<&AstNode>, + ) -> FxHashMap> { + let mut generic_candidates: FxHashMap> = FxHashMap::default(); + let mut positional_candidates = Vec::new(); + + let mut arguments = if self.annotation_map.get(operator).is_some_and(|opt| opt.is_fnptr()) { // When dealing with a function pointer (which are only supported in the context of methods and // direct function block calls), the first argument will be a instance of the POU, e.g. // `fnPtrToMyFbEcho^(instanceFb)`, hence we must skip the first argument as otherwise the @@ -317,108 +363,78 @@ impl TypeAnnotator<'_> { // `DINT`. This then results in an error in the codegen. Somewhat "ugly" I have to admit and a // better approach would be to lower method calls from `fbInstance.echo('stringValue', 5)` to // `fbInstance.echo(fbInstance, 'stringValue', 5)` but this has to do for now - parameters[1..].iter() + arguments[1..].iter() } else { - parameters.iter() + arguments.iter() }; - // If we are dealing with an action call statement, we need to get the declared parameters from the parent POU in order - // to annotate them with the correct type hint. - let operator_qualifier = &self.get_call_name(operator); - let implementation = self.index.find_implementation_by_name(operator_qualifier); - let operator_qualifier = implementation.map(|it| it.get_type_name()).unwrap_or(operator_qualifier); - for m in self.index.get_declared_parameters(operator_qualifier).into_iter() { - if let Some(p) = parameters.next() { - let type_name = m.get_type_name(); - if let Some((key, candidate)) = - TypeAnnotator::get_generic_candidate(self.index, &self.annotation_map, type_name, p) - { - generics_candidates.entry(key.to_string()).or_default().push(candidate.to_string()) - } else { - params.push((p, type_name.to_string(), m.get_location_in_parent())) + // Zip the parameters together with the arguments, then link the correct type information to them. + for (parameter, argument) in self.index.get_declared_parameters(pou_name).iter().zip(&mut arguments) { + let parameter_type_name = parameter.get_type_name(); + + match self.get_generic_candidate(parameter_type_name, argument) { + Some((key, candidate)) => { + generic_candidates.entry(key.to_string()).or_default().push(candidate.to_string()); + } + + None => { + let candidate = (argument, parameter_type_name.to_string(), parameter.get_position()); + positional_candidates.push(candidate); } } } - //We possibly did not consume all parameters, see if the variadic arguments are derivable - match self.index.find_pou(operator_qualifier) { - Some(pou) if pou.is_variadic() => { - //get variadic argument type, if it is generic, update the generic candidates - if let Some(variadic) = self.index.get_variadic_member(pou.get_name()) { - let type_name = variadic.get_type_name(); - for parameter in parameters { - if let Some((key, candidate)) = TypeAnnotator::get_generic_candidate( - self.index, - &self.annotation_map, - type_name, - parameter, - ) { - generics_candidates - .entry(key.to_string()) - .or_default() - .push(candidate.to_string()) - } else { - // intrinsic type promotion for variadics in order to be compatible with the C standard. - // see ISO/IEC 9899:1999, 6.5.2.2 Function calls (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf) - // or https://en.cppreference.com/w/cpp/language/implicit_conversion#Integral_promotion - // for more about default argument promotion. - - // varargs without a type declaration will be annotated "VOID", so in order to check if a - // promotion is necessary, we need to first check the type of each parameter. in the case of numerical - // types, we promote if the type is smaller than double/i32 (except for booleans). - let type_name = if let Some(data_type) = - self.annotation_map.get_type(parameter, self.index) - { - match &data_type.information { - DataTypeInformation::Float { .. } => get_bigger_type( - data_type, - self.index.get_type_or_panic(LREAL_TYPE), - self.index, - ) - .get_name(), - DataTypeInformation::Integer { .. } - if !data_type.information.is_bool() - && !data_type.information.is_character() => - { - get_bigger_type( - data_type, - self.index.get_type_or_panic(DINT_TYPE), - self.index, - ) - .get_name() - } - _ => type_name, - } - } else { - // default to original type in case no type could be found - // and let the validator handle situations that might lead here - type_name - }; - - params.push(( - parameter, - type_name.to_string(), - variadic.get_location_in_parent(), - )); - } - } + // When dealing with variadic arguments, the previous zip will not have consumed all arguments because + // potentially we have more arguments than parameters. In that case, check if we are dealing with a + // variadic argument and if so, iterate over the remaining arguments. + if let Some(vararg) = self.index.get_variadic_member(pou_name) { + for argument in arguments { + if let Some((key, candidate)) = self.get_generic_candidate(vararg.get_type_name(), argument) { + generic_candidates.entry(key.to_string()).or_default().push(candidate.to_string()); + continue; } + + let type_name = self.get_vararg_type_name(argument, vararg); + positional_candidates.push((argument, type_name, vararg.get_position())); } - _ => {} } - for (p, name, position) in params { - self.annotate_parameters(p, &name, position as usize); + for (argument, type_name, position) in positional_candidates { + self.annotate_argument(pou_name, argument, &type_name, 0, position as usize); } - // Attempt to resolve the generic signature here - self.update_generic_call_statement( - generics_candidates, - operator_qualifier, - operator, - parameters_stmt, - ctx.to_owned(), - ); + generic_candidates + } + + fn get_vararg_type_name(&mut self, argument: &&AstNode, vararg: &VariableIndexEntry) -> String { + // intrinsic type promotion for variadics in order to be compatible with the C standard. + // see ISO/IEC 9899:1999, 6.5.2.2 Function calls (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf) + // or https://en.cppreference.com/w/cpp/language/implicit_conversion#Integral_promotion + // for more about default argument promotion. + // + // varargs without a type declaration will be annotated "VOID", so in order to check if a + // promotion is necessary, we need to first check the type of each parameter. in the case of numerical + // types, we promote if the type is smaller than double/i32 (except for booleans). + + let type_name = match self.annotation_map.get_type(argument, self.index) { + Some(data_type) => match &data_type.information { + DataTypeInformation::Float { .. } => { + get_bigger_type(data_type, self.index.get_type_or_panic(LREAL_TYPE), self.index) + .get_name() + } + DataTypeInformation::Integer { .. } + if !data_type.information.is_bool() && !data_type.information.is_character() => + { + get_bigger_type(data_type, self.index.get_type_or_panic(DINT_TYPE), self.index).get_name() + } + + _ => vararg.get_type_name(), + }, + + None => vararg.get_type_name(), + }; + + type_name.to_string() } } @@ -460,8 +476,25 @@ pub enum StatementAnnotation { /// The resulting type of this argument resulting_type: String, - /// The position of the parameter this argument is assigned to + /// The position of the parameter within its declared POU. position: usize, + + /// The depth between where the arguments parameter is declared versus where it is being called from. + /// + /// Given an inheritance chain `A <- B <- C` and `instanceC(inA := 1, inB := 2, inC := 3)`: + /// - `inA := 1` will have a depth of 2 (declared in grandparent A) + /// - `inB := 2` will have a depth of 1 (declared in parent B) + /// - `inC := 3` will have a depth of 0 (declared in C itself) + depth: usize, + + /// The POU name where this arguments parameter is declared, which may differ from the actual POU + /// being called. + /// + /// Given an inheritance chain `A <- B <- C` and `instanceC(inA := 1, inB := 2, inC := 3)`: + /// - `inA := 1` will have a POU name of `A` + /// - `inB := 2` will have a POU name of `B` + /// - `inC := 3` will have a POU name of `C` + pou: String, }, /// a reference that resolves to a declared variable (e.g. `a` --> `PLC_PROGRAM.a`) Variable { @@ -773,27 +806,6 @@ impl StatementAnnotation { } } - /// Returns the location of a parameter in some POU the argument is assigned to, for example - /// `foo(a, b, c)` will return `0` for `a`, `1` for `b` and `3` for c if `foo` has the following variable - /// blocks - /// ```text - /// VAR_INPUT - /// a, b : DINT; - /// END_VAR - /// VAR - /// placeholder: DINT; - /// END_VAR - /// VAR_INPUT - /// c : DINT; - /// END_VAR` - /// ``` - pub(crate) fn get_location_in_parent(&self) -> Option { - match self { - StatementAnnotation::Argument { position, .. } => Some(*position as u32), - _ => None, - } - } - pub fn is_fnptr(&self) -> bool { matches!(self, StatementAnnotation::FunctionPointer { .. }) } @@ -2268,9 +2280,9 @@ impl<'i> TypeAnnotator<'i> { if let Some(annotation) = builtins::get_builtin(&operator_qualifier).and_then(BuiltIn::get_annotation) { annotation(self, statement, operator, parameters_stmt, ctx.to_owned()); - } else { + } else if let Some(arguments) = parameters_stmt { //This is skipped for builtins that provide their own annotation-logic - self.annotate_call_statement(operator, parameters_stmt, &ctx); + self.annotate_arguments(operator, arguments, &ctx); }; match self.annotation_map.get(operator) { @@ -2336,17 +2348,23 @@ impl<'i> TypeAnnotator<'i> { operator_qualifier } - pub(crate) fn annotate_parameters(&mut self, p: &AstNode, type_name: &str, position: usize) { - if let Some(effective_member_type) = self.index.find_effective_type_by_name(type_name) { - //update the type hint - // self.annotation_map.annotate_type_hint(p, StatementAnnotation::value(effective_member_type.get_name())) - self.annotation_map.annotate_type_hint( - p, - StatementAnnotation::Argument { - resulting_type: effective_member_type.get_name().to_string(), - position, - }, - ); + fn annotate_argument( + &mut self, + pou_name: &str, + argument: &AstNode, + type_name: &str, + depth: usize, + position: usize, + ) { + if let Some(resulting_type) = self.index.find_effective_type_by_name(type_name) { + let annotation = StatementAnnotation::Argument { + resulting_type: resulting_type.get_name().to_string(), + position, + depth, + pou: pou_name.to_string(), + }; + + self.annotation_map.annotate_type_hint(argument, annotation); } } diff --git a/src/resolver/generics.rs b/src/resolver/generics.rs index d42d73e57a..551dcdbdfd 100644 --- a/src/resolver/generics.rs +++ b/src/resolver/generics.rs @@ -6,7 +6,7 @@ use rustc_hash::FxHashMap; use crate::{ builtins, codegen::generators::expression_generator::get_implicit_call_parameter, - index::{ArgumentType, Index, PouIndexEntry, VariableType}, + index::{ArgumentType, PouIndexEntry, VariableType}, resolver::AnnotationMap, typesystem::{ self, DataType, DataTypeInformation, StringEncoding, BOOL_TYPE, CHAR_TYPE, DATE_TYPE, REAL_TYPE, @@ -14,7 +14,7 @@ use crate::{ }, }; -use super::{AnnotationMapImpl, StatementAnnotation, TypeAnnotator, VisitorContext}; +use super::{StatementAnnotation, TypeAnnotator, VisitorContext}; #[derive(Debug)] pub struct GenericType { @@ -29,18 +29,13 @@ impl TypeAnnotator<'_> { /// determines a possible generic for the current statement /// returns a pair with the possible generics symbol and the real datatype /// e.g. `( "T", "INT" )` - pub(super) fn get_generic_candidate<'idx>( - index: &'idx Index, - annotation_map: &'idx AnnotationMapImpl, - type_name: &str, - statement: &AstNode, - ) -> Option<(&'idx str, &'idx str)> { + pub fn get_generic_candidate(&self, type_name: &str, statement: &AstNode) -> Option<(&str, &str)> { //find inner type if this was turned into an array or pointer (if this is `POINTER TO T` lets find out what T is) - let effective_type = index.find_effective_type_info(type_name); + let effective_type = self.index.find_effective_type_info(type_name); let candidate = match effective_type { Some(DataTypeInformation::Pointer { inner_type_name, .. }) | Some(DataTypeInformation::Array { inner_type_name, .. }) => { - index.find_effective_type_info(inner_type_name) + self.index.find_effective_type_info(inner_type_name) } _ => effective_type, }; @@ -53,7 +48,9 @@ impl TypeAnnotator<'_> { _ => statement, }; //Find the statement's type - annotation_map.get_type(statement, index).map(|it| (generic_symbol.as_str(), it.get_name())) + self.annotation_map + .get_type(statement, self.index) + .map(|it| (generic_symbol.as_str(), it.get_name())) } else { None } @@ -66,7 +63,7 @@ impl TypeAnnotator<'_> { generics_candidates: FxHashMap>, implementation_name: &str, operator: &AstNode, - parameters: Option<&AstNode>, + parameters: &AstNode, ctx: VisitorContext, ) { if let Some(PouIndexEntry::Function { generics, .. }) = self.index.find_pou(implementation_name) { @@ -114,10 +111,8 @@ impl TypeAnnotator<'_> { self.annotate(operator, annotation); } // Adjust annotations on the inner statement - if let Some(s) = parameters.as_ref() { - self.visit_statement(&ctx, s); - self.update_generic_function_parameters(s, implementation_name, generic_map); - } + self.visit_statement(&ctx, parameters); + self.update_generic_function_parameters(parameters, implementation_name, generic_map); } } } diff --git a/src/resolver/tests/resolve_expressions_tests.rs b/src/resolver/tests/resolve_expressions_tests.rs index fe5babceb4..83e17e95da 100644 --- a/src/resolver/tests/resolve_expressions_tests.rs +++ b/src/resolver/tests/resolve_expressions_tests.rs @@ -6042,6 +6042,8 @@ fn implicit_output_assignment_arguments_are_annotated() { Argument { resulting_type: "DINT", position: 1, + depth: 0, + pou: "fb", } "#); @@ -6049,6 +6051,8 @@ fn implicit_output_assignment_arguments_are_annotated() { Argument { resulting_type: "BOOL", position: 2, + depth: 0, + pou: "fb", } "#); } @@ -6108,6 +6112,8 @@ fn explicit_output_assignment_arguments_are_annotated() { Argument { resulting_type: "DINT", position: 1, + depth: 0, + pou: "QUUX", }, ) "#); @@ -6117,6 +6123,8 @@ fn explicit_output_assignment_arguments_are_annotated() { Argument { resulting_type: "BOOL", position: 3, + depth: 0, + pou: "QUUX", }, ) "#); @@ -6174,23 +6182,27 @@ fn program_call_declared_as_variable_is_annotated() { ) "###); - insta::assert_debug_snapshot!(annotations.get_hint(&expressions[0]), @r###" + insta::assert_debug_snapshot!(annotations.get_hint(&expressions[0]), @r#" Some( Argument { resulting_type: "DINT", position: 1, + depth: 0, + pou: "ridiculous_chaining", }, ) - "###); + "#); - insta::assert_debug_snapshot!(annotations.get_hint(&expressions[1]), @r###" + insta::assert_debug_snapshot!(annotations.get_hint(&expressions[1]), @r#" Some( Argument { resulting_type: "DINT", position: 2, + depth: 0, + pou: "ridiculous_chaining", }, ) - "###); + "#); } #[test] @@ -6473,3 +6485,258 @@ fn this_in_conditionals() { assert_eq!(resulting_type, "FB_Test.__THIS"); assert!(index.find_type("fb_test.__THIS").is_some()); } + +#[test] +fn named_arguments_are_annotated_with_location_and_depth() { + let id_provider = IdProvider::default(); + let (unit, mut index) = index_with_ids( + " + FUNCTION_BLOCK FbA + VAR + fillerA: DINT; + END_VAR + + VAR_INPUT + inA: DINT; + END_VAR + + VAR_OUTPUT + outA: DINT; + END_VAR + + VAR_IN_OUT + inoutA: DINT; + END_VAR + END_FUNCTION_BLOCK + + FUNCTION_BLOCK FbB EXTENDS FbA + VAR_INPUT + inB: DINT; + END_VAR + + VAR + fillerB: DINT; + END_VAR + + VAR_OUTPUT + outB: DINT; + END_VAR + + VAR_IN_OUT + inoutB: DINT; + END_VAR + END_FUNCTION_BLOCK + + FUNCTION_BLOCK FbC EXTENDS FbB + VAR_INPUT + inC: DINT; + END_VAR + + VAR_OUTPUT + outC: DINT; + END_VAR + + VAR + fillerC: DINT; + END_VAR + + VAR_IN_OUT + inoutC: DINT; + END_VAR + END_FUNCTION_BLOCK + + FUNCTION_BLOCK FbD EXTENDS FbC + VAR_INPUT + inD: DINT; + END_VAR + + VAR_OUTPUT + outD: DINT; + END_VAR + + VAR_IN_OUT + inoutD: DINT; + END_VAR + + VAR + fillerD: DINT; + END_VAR + END_FUNCTION_BLOCK + + FUNCTION main + VAR + instanceD: FbD; + localOutVar: DINT; + localInOutVar: DINT; + END_VAR + + instanceD( + inA := 1, + outA => localOutVar, + inoutA := localInOutVar, + inB := 2, + outB => localOutVar, + inoutB := localInOutVar, + inC := 3, + outC => localOutVar, + inoutC := localInOutVar, + inD := 4, + outD => localOutVar, + inoutD := localInOutVar, + ); + END_FUNCTION + ", + id_provider.clone(), + ); + + let annotations = annotate_with_ids(&unit, &mut index, id_provider); + let main = &unit.implementations.iter().find(|imp| imp.name == "main").unwrap(); + + // instanceD(inA := 1, outA => localOutVar, ..., outD => localOutVar, inoutD := localInOutVar); + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + let AstStatement::CallStatement(CallStatement { parameters, .. }) = main.statements[0].get_stmt() else { + unreachable!() + }; + + let AstStatement::ExpressionList(args) = ¶meters.as_ref().unwrap().as_ref().stmt else { + unreachable!() + }; + + let tuple = args.iter().map(|arg| (arg.as_string(), annotations.get_hint(arg))).collect::>(); + insta::assert_debug_snapshot!(tuple, @r#" + [ + ( + "inA := 1", + Some( + Argument { + resulting_type: "DINT", + position: 1, + depth: 3, + pou: "FbA", + }, + ), + ), + ( + "outA => localOutVar", + Some( + Argument { + resulting_type: "DINT", + position: 2, + depth: 3, + pou: "FbA", + }, + ), + ), + ( + "inoutA := localInOutVar", + Some( + Argument { + resulting_type: "__auto_pointer_to_DINT", + position: 3, + depth: 3, + pou: "FbA", + }, + ), + ), + ( + "inB := 2", + Some( + Argument { + resulting_type: "DINT", + position: 0, + depth: 2, + pou: "FbB", + }, + ), + ), + ( + "outB => localOutVar", + Some( + Argument { + resulting_type: "DINT", + position: 2, + depth: 2, + pou: "FbB", + }, + ), + ), + ( + "inoutB := localInOutVar", + Some( + Argument { + resulting_type: "__auto_pointer_to_DINT", + position: 3, + depth: 2, + pou: "FbB", + }, + ), + ), + ( + "inC := 3", + Some( + Argument { + resulting_type: "DINT", + position: 0, + depth: 1, + pou: "FbC", + }, + ), + ), + ( + "outC => localOutVar", + Some( + Argument { + resulting_type: "DINT", + position: 1, + depth: 1, + pou: "FbC", + }, + ), + ), + ( + "inoutC := localInOutVar", + Some( + Argument { + resulting_type: "__auto_pointer_to_DINT", + position: 3, + depth: 1, + pou: "FbC", + }, + ), + ), + ( + "inD := 4", + Some( + Argument { + resulting_type: "DINT", + position: 0, + depth: 0, + pou: "FbD", + }, + ), + ), + ( + "outD => localOutVar", + Some( + Argument { + resulting_type: "DINT", + position: 1, + depth: 0, + pou: "FbD", + }, + ), + ), + ( + "inoutD := localInOutVar", + Some( + Argument { + resulting_type: "__auto_pointer_to_DINT", + position: 2, + depth: 0, + pou: "FbD", + }, + ), + ), + ] + "#); +} diff --git a/src/tests/adr/annotated_ast_adr.rs b/src/tests/adr/annotated_ast_adr.rs index bf5fa34e80..5292a61a44 100644 --- a/src/tests/adr/annotated_ast_adr.rs +++ b/src/tests/adr/annotated_ast_adr.rs @@ -263,7 +263,12 @@ fn type_annotations_indicates_necessary_casts() { // 3 is a DINT but hinted as an INT because foo expects an INT as first parameter assert_eq!( annotations.get_hint(parameters[0]).unwrap(), - &StatementAnnotation::Argument { resulting_type: "INT".to_string(), position: 0 } + &StatementAnnotation::Argument { + resulting_type: "INT".to_string(), + position: 0, + depth: 0, + pou: "foo".to_string() + } ); } diff --git a/src/tests/adr/pou_adr.rs b/src/tests/adr/pou_adr.rs index ac1ef8c706..deeb8a814d 100644 --- a/src/tests/adr/pou_adr.rs +++ b/src/tests/adr/pou_adr.rs @@ -354,12 +354,12 @@ fn calling_a_function_block() { %x = getelementptr inbounds %foo, %foo* %0, i32 0, i32 0 %y = getelementptr inbounds %foo, %foo* %0, i32 0, i32 1 %fb = getelementptr inbounds %foo, %foo* %0, i32 0, i32 2 - %1 = getelementptr inbounds %main_fb, %main_fb* %fb, i32 0, i32 0 + %1 = getelementptr %main_fb, %main_fb* %fb, i32 0, i32 0 store i16 1, i16* %1, align 2 - %2 = getelementptr inbounds %main_fb, %main_fb* %fb, i32 0, i32 1 + %2 = getelementptr %main_fb, %main_fb* %fb, i32 0, i32 1 store i16* %y, i16** %2, align 8 call void @main_fb(%main_fb* %fb) - %3 = getelementptr inbounds %main_fb, %main_fb* %fb, i32 0, i32 2 + %3 = getelementptr %main_fb, %main_fb* %fb, i32 0, i32 2 %4 = load i16, i16* %3, align 2 store i16 %4, i16* %x, align 2 ret void diff --git a/src/typesystem.rs b/src/typesystem.rs index dd9d8f3de3..fe9bc78251 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -237,7 +237,7 @@ impl DataType { members .iter() .filter(|item| item.is_parameter() && !item.is_variadic()) - .find(|member| member.get_location_in_parent() == location) + .find(|member| member.get_position() == location) } else { None } diff --git a/src/validation/statement.rs b/src/validation/statement.rs index 9b61ab51fb..e84b28809c 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -1504,7 +1504,7 @@ fn validate_call( validate_call_by_ref(validator, left, argument); // 'parameter location in parent' and 'variable location in parent' are not the same (e.g VAR blocks are not counted as param). // save actual location in parent for InOut validation - variable_location_in_parent.push(left.get_location_in_parent()); + variable_location_in_parent.push(left.get_position()); } // explicit call parameter assignments will be handled by @@ -1553,7 +1553,7 @@ fn validate_call( if !declared_in_out_params.is_empty() { // Check if all IN_OUT arguments were passed by cross-checking with the parameters declared_in_out_params.into_iter().for_each(|p| { - if !variable_location_in_parent.contains(&p.get_location_in_parent()) { + if !variable_location_in_parent.contains(&p.get_position()) { validator.push_diagnostic( Diagnostic::new(format!("Argument `{}` is missing", p.get_name())) .with_error_code("E030") diff --git a/src/validation/tests/generic_validation_tests.rs b/src/validation/tests/generic_validation_tests.rs index 1be22f3c99..840a1e3dbf 100644 --- a/src/validation/tests/generic_validation_tests.rs +++ b/src/validation/tests/generic_validation_tests.rs @@ -1627,5 +1627,29 @@ fn generic_call_with_formal_parameter() { "; let diagnostics = parse_and_validate_buffered(src); - insta::assert_snapshot!(diagnostics); + insta::assert_snapshot!(diagnostics, @r" + error[E089]: Invalid call parameters + ┌─ :20:30 + │ + 20 │ myLocalNumber := FOO(y := 0); // unresolved reference + │ ^^^^^^ Invalid call parameters + + error[E048]: Could not resolve reference to y + ┌─ :20:30 + │ + 20 │ myLocalNumber := FOO(y := 0); // unresolved reference + │ ^ Could not resolve reference to y + + error[E062]: Invalid type nature for generic argument. __STRING_19 is no ANY_NUMBER + ┌─ :21:35 + │ + 21 │ myLocalNumber := FOO(x := 'INVALID TYPE NATURE'); // invalid type nature + │ ^^^^^^^^^^^^^^^^^^^^^ Invalid type nature for generic argument. __STRING_19 is no ANY_NUMBER + + error[E037]: Invalid assignment: cannot assign 'STRING' to 'USINT' + ┌─ :21:30 + │ + 21 │ myLocalNumber := FOO(x := 'INVALID TYPE NATURE'); // invalid type nature + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment: cannot assign 'STRING' to 'USINT' + "); } diff --git a/src/validation/tests/snapshots/rusty__validation__tests__generic_validation_tests__generic_call_with_formal_parameter.snap b/src/validation/tests/snapshots/rusty__validation__tests__generic_validation_tests__generic_call_with_formal_parameter.snap index 5150b6dd4f..f7aafca615 100644 --- a/src/validation/tests/snapshots/rusty__validation__tests__generic_validation_tests__generic_call_with_formal_parameter.snap +++ b/src/validation/tests/snapshots/rusty__validation__tests__generic_validation_tests__generic_call_with_formal_parameter.snap @@ -25,5 +25,3 @@ error[E037]: Invalid assignment: cannot assign 'STRING' to 'USINT' │ 21 │ myLocalNumber := FOO(x := 'INVALID TYPE NATURE'); // invalid type nature │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment: cannot assign 'STRING' to 'USINT' - - diff --git a/src/validation/tests/statement_validation_tests.rs b/src/validation/tests/statement_validation_tests.rs index 89199eaa65..c60bc4b61c 100644 --- a/src/validation/tests/statement_validation_tests.rs +++ b/src/validation/tests/statement_validation_tests.rs @@ -891,6 +891,12 @@ fn builtin_functions_named_arguments_invalid_parameter_names() { 14 │ arr2 := MOVE(SOURCE := arr); │ ^^^^^^ Could not resolve reference to SOURCE + error[E037]: Invalid assignment: cannot assign 'SEL with wrong parameter names a := SEL(WRONG := sel, IN0 := a, IN1 := b); a := SEL(G := sel, INVALID := a,' to 'ARRAY[0..5] OF INT' + ┌─ :14:13 + │ + 14 │ arr2 := MOVE(SOURCE := arr); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment: cannot assign 'SEL with wrong parameter names a := SEL(WRONG := sel, IN0 := a, IN1 := b); a := SEL(G := sel, INVALID := a,' to 'ARRAY[0..5] OF INT' + error[E089]: Invalid call parameters ┌─ :17:25 │ diff --git a/tests/correctness/functions.rs b/tests/correctness/functions.rs index 4a4f09ec33..e6b0b3d20d 100644 --- a/tests/correctness/functions.rs +++ b/tests/correctness/functions.rs @@ -436,7 +436,7 @@ fn var_output_assignment() { var1 : DINT; var2 : DINT; END_VAR - foo(7, 8, output1 => var1, output2 => var2); + foo(input1 := 7, input2 := 8, output1 => var1, output2 => var2); END_PROGRAM "#; diff --git a/tests/lit/single/oop/fb_direct_calls.st b/tests/lit/single/oop/fb_direct_calls.st new file mode 100644 index 0000000000..b69476ec1d --- /dev/null +++ b/tests/lit/single/oop/fb_direct_calls.st @@ -0,0 +1,115 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s + +TYPE Position2D: + STRUCT + x: DINT; + y: DINT; + END_STRUCT +END_TYPE + +TYPE Position3D: + STRUCT + x: DINT; + y: DINT; + z: DINT; + END_STRUCT +END_TYPE + +FUNCTION_BLOCK FbA + VAR + placeholderA: STRING; + END_VAR + + VAR_INPUT + x: DINT; + y: DINT; + END_VAR + + VAR_OUTPUT + result2D: Position2D; + END_VAR + + result2D.x := x; + result2D.y := y; +END_FUNCTION_BLOCK + +FUNCTION_BLOCK FbB EXTENDS FbA + VAR_INPUT + z: DINT; + END_VAR + + VAR_OUTPUT + result3D: Position3D; + END_VAR + + // using the values from FbA + result3D.x := x; + result3D.y := y; + result3D.z := z; +END_FUNCTION_BLOCK + +FUNCTION_BLOCK FbC EXTENDS FbB + VAR_INPUT + multiplier2D: DINT; + multiplier3D: DINT; + END_VAR + + VAR + placeholderC1: STRING; + placeholderC2: INT; + END_VAR + + VAR_IN_OUT + resultMultiplied2D: Position2D; + resultMultiplied3D: Position3D; + END_VAR + + resultMultiplied2D.x := x * multiplier2D; + resultMultiplied2D.y := y * multiplier2D; + + resultMultiplied3D.x := x * multiplier3D; + resultMultiplied3D.y := y * multiplier3D; + resultMultiplied3D.z := z * multiplier3D; +END_FUNCTION_BLOCK + +FUNCTION main + VAR + instanceA: FbA; + instanceB: FbB; + instanceC: FbC; + + pos2D: Position2D; + pos3D: Position3D; + END_VAR + + // CHECK: Position2D(10, 20) + instanceA( + x := 10, + result2D => pos2D, + y := 20, + ); + printf('Position2D(%d, %d)$N', pos2D.x, pos2D.y); + + // CHECK-NEXT: Position3D(40, 50, 60) + instanceB( + z := 60, + y := 50, + result3D => pos3D, + x := 40, + ); + printf('Position3D(%d, %d, %d)$N', pos3D.x, pos3D.y, pos3D.z); + + // CHECK-NEXT: Position2D(140, 160) + // CHECK-NEXT: Position3D(280, 320, 360) + instanceC( + multiplier2D := 2, + z := 90, + x := 70, + resultMultiplied3D := pos3D, + y := 80, + multiplier3D := 4, + resultMultiplied2D := pos2D, + ); + printf('Position2D(%d, %d)$N', pos2D.x, pos2D.y); + printf('Position3D(%d, %d, %d)$N', pos3D.x, pos3D.y, pos3D.z); +END_FUNCTION \ No newline at end of file