diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 364c0e761c..bd498cf64a 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -553,7 +553,7 @@ fn replace_reference( Some(*old_data_type) } -#[derive(Clone, PartialEq, Debug)] +#[derive(Debug, Clone, PartialEq)] pub enum ReferenceAccess { /** * a, a.b diff --git a/compiler/plc_ast/src/literals.rs b/compiler/plc_ast/src/literals.rs index 3cc8942a5f..d5b1eb3bde 100644 --- a/compiler/plc_ast/src/literals.rs +++ b/compiler/plc_ast/src/literals.rs @@ -38,14 +38,14 @@ pub enum AstLiteral { Array(Array), } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct Date { year: i32, month: u32, day: u32, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct DateAndTime { year: i32, month: u32, @@ -56,7 +56,7 @@ pub struct DateAndTime { nano: u32, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct TimeOfDay { hour: u32, min: u32, @@ -64,7 +64,7 @@ pub struct TimeOfDay { nano: u32, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct Time { pub day: f64, pub hour: f64, @@ -76,13 +76,13 @@ pub struct Time { pub negative: bool, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct StringValue { pub value: String, pub is_wide: bool, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct Array { pub elements: Option>, // expression-list } diff --git a/compiler/plc_diagnostics/src/diagnostics.rs b/compiler/plc_diagnostics/src/diagnostics.rs index 25783c20f3..7f0593819a 100644 --- a/compiler/plc_diagnostics/src/diagnostics.rs +++ b/compiler/plc_diagnostics/src/diagnostics.rs @@ -664,6 +664,14 @@ impl Diagnostic { } } + pub fn array_struct_assignment(range: SourceLocation) -> Diagnostic { + Diagnostic::SyntaxError { + message: "Struct initializers within arrays have to be wrapped by `()`".to_string(), + range: vec![range], + err_no: ErrNo::arr__invalid_array_assignment, + } + } + pub fn array_size(name: &str, len_lhs: usize, len_rhs: usize, range: SourceLocation) -> Diagnostic { Diagnostic::SemanticError { message: format!("Array {name} has a size of {len_lhs}, but {len_rhs} elements were provided"), diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs index 5052bd97c2..5ab8f746ed 100644 --- a/src/codegen/generators/expression_generator.rs +++ b/src/codegen/generators/expression_generator.rs @@ -1774,6 +1774,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { AstStatement::MultipliedStatement { .. } => { self.generate_literal_array(literal_statement).map(ExpressionValue::RValue) } + AstStatement::ParenExpression(expr) => self.generate_literal(expr), // if there is an expression-list this might be a struct-initialization or array-initialization AstStatement::ExpressionList { .. } => { let type_hint = self.get_type_hint_info_for(literal_statement)?; diff --git a/src/parser/tests/snapshots/rusty__parser__tests__expressions_parser_tests__additon_of_two_variables_parsed.snap b/src/parser/tests/snapshots/rusty__parser__tests__expressions_parser_tests__additon_of_two_variables_parsed.snap deleted file mode 100644 index 5c250ad768..0000000000 --- a/src/parser/tests/snapshots/rusty__parser__tests__expressions_parser_tests__additon_of_two_variables_parsed.snap +++ /dev/null @@ -1,141 +0,0 @@ ---- -source: src/parser/tests/expressions_parser_tests.rs -expression: statement ---- -[ - BinaryExpression { - operator: Plus, - left: ReferenceExpr { - kind: Member( - Reference { - name: "x", - }, - ), - base: None, - }, - right: ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: None, - }, - }, - BinaryExpression { - operator: Equal, - left: ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "x", - }, - ), - base: None, - }, - ), - }, - right: ReferenceExpr { - kind: Member( - Reference { - name: "z", - }, - ), - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: None, - }, - ), - }, - }, - BinaryExpression { - operator: Minus, - left: ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "x", - }, - ), - base: None, - }, - ), - }, - right: ReferenceExpr { - kind: Member( - Reference { - name: "z", - }, - ), - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: None, - }, - ), - }, - }, - BinaryExpression { - operator: Equal, - left: ReferenceExpr { - kind: Address, - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "x", - }, - ), - base: None, - }, - ), - }, - ), - }, - right: ReferenceExpr { - kind: Member( - Reference { - name: "z", - }, - ), - base: Some( - ReferenceExpr { - kind: Member( - Reference { - name: "y", - }, - ), - base: None, - }, - ), - }, - }, -] diff --git a/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_recursive_function_call.snap b/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_recursive_function_call.snap deleted file mode 100644 index 73acc3ce37..0000000000 --- a/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_recursive_function_call.snap +++ /dev/null @@ -1,86 +0,0 @@ ---- -source: src/resolver/tests/resolve_expressions_tests.rs -expression: annotated_types ---- -{ - 1: Function { - return_type: "DINT", - qualified_name: "foo", - call_name: None, - }, - 3: Variable { - resulting_type: "DINT", - qualified_name: "foo.var1", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, - 2: Variable { - resulting_type: "DINT", - qualified_name: "foo.input1", - constant: false, - argument_type: ByVal( - Input, - ), - is_auto_deref: false, - }, - 6: Variable { - resulting_type: "DINT", - qualified_name: "foo.var2", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, - 5: Variable { - resulting_type: "DINT", - qualified_name: "foo.inout1", - constant: false, - argument_type: ByRef( - InOut, - ), - is_auto_deref: true, - }, - 8: Variable { - resulting_type: "DINT", - qualified_name: "foo.output1", - constant: false, - argument_type: ByRef( - Output, - ), - is_auto_deref: true, - }, - 9: Variable { - resulting_type: "DINT", - qualified_name: "foo.var3", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, - 12: Value { - resulting_type: "DINT", - }, - 14: Variable { - resulting_type: "DINT", - qualified_name: "foo.var1", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, - 13: Variable { - resulting_type: "DINT", - qualified_name: "foo.foo", - constant: false, - argument_type: ByVal( - Return, - ), - is_auto_deref: false, - }, -} diff --git a/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_recursive_program_call.snap b/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_recursive_program_call.snap deleted file mode 100644 index 520e48e507..0000000000 --- a/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_recursive_program_call.snap +++ /dev/null @@ -1,63 +0,0 @@ ---- -source: src/resolver/tests/resolve_expressions_tests.rs -expression: annotated_types ---- -{ - 1: Program { - qualified_name: "mainProg", - }, - 3: Variable { - resulting_type: "DINT", - qualified_name: "mainProg.var1", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, - 2: Variable { - resulting_type: "DINT", - qualified_name: "mainProg.input1", - constant: false, - argument_type: ByVal( - Input, - ), - is_auto_deref: false, - }, - 6: Variable { - resulting_type: "DINT", - qualified_name: "mainProg.var2", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, - 5: Variable { - resulting_type: "DINT", - qualified_name: "mainProg.inout1", - constant: false, - argument_type: ByRef( - InOut, - ), - is_auto_deref: true, - }, - 8: Variable { - resulting_type: "DINT", - qualified_name: "mainProg.output1", - constant: false, - argument_type: ByVal( - Output, - ), - is_auto_deref: false, - }, - 9: Variable { - resulting_type: "DINT", - qualified_name: "mainProg.var3", - constant: false, - argument_type: ByVal( - Local, - ), - is_auto_deref: false, - }, -} diff --git a/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_return_variable_in_nested_call.snap b/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_return_variable_in_nested_call.snap deleted file mode 100644 index 287e792b44..0000000000 --- a/src/resolver/tests/snapshots/rusty__resolver__tests__resolve_expressions_tests__resolve_return_variable_in_nested_call.snap +++ /dev/null @@ -1,32 +0,0 @@ ---- -source: src/resolver/tests/resolve_expressions_tests.rs -expression: codegen(src) ---- -; ModuleID = 'main' -source_filename = "main" - -define i32 @main() { -entry: - %main = alloca i32, align 4 - %x1 = alloca i32, align 4 - %x2 = alloca i32, align 4 - store i32 0, i32* %x1, align 4 - store i32 0, i32* %x2, align 4 - store i32 0, i32* %main, align 4 - %0 = ptrtoint i32* %main to i64 - %call = call i32 @SMC_Read(i64 %0) - store i32 %call, i32* %x1, align 4 - %main_ret = load i32, i32* %main, align 4 - ret i32 %main_ret -} - -define i32 @SMC_Read(i64 %0) { -entry: - %SMC_Read = alloca i32, align 4 - %ValAddr = alloca i64, align 8 - store i64 %0, i64* %ValAddr, align 4 - store i32 0, i32* %SMC_Read, align 4 - %SMC_Read_ret = load i32, i32* %SMC_Read, align 4 - ret i32 %SMC_Read_ret -} - diff --git a/src/validation/array.rs b/src/validation/array.rs index cfcdebea75..e5a484a670 100644 --- a/src/validation/array.rs +++ b/src/validation/array.rs @@ -26,41 +26,70 @@ pub(super) enum Wrapper<'a> { Variable(&'a Variable), } -pub(super) fn validate_array_assignment( +pub(super) fn validate_array_assignment( validator: &mut Validator, context: &ValidationContext, wrapper: Wrapper, -) where - T: AnnotationMap, -{ - let Some(dti_lhs) = wrapper.datatype_info_lhs(context) else { return }; - let Some(stmt_rhs) = wrapper.get_rhs() else { return }; +) { + let Some(lhs_type) = wrapper.datatype_info_lhs(context) else { return }; + let Some(rhs_stmt) = wrapper.get_rhs() else { return }; - if !dti_lhs.is_array() { + if !lhs_type.is_array() { return; } + validate_array(validator, context, lhs_type, rhs_stmt); + validate_array_of_structs(validator, context, lhs_type, rhs_stmt); +} + +fn validate_array( + validator: &mut Validator, + context: &ValidationContext, + lhs_type: &DataTypeInformation, + rhs_stmt: &AstNode, +) { + let stmt_rhs = peel(rhs_stmt); let stmt_rhs = peel(stmt_rhs); if !(stmt_rhs.is_literal_array() || stmt_rhs.is_reference()) { validator.push_diagnostic(Diagnostic::array_assignment(stmt_rhs.get_location())); return; // Return here, because array size validation is error-prone with incorrect assignments } - let len_lhs = dti_lhs.get_array_length(context.index).unwrap_or(0); + let len_lhs = lhs_type.get_array_length(context.index).unwrap_or(0); let len_rhs = statement_to_array_length(stmt_rhs); if len_lhs < len_rhs { - let name = dti_lhs.get_name(); + let name = lhs_type.get_name(); let location = stmt_rhs.get_location(); validator.push_diagnostic(Diagnostic::array_size(name, len_lhs, len_rhs, location)); } } +fn validate_array_of_structs( + validator: &mut Validator, + context: &ValidationContext, + lhs_type: &DataTypeInformation, + rhs_stmt: &AstNode, +) { + let Some(array_type_name) = lhs_type.get_inner_array_type_name() else { return }; + let Some(dti) = context.index.find_effective_type_by_name(array_type_name) else { return }; + + if dti.is_struct() { + let AstStatement::Literal(AstLiteral::Array(array)) = rhs_stmt.get_stmt() else { return }; + let Some(AstStatement::ExpressionList(expressions)) = array.elements().map(AstNode::get_stmt) else { return }; + + for invalid in expressions.iter().filter(|it| !it.is_paren()) { + validator.push_diagnostic(Diagnostic::array_struct_assignment(invalid.get_location())); + } + } +} + /// Takes an [`AstStatementKind`] and returns its length as if it was an array. For example calling this function /// on an expression-list such as `[(...), (...)]` would return 2. fn statement_to_array_length(statement: &AstNode) -> usize { match statement.get_stmt() { AstStatement::ExpressionList { .. } => 1, + AstStatement::ParenExpression(_) => 1, AstStatement::MultipliedStatement(data) => data.multiplier as usize, AstStatement::Literal(AstLiteral::Array(arr)) => match arr.elements() { Some(AstNode { stmt: AstStatement::ExpressionList(expressions), .. }) => { diff --git a/src/validation/tests/array_validation_test.rs b/src/validation/tests/array_validation_test.rs index 320fbfcf99..93700c4df9 100644 --- a/src/validation/tests/array_validation_test.rs +++ b/src/validation/tests/array_validation_test.rs @@ -296,3 +296,26 @@ fn assignment_multiplied_statement() { assert_snapshot!(diagnostics); } + +#[test] +fn parenthesized_struct_initializers() { + let diagnostics = parse_and_validate_buffered( + " + TYPE foo : STRUCT + idx : DINT; + val : DINT; + END_STRUCT END_TYPE + + FUNCTION main : DINT + VAR + foo_valid : ARRAY[1..2] OF foo := [(idx := 0, val := 0), (idx := 1, val := 1)]; + foo_invalid_a : ARRAY[1..2] OF foo := [idx := 0, val := 0, idx := 1, val := 1]; // Both initializers missing parens + foo_invalid_b : ARRAY[1..2] OF foo := [idx := 0, val := 0, (idx := 1, val := 1)]; // First initializer missing parens + foo_invalid_c : ARRAY[1..2] OF foo := [(idx := 0, val := 0), idx := 1, val := 1]; // Second initializer missing parens + END_VAR + END_FUNCTION + ", + ); + + assert_snapshot!(diagnostics); +} diff --git a/src/validation/tests/reference_resolve_tests.rs b/src/validation/tests/reference_resolve_tests.rs index dcd99911dc..b97c03aca4 100644 --- a/src/validation/tests/reference_resolve_tests.rs +++ b/src/validation/tests/reference_resolve_tests.rs @@ -305,7 +305,7 @@ fn resolve_array_of_struct_as_member_of_another_struct_initializer() { " PROGRAM mainProg VAR - var_str1 : STRUCT1 := (myArr := [x1 := FALSE, x2 := TRUE]); + var_str1 : STRUCT1 := (myArr := [(x1 := FALSE, x2 := TRUE)]); END_VAR END_PROGRAM @@ -359,5 +359,5 @@ fn array_of_struct_as_member_of_another_struct_and_variable_declaration_is_initi ", ); - assert_eq!(diagnostics, vec![]); + assert!(diagnostics.is_empty()); } diff --git a/src/validation/tests/snapshots/rusty__validation__tests__array_validation_test__parenthesized_struct_initializers.snap b/src/validation/tests/snapshots/rusty__validation__tests__array_validation_test__parenthesized_struct_initializers.snap new file mode 100644 index 0000000000..6ecd65b7f3 --- /dev/null +++ b/src/validation/tests/snapshots/rusty__validation__tests__array_validation_test__parenthesized_struct_initializers.snap @@ -0,0 +1,53 @@ +--- +source: src/validation/tests/array_validation_test.rs +expression: diagnostics +--- +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :10:56 + │ +10 │ foo_invalid_a : ARRAY[1..2] OF foo := [idx := 0, val := 0, idx := 1, val := 1]; // Both initializers missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :10:66 + │ +10 │ foo_invalid_a : ARRAY[1..2] OF foo := [idx := 0, val := 0, idx := 1, val := 1]; // Both initializers missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :10:76 + │ +10 │ foo_invalid_a : ARRAY[1..2] OF foo := [idx := 0, val := 0, idx := 1, val := 1]; // Both initializers missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :10:86 + │ +10 │ foo_invalid_a : ARRAY[1..2] OF foo := [idx := 0, val := 0, idx := 1, val := 1]; // Both initializers missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :11:56 + │ +11 │ foo_invalid_b : ARRAY[1..2] OF foo := [idx := 0, val := 0, (idx := 1, val := 1)]; // First initializer missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :11:66 + │ +11 │ foo_invalid_b : ARRAY[1..2] OF foo := [idx := 0, val := 0, (idx := 1, val := 1)]; // First initializer missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :12:78 + │ +12 │ foo_invalid_c : ARRAY[1..2] OF foo := [(idx := 0, val := 0), idx := 1, val := 1]; // Second initializer missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + +error: Struct initializers within arrays have to be wrapped by `()` + ┌─ :12:88 + │ +12 │ foo_invalid_c : ARRAY[1..2] OF foo := [(idx := 0, val := 0), idx := 1, val := 1]; // Second initializer missing parens + │ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()` + + diff --git a/tests/integration/snapshots/tests__integration__cfc__codegen__variable_source_to_variable_and_block_sink.snap b/tests/integration/snapshots/tests__integration__cfc__codegen__variable_source_to_variable_and_block_sink.snap deleted file mode 100644 index 3ad3d6af33..0000000000 --- a/tests/integration/snapshots/tests__integration__cfc__codegen__variable_source_to_variable_and_block_sink.snap +++ /dev/null @@ -1,50 +0,0 @@ ---- -source: tests/integration/cfc.rs -expression: output_file_content_without_headers.join(LINE_ENDING) ---- -define i32 @main() { -entry: - %main = alloca i32, align 4 - %value = alloca i32, align 4 - store i32 2, i32* %value, align 4 - store i32 0, i32* %main, align 4 - %load_value = load i32, i32* %value, align 4 - %call = call i32 @myConnection(i32 %load_value) - store i32 %call, i32* %main, align 4 - %main_ret = load i32, i32* %main, align 4 - ret i32 %main_ret -} - -define i32 @myAdd(i32 %0, i32 %1) { -entry: - %myAdd = alloca i32, align 4 - %a = alloca i32, align 4 - store i32 %0, i32* %a, align 4 - %b = alloca i32, align 4 - store i32 %1, i32* %b, align 4 - store i32 0, i32* %myAdd, align 4 - %load_a = load i32, i32* %a, align 4 - %load_b = load i32, i32* %b, align 4 - %tmpVar = add i32 %load_a, %load_b - store i32 %tmpVar, i32* %myAdd, align 4 - %myAdd_ret = load i32, i32* %myAdd, align 4 - ret i32 %myAdd_ret -} - -define i32 @myConnection(i32 %0) { -entry: - %myConnection = alloca i32, align 4 - %x = alloca i32, align 4 - store i32 %0, i32* %x, align 4 - %y = alloca i32, align 4 - store i32 0, i32* %y, align 4 - store i32 0, i32* %myConnection, align 4 - %load_x = load i32, i32* %x, align 4 - store i32 %load_x, i32* %y, align 4 - %load_y = load i32, i32* %y, align 4 - %load_x1 = load i32, i32* %x, align 4 - %call = call i32 @myAdd(i32 %load_y, i32 %load_x1) - store i32 %call, i32* %myConnection, align 4 - %myConnection_ret = load i32, i32* %myConnection, align 4 - ret i32 %myConnection_ret -}