From 91f485f4d35cecf00ab5505ef3c62264e7e32413 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Wed, 19 Jun 2024 17:15:36 +0200 Subject: [PATCH 01/20] =?UTF-8?q?Initial=20commit=20w/=20segfault=20?= =?UTF-8?q?=F0=9F=98=8E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- compiler/plc_ast/src/ast.rs | 21 ++++++++++++ compiler/plc_ast/src/visitor.rs | 1 + src/codegen/generators/statement_generator.rs | 24 +++++++++++++- src/codegen/tests/statement_codegen_test.rs | 32 +++++++++++++++++++ src/lexer/tokens.rs | 3 ++ src/parser/expressions_parser.rs | 20 ++++++++---- src/parser/tests/statement_parser_tests.rs | 29 ++++++++++++++++- src/resolver.rs | 5 +-- tests/correctness/pointers.rs | 18 +++++++++++ 9 files changed, 143 insertions(+), 10 deletions(-) diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 7aa3a0a546..fa24623ef7 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -616,6 +616,11 @@ pub enum AstStatement { Assignment(Assignment), // OutputAssignment OutputAssignment(Assignment), + + // x REF= y + // XXX: Not a big fan of how Assignment, OutputAssignment and ReferenceAssignment are three different + // enums instead of one with a type / kind field distinguishing them + ReferenceAssignment(Assignment), //Call Statement CallStatement(CallStatement), // Control Statements @@ -661,6 +666,9 @@ impl Debug for AstNode { AstStatement::OutputAssignment(Assignment { left, right }) => { f.debug_struct("OutputAssignment").field("left", left).field("right", right).finish() } + AstStatement::ReferenceAssignment(Assignment { left, right }) => { + f.debug_struct("ReferenceAssignment").field("left", left).field("right", right).finish() + } AstStatement::CallStatement(CallStatement { operator, parameters }) => f .debug_struct("CallStatement") .field("operator", operator) @@ -1311,6 +1319,19 @@ impl AstFactory { ) } + // XXX: Deduplicate code here? + pub fn create_reference_assignment(left: AstNode, right: AstNode, id: AstId) -> AstNode { + let location = left.location.span(&right.location); + AstNode { + stmt: AstStatement::ReferenceAssignment(Assignment { + left: Box::new(left), + right: Box::new(right), + }), + id, + location, + } + } + pub fn create_member_reference(member: AstNode, base: Option, id: AstId) -> AstNode { let location = base .as_ref() diff --git a/compiler/plc_ast/src/visitor.rs b/compiler/plc_ast/src/visitor.rs index 0aab9108ea..4db1121df9 100644 --- a/compiler/plc_ast/src/visitor.rs +++ b/compiler/plc_ast/src/visitor.rs @@ -556,6 +556,7 @@ impl Walker for AstNode { AstStatement::VlaRangeStatement => visitor.visit_vla_range_statement(node), AstStatement::Assignment(stmt) => visitor.visit_assignment(stmt, node), AstStatement::OutputAssignment(stmt) => visitor.visit_output_assignment(stmt, node), + AstStatement::ReferenceAssignment(stmt) => todo!(), AstStatement::CallStatement(stmt) => visitor.visit_call_statement(stmt, node), AstStatement::ControlStatement(stmt) => visitor.visit_control_statement(stmt, node), AstStatement::CaseCondition(stmt) => visitor.visit_case_condition(stmt, node), diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index 40b36c193f..ee8776e57b 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -1,6 +1,6 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder use super::{ - expression_generator::{to_i1, ExpressionCodeGenerator}, + expression_generator::{to_i1, ExpressionCodeGenerator, ExpressionValue}, llvm::Llvm, }; use crate::{ @@ -122,6 +122,9 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { AstStatement::Assignment(data, ..) => { self.generate_assignment_statement(&data.left, &data.right)?; } + AstStatement::ReferenceAssignment(data, ..) => { + self.generate_reference_assignment_statement(&data.left, &data.right)?; + } AstStatement::ControlStatement(ctl_statement, ..) => { self.generate_control_statement(ctl_statement)? @@ -231,6 +234,25 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { } } + // XXX: Replace Result with CodegenResult? + pub fn generate_reference_assignment_statement( + &self, + left: &AstNode, + right: &AstNode, + ) -> Result<(), Diagnostic> { + let exp_gen = self.create_expr_generator(); + + let left_type = exp_gen.get_type_hint_info_for(left)?; + let left_pvalue: PointerValue = exp_gen.generate_expression_value(left).and_then(|it| { + it.get_basic_value_enum() + .try_into() + .map_err(|err| Diagnostic::codegen_error(format!("{err:?}").as_str(), left.get_location())) + })?; + + exp_gen.generate_store(left_pvalue, left_type, right)?; + Ok(()) + } + /// generates an assignment statement _left_ := _right_ /// /// `left_statement` the left side of the assignment diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index b4dd2e88ca..991b3ecccc 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -184,3 +184,35 @@ fn floating_point_type_casting() { insta::assert_snapshot!(result); } + +#[test] +fn reference_assignment() { + let result = codegen( + r#" + FUNCTION main + VAR + a : REF_TO DINT; + b : DINT; + END_VAR + a REF= b; + END_PROGRAM + "#, + ); + + insta::assert_snapshot!(result, @r###" + ; ModuleID = 'main' + source_filename = "main" + + define void @main() section "fn-$RUSTY$main:v" { + entry: + %a = alloca i32*, align 8 + %b = alloca i32, align 4 + store i32* null, i32** %a, align 8 + store i32 0, i32* %b, align 4 + %load_b = load i32, i32* %b, align 4 + %0 = inttoptr i32 %load_b to i32* + store i32* %0, i32** %a, align 8 + ret void + } + "###); +} diff --git a/src/lexer/tokens.rs b/src/lexer/tokens.rs index 02baa2a7e1..70e54368f0 100644 --- a/src/lexer/tokens.rs +++ b/src/lexer/tokens.rs @@ -159,6 +159,9 @@ pub enum Token { #[token("=>")] KeywordOutputAssignment, + #[token("REF=")] + KeywordReferenceAssignment, + #[token("(")] KeywordParensOpen, diff --git a/src/parser/expressions_parser.rs b/src/parser/expressions_parser.rs index 798de92813..af8c57c76f 100644 --- a/src/parser/expressions_parser.rs +++ b/src/parser/expressions_parser.rs @@ -213,17 +213,25 @@ fn parse_leaf_expression(lexer: &mut ParseSession) -> AstNode { }; match literal_parse_result { - Some(statement) => { - if lexer.token == KeywordAssignment { + Some(statement) => match lexer.token { + KeywordAssignment => { lexer.advance(); AstFactory::create_assignment(statement, parse_range_statement(lexer), lexer.next_id()) - } else if lexer.token == KeywordOutputAssignment { + } + KeywordOutputAssignment => { lexer.advance(); AstFactory::create_output_assignment(statement, parse_range_statement(lexer), lexer.next_id()) - } else { - statement } - } + KeywordReferenceAssignment => { + lexer.advance(); + AstFactory::create_reference_assignment( + statement, + parse_range_statement(lexer), + lexer.next_id(), + ) + } + _ => statement, + }, None => { let statement = AstFactory::create_empty_statement( lexer.diagnostics.last().map_or(SourceLocation::undefined(), |d| d.get_location()), diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index ffddd7578f..9ed3a8921f 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -1,6 +1,6 @@ use crate::{ parser::tests::{empty_stmt, ref_to}, - test_utils::tests::parse, + test_utils::tests::{parse, parse_and_preprocess}, typesystem::DINT_TYPE, }; use insta::assert_snapshot; @@ -262,3 +262,30 @@ fn empty_parameter_assignments_in_call_statement() { let ast_string = format!("{:#?}", &result); insta::assert_snapshot!(ast_string); } + +#[test] +fn reference_assignment_is_parsed() { + let result = &parse("PROGRAM main x REF= y END_PROGRAM").0.implementations[0]; + insta::assert_debug_snapshot!(result.statements, @r###" + [ + ReferenceAssignment { + left: ReferenceExpr { + kind: Member( + Identifier { + name: "x", + }, + ), + base: None, + }, + right: ReferenceExpr { + kind: Member( + Identifier { + name: "y", + }, + ), + base: None, + }, + }, + ] + "###) +} diff --git a/src/resolver.rs b/src/resolver.rs index 6bf25261a2..0362474fde 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -901,7 +901,8 @@ impl<'i> TypeAnnotator<'i> { } } } - AstStatement::Assignment(Assignment { left, right }, ..) => { + AstStatement::ReferenceAssignment(Assignment { left, right }, ..) + | AstStatement::Assignment(Assignment { left, right }, ..) => { // struct initialization (left := right) // find out left's type and update a type hint for right if let ( @@ -1416,7 +1417,7 @@ impl<'i> TypeAnnotator<'i> { AstStatement::RangeStatement(data, ..) => { visit_all_statements!(self, ctx, &data.start, &data.end); } - AstStatement::Assignment(data, ..) => { + AstStatement::ReferenceAssignment(data, ..) | AstStatement::Assignment(data, ..) => { self.visit_statement(&ctx.enter_control(), &data.right); if let Some(lhs) = ctx.lhs { //special context for left hand side diff --git a/tests/correctness/pointers.rs b/tests/correctness/pointers.rs index 49fc28bc0c..773f049823 100644 --- a/tests/correctness/pointers.rs +++ b/tests/correctness/pointers.rs @@ -242,3 +242,21 @@ fn value_behind_function_block_pointer_is_assigned_to_correctly() { assert!(!maintype.a); assert!(maintype.b); } + +#[test] +fn reference_assignment() { + let function = r" + FUNCTION main : DINT + VAR + a : REF_TO DINT; + b : DINT := 5; + END_VAR + + a REF= b; + main := a^; + END_FUNCTION + "; + + let res: i32 = compile_and_run(function.to_string(), &mut MainType::default()); + assert_eq!(5, res); +} From 963064b05bf39f38051bb22660ab35c5dc5d9b7e Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Thu, 20 Jun 2024 08:26:21 +0200 Subject: [PATCH 02/20] Create ADR call in parser --- compiler/plc_ast/src/visitor.rs | 2 +- src/codegen/generators/statement_generator.rs | 40 +++++++++---------- src/codegen/tests/statement_codegen_test.rs | 6 +-- src/parser/expressions_parser.rs | 19 +++++++-- src/parser/tests/statement_parser_tests.rs | 19 ++++++--- 5 files changed, 52 insertions(+), 34 deletions(-) diff --git a/compiler/plc_ast/src/visitor.rs b/compiler/plc_ast/src/visitor.rs index 4db1121df9..3d37e9697c 100644 --- a/compiler/plc_ast/src/visitor.rs +++ b/compiler/plc_ast/src/visitor.rs @@ -556,7 +556,7 @@ impl Walker for AstNode { AstStatement::VlaRangeStatement => visitor.visit_vla_range_statement(node), AstStatement::Assignment(stmt) => visitor.visit_assignment(stmt, node), AstStatement::OutputAssignment(stmt) => visitor.visit_output_assignment(stmt, node), - AstStatement::ReferenceAssignment(stmt) => todo!(), + AstStatement::ReferenceAssignment(_) => todo!(), AstStatement::CallStatement(stmt) => visitor.visit_call_statement(stmt, node), AstStatement::ControlStatement(stmt) => visitor.visit_control_statement(stmt, node), AstStatement::CaseCondition(stmt) => visitor.visit_case_condition(stmt, node), diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index ee8776e57b..696ff1a12f 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -1,6 +1,6 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder use super::{ - expression_generator::{to_i1, ExpressionCodeGenerator, ExpressionValue}, + expression_generator::{to_i1, ExpressionCodeGenerator}, llvm::Llvm, }; use crate::{ @@ -234,25 +234,6 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { } } - // XXX: Replace Result with CodegenResult? - pub fn generate_reference_assignment_statement( - &self, - left: &AstNode, - right: &AstNode, - ) -> Result<(), Diagnostic> { - let exp_gen = self.create_expr_generator(); - - let left_type = exp_gen.get_type_hint_info_for(left)?; - let left_pvalue: PointerValue = exp_gen.generate_expression_value(left).and_then(|it| { - it.get_basic_value_enum() - .try_into() - .map_err(|err| Diagnostic::codegen_error(format!("{err:?}").as_str(), left.get_location())) - })?; - - exp_gen.generate_store(left_pvalue, left_type, right)?; - Ok(()) - } - /// generates an assignment statement _left_ := _right_ /// /// `left_statement` the left side of the assignment @@ -295,6 +276,25 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { Ok(()) } + // XXX: Replace Result with CodegenResult? + pub fn generate_reference_assignment_statement( + &self, + left: &AstNode, + right: &AstNode, + ) -> Result<(), Diagnostic> { + let exp_gen = self.create_expr_generator(); + + let left_type = exp_gen.get_type_hint_info_for(left)?; + let left_pvalue: PointerValue = exp_gen.generate_expression_value(left).and_then(|it| { + it.get_basic_value_enum() + .try_into() + .map_err(|err| Diagnostic::codegen_error(format!("{err:?}").as_str(), left.get_location())) + })?; + + exp_gen.generate_store(left_pvalue, left_type, right)?; + Ok(()) + } + fn register_debug_location(&self, statement: &AstNode) { let line = statement.get_location().get_line_plus_one(); let column = statement.get_location().get_column(); diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index 991b3ecccc..fb79c632c4 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -209,9 +209,9 @@ fn reference_assignment() { %b = alloca i32, align 4 store i32* null, i32** %a, align 8 store i32 0, i32* %b, align 4 - %load_b = load i32, i32* %b, align 4 - %0 = inttoptr i32 %load_b to i32* - store i32* %0, i32** %a, align 8 + %0 = ptrtoint i32* %b to i64 + %1 = inttoptr i64 %0 to i32* + store i32* %1, i32** %a, align 8 ret void } "###); diff --git a/src/parser/expressions_parser.rs b/src/parser/expressions_parser.rs index af8c57c76f..61d1aeb56b 100644 --- a/src/parser/expressions_parser.rs +++ b/src/parser/expressions_parser.rs @@ -1,5 +1,6 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder +use crate::builtins::get_builtin; use crate::{ expect_token, lexer::Token::*, @@ -223,12 +224,22 @@ fn parse_leaf_expression(lexer: &mut ParseSession) -> AstNode { AstFactory::create_output_assignment(statement, parse_range_statement(lexer), lexer.next_id()) } KeywordReferenceAssignment => { + debug_assert!( + get_builtin("ADR").is_some(), + "The ADR builtin must exist for the following REF= syntactic sugar" + ); + lexer.advance(); - AstFactory::create_reference_assignment( - statement, - parse_range_statement(lexer), + let right = parse_range_statement(lexer); + let operator = + AstFactory::create_identifier("ADR", &SourceLocation::undefined(), lexer.next_id()); + let call = AstFactory::create_call_statement( + operator, + Some(right), lexer.next_id(), - ) + SourceLocation::undefined(), + ); + AstFactory::create_assignment(statement, call, lexer.next_id()) } _ => statement, }, diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index 9ed3a8921f..b57640836a 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -268,7 +268,7 @@ fn reference_assignment_is_parsed() { let result = &parse("PROGRAM main x REF= y END_PROGRAM").0.implementations[0]; insta::assert_debug_snapshot!(result.statements, @r###" [ - ReferenceAssignment { + Assignment { left: ReferenceExpr { kind: Member( Identifier { @@ -277,13 +277,20 @@ fn reference_assignment_is_parsed() { ), base: None, }, - right: ReferenceExpr { - kind: Member( - Identifier { - name: "y", + right: CallStatement { + operator: Identifier { + name: "ADR", + }, + parameters: Some( + ReferenceExpr { + kind: Member( + Identifier { + name: "y", + }, + ), + base: None, }, ), - base: None, }, }, ] From 9486255d9c78db4d82dd92886fb742cb48ec24a7 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Thu, 20 Jun 2024 09:57:07 +0200 Subject: [PATCH 03/20] Remove ReferenceAssignment node --- compiler/plc_ast/src/ast.rs | 32 ++++--------------- compiler/plc_ast/src/visitor.rs | 1 - src/codegen/generators/statement_generator.rs | 23 ------------- src/codegen/tests/statement_codegen_test.rs | 4 +-- src/parser/expressions_parser.rs | 22 ++++++------- src/parser/tests/statement_parser_tests.rs | 2 +- src/resolver.rs | 5 ++- 7 files changed, 22 insertions(+), 67 deletions(-) diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 451401c1f6..01bb0d3a59 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -596,11 +596,14 @@ pub struct AstNode { #[derive(Debug, Clone, PartialEq)] pub enum AstStatement { EmptyStatement(EmptyStatement), - // a placeholder that indicates a default value of a datatype + + // A placeholder which indicates a default value of a datatype DefaultValue(DefaultValue), + // Literals Literal(AstLiteral), MultipliedStatement(MultipliedStatement), + // Expressions ReferenceExpr(ReferenceExpr), Identifier(String), @@ -612,20 +615,15 @@ pub enum AstStatement { ParenExpression(Box), RangeStatement(RangeStatement), VlaRangeStatement, - // Assignment + + // Assignments Assignment(Assignment), - // OutputAssignment OutputAssignment(Assignment), - // x REF= y - // XXX: Not a big fan of how Assignment, OutputAssignment and ReferenceAssignment are three different - // enums instead of one with a type / kind field distinguishing them - ReferenceAssignment(Assignment), - //Call Statement CallStatement(CallStatement), + // Control Statements ControlStatement(AstControlStatement), - CaseCondition(Box), ExitStatement(()), ContinueStatement(()), @@ -666,9 +664,6 @@ impl Debug for AstNode { AstStatement::OutputAssignment(Assignment { left, right }) => { f.debug_struct("OutputAssignment").field("left", left).field("right", right).finish() } - AstStatement::ReferenceAssignment(Assignment { left, right }) => { - f.debug_struct("ReferenceAssignment").field("left", left).field("right", right).finish() - } AstStatement::CallStatement(CallStatement { operator, parameters }) => f .debug_struct("CallStatement") .field("operator", operator) @@ -1327,19 +1322,6 @@ impl AstFactory { ) } - // XXX: Deduplicate code here? - pub fn create_reference_assignment(left: AstNode, right: AstNode, id: AstId) -> AstNode { - let location = left.location.span(&right.location); - AstNode { - stmt: AstStatement::ReferenceAssignment(Assignment { - left: Box::new(left), - right: Box::new(right), - }), - id, - location, - } - } - pub fn create_member_reference(member: AstNode, base: Option, id: AstId) -> AstNode { let location = base .as_ref() diff --git a/compiler/plc_ast/src/visitor.rs b/compiler/plc_ast/src/visitor.rs index 3d37e9697c..0aab9108ea 100644 --- a/compiler/plc_ast/src/visitor.rs +++ b/compiler/plc_ast/src/visitor.rs @@ -556,7 +556,6 @@ impl Walker for AstNode { AstStatement::VlaRangeStatement => visitor.visit_vla_range_statement(node), AstStatement::Assignment(stmt) => visitor.visit_assignment(stmt, node), AstStatement::OutputAssignment(stmt) => visitor.visit_output_assignment(stmt, node), - AstStatement::ReferenceAssignment(_) => todo!(), AstStatement::CallStatement(stmt) => visitor.visit_call_statement(stmt, node), AstStatement::ControlStatement(stmt) => visitor.visit_control_statement(stmt, node), AstStatement::CaseCondition(stmt) => visitor.visit_case_condition(stmt, node), diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index 92522c38fb..7e7f8a2148 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -122,10 +122,6 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { AstStatement::Assignment(data, ..) => { self.generate_assignment_statement(&data.left, &data.right)?; } - AstStatement::ReferenceAssignment(data, ..) => { - self.generate_reference_assignment_statement(&data.left, &data.right)?; - } - AstStatement::ControlStatement(ctl_statement, ..) => { self.generate_control_statement(ctl_statement)? } @@ -276,25 +272,6 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { Ok(()) } - // XXX: Replace Result with CodegenResult? - pub fn generate_reference_assignment_statement( - &self, - left: &AstNode, - right: &AstNode, - ) -> Result<(), Diagnostic> { - let exp_gen = self.create_expr_generator(); - - let left_type = exp_gen.get_type_hint_info_for(left)?; - let left_pvalue: PointerValue = exp_gen.generate_expression_value(left).and_then(|it| { - it.get_basic_value_enum() - .try_into() - .map_err(|err| Diagnostic::codegen_error(format!("{err:?}").as_str(), left.get_location())) - })?; - - exp_gen.generate_store(left_pvalue, left_type, right)?; - Ok(()) - } - fn register_debug_location(&self, statement: &AstNode) { let line = statement.get_location().get_line_plus_one(); let column = statement.get_location().get_column(); diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index fb79c632c4..b879f70b57 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -209,9 +209,7 @@ fn reference_assignment() { %b = alloca i32, align 4 store i32* null, i32** %a, align 8 store i32 0, i32* %b, align 4 - %0 = ptrtoint i32* %b to i64 - %1 = inttoptr i64 %0 to i32* - store i32* %1, i32** %a, align 8 + store i32* %b, i32** %a, align 8 ret void } "###); diff --git a/src/parser/expressions_parser.rs b/src/parser/expressions_parser.rs index 61d1aeb56b..8f33e58a93 100644 --- a/src/parser/expressions_parser.rs +++ b/src/parser/expressions_parser.rs @@ -223,23 +223,23 @@ fn parse_leaf_expression(lexer: &mut ParseSession) -> AstNode { lexer.advance(); AstFactory::create_output_assignment(statement, parse_range_statement(lexer), lexer.next_id()) } + // TODO: This is a good candidate for lowering (if we ever implement it) KeywordReferenceAssignment => { debug_assert!( - get_builtin("ADR").is_some(), - "The ADR builtin must exist for the following REF= syntactic sugar" + get_builtin("REF").is_some(), + "The REF builtin must exist for the REF= syntactic sugar" ); lexer.advance(); - let right = parse_range_statement(lexer); - let operator = - AstFactory::create_identifier("ADR", &SourceLocation::undefined(), lexer.next_id()); - let call = AstFactory::create_call_statement( - operator, - Some(right), + let fn_arg = parse_range_statement(lexer); + let fn_loc = fn_arg.location.clone(); + let fn_name = AstFactory::create_identifier("REF", &fn_loc, lexer.next_id()); + + AstFactory::create_assignment( + statement, + AstFactory::create_call_statement(fn_name, Some(fn_arg), lexer.next_id(), fn_loc), lexer.next_id(), - SourceLocation::undefined(), - ); - AstFactory::create_assignment(statement, call, lexer.next_id()) + ) } _ => statement, }, diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index b57640836a..d1c352e239 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -279,7 +279,7 @@ fn reference_assignment_is_parsed() { }, right: CallStatement { operator: Identifier { - name: "ADR", + name: "REF", }, parameters: Some( ReferenceExpr { diff --git a/src/resolver.rs b/src/resolver.rs index 9f3ee2c8b5..a77e99ad46 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -904,8 +904,7 @@ impl<'i> TypeAnnotator<'i> { } } } - AstStatement::ReferenceAssignment(Assignment { left, right }, ..) - | AstStatement::Assignment(Assignment { left, right }, ..) => { + AstStatement::Assignment(Assignment { left, right }, ..) => { // struct initialization (left := right) // find out left's type and update a type hint for right if let ( @@ -1420,7 +1419,7 @@ impl<'i> TypeAnnotator<'i> { AstStatement::RangeStatement(data, ..) => { visit_all_statements!(self, ctx, &data.start, &data.end); } - AstStatement::ReferenceAssignment(data, ..) | AstStatement::Assignment(data, ..) => { + AstStatement::Assignment(data, ..) => { self.visit_statement(&ctx.enter_control(), &data.right); if let Some(lhs) = ctx.lhs { //special context for left hand side From c79d74e6b02a47e6762101f6a2c1411fb45afe53 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Mon, 24 Jun 2024 13:43:42 +0200 Subject: [PATCH 04/20] Initial commit --- compiler/plc_ast/src/ast.rs | 13 +++++ err.conf | 3 ++ .../.statement_codegen_test.rs.pending-snap | 16 +++++++ src/codegen/tests/statement_codegen_test.rs | 34 +++++++++++++ ...generates_inline_pointer_to_pointer-2.snap | 1 + ...g_generates_inline_pointer_to_pointer.snap | 1 + ..._processing_generates_inline_pointers.snap | 1 + ...g_generates_pointer_to_pointer_type-2.snap | 1 + ...ing_generates_pointer_to_pointer_type.snap | 1 + src/index/visitor.rs | 6 ++- src/lexer/tokens.rs | 3 ++ src/parser.rs | 21 +++++++- .../parse_error_statements_tests.rs | 2 + ...r_tests__global_pointer_declaration-2.snap | 1 + ...ser_tests__global_pointer_declaration.snap | 1 + ..._type_parser_tests__pointer_type_test.snap | 1 + ...sts__type_parser_tests__ref_type_test.snap | 1 + src/parser/tests/statement_parser_tests.rs | 48 +++++++++++++++++++ src/resolver.rs | 1 + tests/correctness/pointers.rs | 17 +++++++ 20 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 err.conf create mode 100644 src/codegen/tests/.statement_codegen_test.rs.pending-snap diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index e7353608c2..38fd42ab5d 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -423,6 +423,18 @@ impl Debug for DataTypeDeclaration { } impl DataTypeDeclaration { + pub fn temp(&mut self) { + match self { + Self::DataTypeDefinition { data_type, .. } => match data_type { + DataType::PointerType { ref mut auto_deref, .. } => { + *auto_deref = true; + } + _ => {} + }, + _ => {} + } + } + pub fn get_name(&self) -> Option<&str> { match self { DataTypeDeclaration::DataTypeReference { referenced_type, .. } => Some(referenced_type.as_str()), @@ -487,6 +499,7 @@ pub enum DataType { PointerType { name: Option, referenced_type: Box, + auto_deref: bool, }, StringType { name: Option, diff --git a/err.conf b/err.conf new file mode 100644 index 0000000000..04a0b3c5af --- /dev/null +++ b/err.conf @@ -0,0 +1,3 @@ +{ +"warning": ["E065", "E037"] +} diff --git a/src/codegen/tests/.statement_codegen_test.rs.pending-snap b/src/codegen/tests/.statement_codegen_test.rs.pending-snap new file mode 100644 index 0000000000..948ef002ff --- /dev/null +++ b/src/codegen/tests/.statement_codegen_test.rs.pending-snap @@ -0,0 +1,16 @@ +{"run_id":"1719219508-220590713","line":204,"new":{"module_name":"rusty__codegen__tests__statement_codegen_test","snapshot_name":"reference_assignment","metadata":{"source":"src/codegen/tests/statement_codegen_test.rs","assertion_line":204,"expression":"result"},"snapshot":"; ModuleID = 'main'\nsource_filename = \"main\"\n\ndefine i32 @main() section \"fn-$RUSTY$main:i32\" {\nentry:\n %main = alloca i32, align 4\n %a = alloca i32*, align 8\n %b = alloca i32*, align 8\n store i32* null, i32** %a, align 8\n store i32* null, i32** %b, align 8\n store i32 0, i32* %main, align 4\n %deref = load i32*, i32** %a, align 8\n store i32 5, i32* %deref, align 4\n %deref1 = load i32*, i32** %b, align 8\n store i32 5, i32* %deref1, align 4\n %deref2 = load i32*, i32** %a, align 8\n %load_a = load i32, i32* %deref2, align 4\n store i32 %load_a, i32* %main, align 4\n %main_ret = load i32, i32* %main, align 4\n ret i32 %main_ret\n}\n"},"old":{"module_name":"rusty__codegen__tests__statement_codegen_test","metadata":{},"snapshot":"; ModuleID = 'main'\nsource_filename = \"main\"\n\ndefine void @main() section \"fn-$RUSTY$main:v\" {\nentry:\n %a = alloca i32*, align 8\n %b = alloca i32*, align 8\n store i32* null, i32** %a, align 8\n store i32* null, i32** %b, align 8\n %deref = load i32*, i32** %a, align 8\n store i32 5, i32* %deref, align 4\n %deref1 = load i32*, i32** %b, align 8\n store i32 5, i32* %deref1, align 4\n ret void\n}"}} +{"run_id":"1719219523-734309","line":203,"new":null,"old":null} +{"run_id":"1719219545-821799609","line":203,"new":null,"old":null} +{"run_id":"1719219553-568688234","line":203,"new":null,"old":null} +{"run_id":"1719219558-742119455","line":203,"new":null,"old":null} +{"run_id":"1719219591-153566692","line":203,"new":null,"old":null} +{"run_id":"1719219598-919647505","line":203,"new":null,"old":null} +{"run_id":"1719219975-561294273","line":203,"new":null,"old":null} +{"run_id":"1719220130-556533083","line":203,"new":null,"old":null} +{"run_id":"1719220168-473614010","line":203,"new":null,"old":null} +{"run_id":"1719220183-418678515","line":203,"new":null,"old":null} +{"run_id":"1719220192-67620263","line":203,"new":null,"old":null} +{"run_id":"1719220240-658317288","line":203,"new":null,"old":null} +{"run_id":"1719220243-256187333","line":203,"new":null,"old":null} +{"run_id":"1719227577-809699386","line":203,"new":null,"old":null} +{"run_id":"1719227589-981866752","line":203,"new":null,"old":null} diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index b4dd2e88ca..d21a2ba4ea 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -184,3 +184,37 @@ fn floating_point_type_casting() { insta::assert_snapshot!(result); } + +#[test] +fn reference_assignment() { + let result = codegen( + r#" + FUNCTION main + VAR + a : REFERENCE TO DINT; + b : REF_TO DINT; + END_VAR + a := 5; + b^ := 5; + END_FUNCTION + "#, + ); + + insta::assert_snapshot!(result, @r###" + ; ModuleID = 'main' + source_filename = "main" + + define void @main() section "fn-$RUSTY$main:v" { + entry: + %a = alloca i32*, align 8 + %b = alloca i32*, align 8 + store i32* null, i32** %a, align 8 + store i32* null, i32** %b, align 8 + %deref = load i32*, i32** %a, align 8 + store i32 5, i32* %deref, align 4 + %deref1 = load i32*, i32** %b, align 8 + store i32 5, i32* %deref1, align 4 + ret void + } + "###); +} diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap index 16bd9f9481..b0b33f5c00 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "__foo_inline_pointer_", }, + auto_deref: false, }, initializer: None, scope: Some( diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap index 493df6f2fd..dbe8dfb22c 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, initializer: None, scope: Some( diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap index e603f07e5d..b1ea8bdafb 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, initializer: None, scope: Some( diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap index df5d0ae77a..60c2f8729f 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "__pointer_to_pointer", }, + auto_deref: false, }, initializer: None, scope: None, diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap index f5332a5730..92218484ad 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, initializer: None, scope: None, diff --git a/src/index/visitor.rs b/src/index/visitor.rs index cff5e49e47..9d3bf74410 100644 --- a/src/index/visitor.rs +++ b/src/index/visitor.rs @@ -400,12 +400,12 @@ fn visit_data_type(index: &mut Index, type_declaration: &UserTypeDeclaration) { DataType::ArrayType { name: Some(name), bounds, referenced_type, .. } => { visit_array(bounds, index, scope, referenced_type, name, type_declaration); } - DataType::PointerType { name: Some(name), referenced_type, .. } => { + DataType::PointerType { name: Some(name), referenced_type, auto_deref } => { let inner_type_name = referenced_type.get_name().expect("named datatype"); let information = DataTypeInformation::Pointer { name: name.clone(), inner_type_name: inner_type_name.into(), - auto_deref: false, + auto_deref: *auto_deref, }; let init = index.get_mut_const_expressions().maybe_add_constant_expression( @@ -413,6 +413,7 @@ fn visit_data_type(index: &mut Index, type_declaration: &UserTypeDeclaration) { name, scope.clone(), ); + // TODO: Here index.register_type(typesystem::DataType { name: name.to_string(), initial_value: init, @@ -572,6 +573,7 @@ fn visit_variable_length_array( referenced_type: dummy_array_name, location: SourceLocation::undefined(), }), + auto_deref: false, }, location: SourceLocation::undefined(), scope: None, diff --git a/src/lexer/tokens.rs b/src/lexer/tokens.rs index 02baa2a7e1..b6ec00f5f8 100644 --- a/src/lexer/tokens.rs +++ b/src/lexer/tokens.rs @@ -252,6 +252,9 @@ pub enum Token { #[token("REFTO", ignore(case))] KeywordRef, + #[token("REFERENCE TO", ignore(case))] + KeywordReferenceTo, + #[token("ARRAY", ignore(case))] KeywordArray, diff --git a/src/parser.rs b/src/parser.rs index 385322fd28..fb88da20e4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -705,7 +705,7 @@ fn parse_pointer_definition( parse_data_type_definition(lexer, None).map(|(decl, initializer)| { ( DataTypeDeclaration::DataTypeDefinition { - data_type: DataType::PointerType { name, referenced_type: Box::new(decl) }, + data_type: DataType::PointerType { name, referenced_type: Box::new(decl), auto_deref: false }, location: lexer.source_range_factory.create_range(start_pos..lexer.last_range.end), scope: lexer.scope.clone(), }, @@ -1108,7 +1108,11 @@ fn parse_variable_line(lexer: &mut ParseSession) -> Vec { // create variables with the same data type for each of the names let mut variables = vec![]; - if let Some((data_type, initializer)) = parse_full_data_type_definition(lexer, None) { + + if lexer.try_consume(&KeywordReferenceTo) { + let (mut data_type, initializer) = + parse_pointer_definition(lexer, None, lexer.last_range.start).unwrap(); + data_type.temp(); for (name, range) in var_names { variables.push(Variable { name, @@ -1118,6 +1122,19 @@ fn parse_variable_line(lexer: &mut ParseSession) -> Vec { address: address.clone(), }); } + lexer.advance(); + } else { + if let Some((data_type, initializer)) = parse_full_data_type_definition(lexer, None) { + for (name, range) in var_names { + variables.push(Variable { + name, + data_type_declaration: data_type.clone(), + location: lexer.source_range_factory.create_range(range), + initializer: initializer.clone(), + address: address.clone(), + }); + } + } } variables } diff --git a/src/parser/tests/parse_errors/parse_error_statements_tests.rs b/src/parser/tests/parse_errors/parse_error_statements_tests.rs index ca29b7abba..5f6fb08a88 100644 --- a/src/parser/tests/parse_errors/parse_error_statements_tests.rs +++ b/src/parser/tests/parse_errors/parse_error_statements_tests.rs @@ -1128,6 +1128,7 @@ fn pointer_type_without_to_test() { referenced_type: "INT".to_string(), location: SourceLocation::undefined(), }), + auto_deref: false, }, location: SourceLocation::undefined(), initializer: None, @@ -1154,6 +1155,7 @@ fn pointer_type_with_wrong_keyword_to_test() { referenced_type: "tu".to_string(), location: SourceLocation::undefined(), }), + auto_deref: false, }, location: SourceLocation::undefined(), initializer: None, diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap index f430f66293..604b464e1f 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap @@ -10,6 +10,7 @@ Variable { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, }, } diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap index f7426bc0ee..29dc5d6aea 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap @@ -10,6 +10,7 @@ Variable { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, }, } diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap index 1bd1d207de..a5d326dd82 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, initializer: None, scope: None, diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap index 9402582b6e..94c71479f5 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap @@ -10,6 +10,7 @@ UserTypeDeclaration { referenced_type: DataTypeReference { referenced_type: "INT", }, + auto_deref: false, }, initializer: None, scope: None, diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index ffddd7578f..1107d19875 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -262,3 +262,51 @@ fn empty_parameter_assignments_in_call_statement() { let ast_string = format!("{:#?}", &result); insta::assert_snapshot!(ast_string); } + +#[test] +fn reference_to_variable() { + let (result, diagnostics) = parse( + r" + FUNCTION foo + VAR + bar : DINT; + baz : REFERENCE TO DINT; + qux : DINT; + END_VAR + END_FUNCTION + ", + ); + + assert!(diagnostics.is_empty()); + insta::assert_debug_snapshot!(result.units[0].variable_blocks[0], @r###" + VariableBlock { + variables: [ + Variable { + name: "bar", + data_type: DataTypeReference { + referenced_type: "DINT", + }, + }, + Variable { + name: "baz", + data_type: DataTypeDefinition { + data_type: PointerType { + name: None, + referenced_type: DataTypeReference { + referenced_type: "DINT", + }, + auto_deref: true, + }, + }, + }, + Variable { + name: "qux", + data_type: DataTypeReference { + referenced_type: "DINT", + }, + }, + ], + variable_block_type: Local, + } + "###); +} diff --git a/src/resolver.rs b/src/resolver.rs index a77e99ad46..1167a15a3a 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -1923,6 +1923,7 @@ fn to_variable_annotation( // real auto-deref pointer (inner_type_name.clone(), AUTO_DEREF) } + // TODO: Autoderef here _ => (v_type.get_name().to_string(), NO_DEREF), }; diff --git a/tests/correctness/pointers.rs b/tests/correctness/pointers.rs index 49fc28bc0c..8237e66c19 100644 --- a/tests/correctness/pointers.rs +++ b/tests/correctness/pointers.rs @@ -242,3 +242,20 @@ fn value_behind_function_block_pointer_is_assigned_to_correctly() { assert!(!maintype.a); assert!(maintype.b); } + +#[test] +fn reference_to_assignment() { + let function = r" + FUNCTION main : DINT + VAR + a : REFERENCE TO DINT; + b : DINT := 5; + END_VAR + a := REF(b); + main := a; + END_FUNCTION + "; + + let res: i32 = compile_and_run(function, &mut MainType::default()); + assert_eq!(5, res); +} From acd5770ece43027d700266edc3b2013630a88b70 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Tue, 25 Jun 2024 11:42:45 +0200 Subject: [PATCH 05/20] Introduce RefAssignment node, adjust codegen --- compiler/plc_ast/src/ast.rs | 14 +++++++++++++ compiler/plc_ast/src/visitor.rs | 10 ++++++++++ .../generators/expression_generator.rs | 3 +-- src/codegen/generators/statement_generator.rs | 20 +++++++++++++++++++ src/parser/expressions_parser.rs | 16 +-------------- src/parser/tests/statement_parser_tests.rs | 19 ++++++------------ src/resolver.rs | 9 +++++++-- 7 files changed, 59 insertions(+), 32 deletions(-) diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 01bb0d3a59..aad3007131 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -619,6 +619,7 @@ pub enum AstStatement { // Assignments Assignment(Assignment), OutputAssignment(Assignment), + RefAssignment(Assignment), CallStatement(CallStatement), @@ -664,6 +665,9 @@ impl Debug for AstNode { AstStatement::OutputAssignment(Assignment { left, right }) => { f.debug_struct("OutputAssignment").field("left", left).field("right", right).finish() } + AstStatement::RefAssignment(Assignment { left, right }) => { + f.debug_struct("ReferenceAssignment").field("left", left).field("right", right).finish() + } AstStatement::CallStatement(CallStatement { operator, parameters }) => f .debug_struct("CallStatement") .field("operator", operator) @@ -1322,6 +1326,16 @@ impl AstFactory { ) } + // TODO: Merge this with create(_output)_assignment + pub fn create_ref_assignment(left: AstNode, right: AstNode, id: AstId) -> AstNode { + let location = left.location.span(&right.location); + AstNode::new( + AstStatement::RefAssignment(Assignment { left: Box::new(left), right: Box::new(right) }), + id, + location, + ) + } + pub fn create_member_reference(member: AstNode, base: Option, id: AstId) -> AstNode { let location = base .as_ref() diff --git a/compiler/plc_ast/src/visitor.rs b/compiler/plc_ast/src/visitor.rs index 0aab9108ea..c50b7c3c1b 100644 --- a/compiler/plc_ast/src/visitor.rs +++ b/compiler/plc_ast/src/visitor.rs @@ -312,6 +312,15 @@ pub trait AstVisitor: Sized { stmt.walk(self) } + /// Visits an `RefAssignment` node. + /// Make sure to call `walk` on the `Assignment` node to visit its children. + /// # Arguments + /// * `stmt` - The unwraped, typed `Assignment` node to visit. + /// * `node` - The wrapped `AstNode` node to visit. Offers access to location information and AstId + fn visit_ref_assignment(&mut self, stmt: &Assignment, _node: &AstNode) { + stmt.walk(self) + } + /// Visits a `CallStatement` node. /// Make sure to call `walk` on the `CallStatement` node to visit its children. /// # Arguments @@ -556,6 +565,7 @@ impl Walker for AstNode { AstStatement::VlaRangeStatement => visitor.visit_vla_range_statement(node), AstStatement::Assignment(stmt) => visitor.visit_assignment(stmt, node), AstStatement::OutputAssignment(stmt) => visitor.visit_output_assignment(stmt, node), + AstStatement::RefAssignment(stmt) => visitor.visit_ref_assignment(stmt, node), AstStatement::CallStatement(stmt) => visitor.visit_call_statement(stmt, node), AstStatement::ControlStatement(stmt) => visitor.visit_control_statement(stmt, node), AstStatement::CaseCondition(stmt) => visitor.visit_case_condition(stmt, node), diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs index 3ad0a9abf7..b9a1941bb5 100644 --- a/src/codegen/generators/expression_generator.rs +++ b/src/codegen/generators/expression_generator.rs @@ -2507,14 +2507,13 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> { /// - `access` the ReferenceAccess of the reference to generate /// - `base` the "previous" segment of an optional qualified reference-access /// - `original_expression` the original ast-statement used to report Diagnostics - fn generate_reference_expression( + pub(crate) fn generate_reference_expression( &self, access: &ReferenceAccess, base: Option<&AstNode>, original_expression: &AstNode, ) -> Result, Diagnostic> { match (access, base) { - // expressions like `base.member`, or just `member` (ReferenceAccess::Member(member), base) => { let base_value = base.map(|it| self.generate_expression_value(it)).transpose()?; diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index 7e7f8a2148..e23cc3b16d 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -122,6 +122,9 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { AstStatement::Assignment(data, ..) => { self.generate_assignment_statement(&data.left, &data.right)?; } + AstStatement::RefAssignment(data, ..) => { + self.generate_ref_assignment(&data.left, &data.right)?; + } AstStatement::ControlStatement(ctl_statement, ..) => { self.generate_control_statement(ctl_statement)? } @@ -230,6 +233,23 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { } } + pub fn generate_ref_assignment(&self, left: &AstNode, right: &AstNode) -> Result<(), Diagnostic> { + let exp = self.create_expr_generator(); + let AstStatement::ReferenceExpr(data) = &left.stmt else { + panic!("this needs a validation, but the lhs must always be a reference?") + }; + + let ref_builtin = self.index.get_builtin_function("REF").expect("REF must exist"); + let left_pointer = { + let expr = exp.generate_reference_expression(&data.access, data.base.as_deref(), left)?; + expr.get_basic_value_enum().into_pointer_value() + }; + let right_expr_value = ref_builtin.codegen(&exp, &[&right], SourceLocation::undefined()).unwrap(); + + self.llvm.builder.build_store(left_pointer, right_expr_value.get_basic_value_enum()); + Ok(()) + } + /// generates an assignment statement _left_ := _right_ /// /// `left_statement` the left side of the assignment diff --git a/src/parser/expressions_parser.rs b/src/parser/expressions_parser.rs index 8f33e58a93..9f251f5618 100644 --- a/src/parser/expressions_parser.rs +++ b/src/parser/expressions_parser.rs @@ -223,23 +223,9 @@ fn parse_leaf_expression(lexer: &mut ParseSession) -> AstNode { lexer.advance(); AstFactory::create_output_assignment(statement, parse_range_statement(lexer), lexer.next_id()) } - // TODO: This is a good candidate for lowering (if we ever implement it) KeywordReferenceAssignment => { - debug_assert!( - get_builtin("REF").is_some(), - "The REF builtin must exist for the REF= syntactic sugar" - ); - lexer.advance(); - let fn_arg = parse_range_statement(lexer); - let fn_loc = fn_arg.location.clone(); - let fn_name = AstFactory::create_identifier("REF", &fn_loc, lexer.next_id()); - - AstFactory::create_assignment( - statement, - AstFactory::create_call_statement(fn_name, Some(fn_arg), lexer.next_id(), fn_loc), - lexer.next_id(), - ) + AstFactory::create_ref_assignment(statement, parse_range_statement(lexer), lexer.next_id()) } _ => statement, }, diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index d1c352e239..9ed3a8921f 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -268,7 +268,7 @@ fn reference_assignment_is_parsed() { let result = &parse("PROGRAM main x REF= y END_PROGRAM").0.implementations[0]; insta::assert_debug_snapshot!(result.statements, @r###" [ - Assignment { + ReferenceAssignment { left: ReferenceExpr { kind: Member( Identifier { @@ -277,20 +277,13 @@ fn reference_assignment_is_parsed() { ), base: None, }, - right: CallStatement { - operator: Identifier { - name: "REF", - }, - parameters: Some( - ReferenceExpr { - kind: Member( - Identifier { - name: "y", - }, - ), - base: None, + right: ReferenceExpr { + kind: Member( + Identifier { + name: "y", }, ), + base: None, }, }, ] diff --git a/src/resolver.rs b/src/resolver.rs index a77e99ad46..10043eb847 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -1419,7 +1419,7 @@ impl<'i> TypeAnnotator<'i> { AstStatement::RangeStatement(data, ..) => { visit_all_statements!(self, ctx, &data.start, &data.end); } - AstStatement::Assignment(data, ..) => { + AstStatement::Assignment(data, ..) | AstStatement::RefAssignment(data, ..) => { self.visit_statement(&ctx.enter_control(), &data.right); if let Some(lhs) = ctx.lhs { //special context for left hand side @@ -1749,7 +1749,12 @@ impl<'i> TypeAnnotator<'i> { } pub(crate) fn annotate_parameters(&mut self, p: &AstNode, type_name: &str) { - if !matches!(p.get_stmt(), AstStatement::Assignment(..) | AstStatement::OutputAssignment(..)) { + if !matches!( + p.get_stmt(), + AstStatement::Assignment(..) + | AstStatement::OutputAssignment(..) + | AstStatement::RefAssignment(..) + ) { if let Some(effective_member_type) = self.index.find_effective_type_by_name(type_name) { //update the type hint self.annotation_map From 29f7e96bb49bc62b9d7515cd490e8ad22cdc6312 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Tue, 25 Jun 2024 12:33:41 +0200 Subject: [PATCH 06/20] polish --- compiler/plc_ast/src/ast.rs | 6 +++++- src/codegen/generators/statement_generator.rs | 21 ++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 70848e1833..7e55f5adbc 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -629,6 +629,7 @@ pub enum AstStatement { RangeStatement(RangeStatement), VlaRangeStatement, + // TODO: Merge these variants with a `kind` field? // Assignments Assignment(Assignment), OutputAssignment(Assignment), @@ -1339,7 +1340,10 @@ impl AstFactory { ) } - // TODO: Merge this with create(_output)_assignment + // TODO: Merge `create_assignment`, `create_output_assignment` and `create_ref_assignment` + // once the the Assignment AstStatements have been merged and a `kind` field is available + // I.e. something like `AstStatement::Assignment { data, kind: AssignmentKind { Normal, Output, Reference } } + // and then fn create_assignment(kind: AssignmentKind, ...) pub fn create_ref_assignment(left: AstNode, right: AstNode, id: AstId) -> AstNode { let location = left.location.span(&right.location); AstNode::new( diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index e23cc3b16d..b712a61106 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -233,20 +233,31 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { } } + // Rules: + // lhs of REF= must be reference declared with `REFERENCE TO` (i.e. a "reference to type") + // rhs of REF= must be lvalue + + /// Generates IR for a `REF=` assignment, which is syntactic sugar for a `REF(...)` call on the + /// right side of an assignment. Specifically `foo REF= bar` and `foo := REF(bar)` are the same. + /// + /// Note: Though somewhat similar to the [`generate_assignment_statement`] function, we can't + /// directly call it here because the left side of a `REF=` assignment is flagged as auto-deref + /// but for `REF=` assignment we don't want (and can't) deref without generating incorrect IR.:w pub fn generate_ref_assignment(&self, left: &AstNode, right: &AstNode) -> Result<(), Diagnostic> { let exp = self.create_expr_generator(); + let ref_builtin = self.index.get_builtin_function("REF").expect("REF must exist"); + let AstStatement::ReferenceExpr(data) = &left.stmt else { - panic!("this needs a validation, but the lhs must always be a reference?") + unreachable!("should be covered by a validation? The left-hand side must be a reference") }; - let ref_builtin = self.index.get_builtin_function("REF").expect("REF must exist"); - let left_pointer = { + let left_ptr_val = { let expr = exp.generate_reference_expression(&data.access, data.base.as_deref(), left)?; expr.get_basic_value_enum().into_pointer_value() }; - let right_expr_value = ref_builtin.codegen(&exp, &[&right], SourceLocation::undefined()).unwrap(); + let right_expr_val = ref_builtin.codegen(&exp, &[&right], right.get_location())?; - self.llvm.builder.build_store(left_pointer, right_expr_value.get_basic_value_enum()); + self.llvm.builder.build_store(left_ptr_val, right_expr_val.get_basic_value_enum()); Ok(()) } From 6496fa11171f809713d6a9e1e13bcb49b795dcba Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Tue, 25 Jun 2024 14:56:22 +0200 Subject: [PATCH 07/20] wip --- .../src/diagnostics/error_codes/E098.md | 19 ++++++++ src/codegen/generators/statement_generator.rs | 4 -- src/validation/statement.rs | 44 ++++++++++++++++++- .../tests/assignment_validation_tests.rs | 39 ++++++++++++++++ 4 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md new file mode 100644 index 0000000000..8262561bdb --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md @@ -0,0 +1,19 @@ +# Invalid REF= assignment + +For `REF=` to be valid, the left-hand side of the assignment must be a variable declared +with the `REFERENCE TO` keyword and the right hand side must be a variable of the type +that is being referenced. + +For example assignments such as the following are invalid + +```smalltalk +VAR + foo : DINT; + bar : DINT; + refFoo : REFERENCE TO DINT; +END_VAR + +refFoo REF= 5; // `5` is not a variable +foo REF= bar; // `foo` is not declared with `REFERENCE TO` +refFoo REF= refFoo; // pointing to oneself doesn't make sense +``` \ No newline at end of file diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index b712a61106..beb03c7ea0 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -233,10 +233,6 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { } } - // Rules: - // lhs of REF= must be reference declared with `REFERENCE TO` (i.e. a "reference to type") - // rhs of REF= must be lvalue - /// Generates IR for a `REF=` assignment, which is syntactic sugar for a `REF(...)` call on the /// right side of an assignment. Specifically `foo REF= bar` and `foo := REF(bar)` are the same. /// diff --git a/src/validation/statement.rs b/src/validation/statement.rs index eb91f49d83..b216ae2943 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -1,6 +1,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use std::mem::discriminant; +use plc_ast::ast::Assignment; use plc_ast::control_statements::ForLoopStatement; use plc_ast::{ ast::{ @@ -85,14 +86,20 @@ pub fn visit_statement( visit_statement(validator, &data.left, context); visit_statement(validator, &data.right, context); - validate_assignment(validator, &data.right, Some(&data.left), &statement.get_location(), context); + validate_assignment(validator, &data.right, Some(&data.left), &statement.location, context); validate_array_assignment(validator, context, statement); } AstStatement::OutputAssignment(data) => { visit_statement(validator, &data.left, context); visit_statement(validator, &data.right, context); - validate_assignment(validator, &data.right, Some(&data.left), &statement.get_location(), context); + validate_assignment(validator, &data.right, Some(&data.left), &statement.location, context); + } + AstStatement::RefAssignment(data) => { + visit_statement(validator, &data.left, context); + visit_statement(validator, &data.right, context); + + validate_ref_assignment(validator, context, data, &statement.location); } AstStatement::CallStatement(data) => { validate_call(validator, &data.operator, data.parameters.as_deref(), &context.set_is_call()); @@ -762,6 +769,39 @@ fn validate_call_by_ref(validator: &mut Validator, param: &VariableIndexEntry, a } } +// For `foo REF= bar` to be valid we need to check if +// - the left-hand side is a reference declared with the `REFERENCE TO` keyword and +// - the right-hand side is a lvalue +fn validate_ref_assignment( + validator: &mut Validator, + context: &ValidationContext, + assignment: &Assignment, + location: &SourceLocation, +) { + // TODO: Validate if variable with `REFERENCE TO` is initialized in a VAR block + // TODO: Hmm, I feel like the `is_auto_deref` check alone isn't sufficient? For example + // VAR_IN_OUT is also flagged as auto-deref afaik? + // Assert that the lhs is a variable declared with `REFERENCE TO` + let Some(StatementAnnotation::Variable { is_auto_deref, .. }) = context.annotations.get(&assignment.left) + else { + todo!("should exist? {:#?}", context.annotations.get(&assignment.left)); + }; + + if !is_auto_deref { + validator.push_diagnostic( + Diagnostic::new(format!("Invalid assignment, lhs must be declared with REFERENCE TO")) + .with_location(location), + ); + } + + // Assert that the rhs is a variable (lvalue) and of the same type the rhs is pointing to + if !assignment.right.is_reference() { + validator.push_diagnostic( + Diagnostic::new(format!("Invalid assignment, rhs must be a reference")).with_location(location), + ); + } +} + fn validate_assignment( validator: &mut Validator, right: &AstNode, diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 40da9e7fc4..eba23888b6 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1217,3 +1217,42 @@ fn void_assignment_validation() { "###) } + +// TODO: Think of an edge-case here, some variable that has the auto-deref trait on which +#[test] +fn ref_assignment() { + let diagnostics = parse_and_validate_buffered( + " + FUNCTION main + VAR + foo : DINT; + refToFoo : REF_TO DINT; + referenceToFoo : REFERENCE TO DINT; + END_VAR + + // Valid + referenceToFoo REF= foo; + + // Invalid + foo REF= foo; + refToFoo REF= foo; + referenceToFoo REF= referenceToFoo; + END_FUNCTION + ", + ); + + assert_snapshot!(diagnostics, @r###" + error[E001]: Invalid assignment, lhs must be declared with REFERENCE TO + ┌─ :13:13 + │ + 13 │ foo REF= foo; + │ ^^^^^^^^^^^^ Invalid assignment, lhs must be declared with REFERENCE TO + + error[E001]: Invalid assignment, lhs must be declared with REFERENCE TO + ┌─ :14:13 + │ + 14 │ refToFoo REF= foo; + │ ^^^^^^^^^^^^^^^^^ Invalid assignment, lhs must be declared with REFERENCE TO + + "###) +} From d4b0b0075d20ad1dad81e53fc2be3a6d8b5c7abd Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Wed, 26 Jun 2024 10:07:11 +0200 Subject: [PATCH 08/20] Add REF= validation --- compiler/plc_ast/src/ast.rs | 13 +++--- .../src/diagnostics/diagnostics_registry.rs | 1 + src/parser.rs | 20 ++++----- src/parser/expressions_parser.rs | 1 - src/parser/tests/statement_parser_tests.rs | 2 +- src/resolver.rs | 7 +++ src/validation/statement.rs | 45 ++++++++++++++----- .../tests/assignment_validation_tests.rs | 29 +++++++++--- 8 files changed, 80 insertions(+), 38 deletions(-) diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 7e55f5adbc..8e30b1ac9a 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -424,14 +424,11 @@ impl Debug for DataTypeDeclaration { impl DataTypeDeclaration { pub fn temp(&mut self) { - match self { - Self::DataTypeDefinition { data_type, .. } => match data_type { - DataType::PointerType { ref mut auto_deref, .. } => { - *auto_deref = true; - } - _ => {} - }, - _ => {} + if let Self::DataTypeDefinition { + data_type: DataType::PointerType { ref mut auto_deref, .. }, .. + } = self + { + *auto_deref = true; } } diff --git a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs index f076bdf045..82c01ac29e 100644 --- a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs +++ b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs @@ -199,6 +199,7 @@ lazy_static! { E095, Error, include_str!("./error_codes/E095.md"), // Action call without `()` E096, Warning, include_str!("./error_codes/E096.md"), // Integer Condition E097, Error, include_str!("./error_codes/E097.md"), // Invalid Array Range + E098, Error, include_str!("./error_codes/E098.md"), // Invalid `REF=` assignment ); } diff --git a/src/parser.rs b/src/parser.rs index fb88da20e4..0ec28eeefb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1123,17 +1123,15 @@ fn parse_variable_line(lexer: &mut ParseSession) -> Vec { }); } lexer.advance(); - } else { - if let Some((data_type, initializer)) = parse_full_data_type_definition(lexer, None) { - for (name, range) in var_names { - variables.push(Variable { - name, - data_type_declaration: data_type.clone(), - location: lexer.source_range_factory.create_range(range), - initializer: initializer.clone(), - address: address.clone(), - }); - } + } else if let Some((data_type, initializer)) = parse_full_data_type_definition(lexer, None) { + for (name, range) in var_names { + variables.push(Variable { + name, + data_type_declaration: data_type.clone(), + location: lexer.source_range_factory.create_range(range), + initializer: initializer.clone(), + address: address.clone(), + }); } } variables diff --git a/src/parser/expressions_parser.rs b/src/parser/expressions_parser.rs index 9f251f5618..c9760451ff 100644 --- a/src/parser/expressions_parser.rs +++ b/src/parser/expressions_parser.rs @@ -1,6 +1,5 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder -use crate::builtins::get_builtin; use crate::{ expect_token, lexer::Token::*, diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index 4d20ec6537..76017efd2e 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -1,6 +1,6 @@ use crate::{ parser::tests::{empty_stmt, ref_to}, - test_utils::tests::{parse, parse_and_preprocess}, + test_utils::tests::parse, typesystem::DINT_TYPE, }; use insta::assert_snapshot; diff --git a/src/resolver.rs b/src/resolver.rs index bb44fcd907..37b52b84b4 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -433,6 +433,13 @@ impl StatementAnnotation { } } + pub fn is_auto_deref(&self) -> bool { + match self { + StatementAnnotation::Variable { is_auto_deref, .. } => *is_auto_deref, + _ => false, + } + } + pub fn data_type(type_name: &str) -> Self { StatementAnnotation::Type { type_name: type_name.into() } } diff --git a/src/validation/statement.rs b/src/validation/statement.rs index b216ae2943..02ceacfb06 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -99,7 +99,7 @@ pub fn visit_statement( visit_statement(validator, &data.left, context); visit_statement(validator, &data.right, context); - validate_ref_assignment(validator, context, data, &statement.location); + validate_ref_assignment(context, validator, data, &statement.location); } AstStatement::CallStatement(data) => { validate_call(validator, &data.operator, data.parameters.as_deref(), &context.set_is_call()); @@ -773,31 +773,52 @@ fn validate_call_by_ref(validator: &mut Validator, param: &VariableIndexEntry, a // - the left-hand side is a reference declared with the `REFERENCE TO` keyword and // - the right-hand side is a lvalue fn validate_ref_assignment( - validator: &mut Validator, context: &ValidationContext, + validator: &mut Validator, assignment: &Assignment, - location: &SourceLocation, + assignment_location: &SourceLocation, ) { + // TODO: Improve error messages? // TODO: Validate if variable with `REFERENCE TO` is initialized in a VAR block // TODO: Hmm, I feel like the `is_auto_deref` check alone isn't sufficient? For example // VAR_IN_OUT is also flagged as auto-deref afaik? + // Assert that the lhs is a variable declared with `REFERENCE TO` - let Some(StatementAnnotation::Variable { is_auto_deref, .. }) = context.annotations.get(&assignment.left) - else { - todo!("should exist? {:#?}", context.annotations.get(&assignment.left)); - }; + if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_auto_deref) { + validator.push_diagnostic( + Diagnostic::new("Invalid assignment, expected a variable declared with `REFERENCE TO`") + .with_location(&assignment.left.location) + .with_error_code("E098"), + ); + } - if !is_auto_deref { + // Assert that the rhs is NOT a variable declared with `REFERENCE TO` + if context.annotations.get(&assignment.right).is_some_and(StatementAnnotation::is_auto_deref) { validator.push_diagnostic( - Diagnostic::new(format!("Invalid assignment, lhs must be declared with REFERENCE TO")) - .with_location(location), + Diagnostic::new("Invalid assignment, variable must not be declared with `REFERENCE TO`") + .with_location(&assignment.right.location) + .with_error_code("E098"), ); } - // Assert that the rhs is a variable (lvalue) and of the same type the rhs is pointing to + // Assert that the rhs is a variable that can be referenced if !assignment.right.is_reference() { validator.push_diagnostic( - Diagnostic::new(format!("Invalid assignment, rhs must be a reference")).with_location(location), + Diagnostic::new("Invalid assignment, expected a reference") + .with_location(&assignment.right.location) + .with_error_code("E098"), + ); + } + + // Lastly, assert the type the lhs references matches with the rhs + let type_lhs = context.annotations.get_type(&assignment.left, context.index); + let type_rhs = context.annotations.get_type(&assignment.right, context.index); + + if type_lhs != type_rhs { + validator.push_diagnostic( + Diagnostic::new("Invalid assignment, types differ") + .with_location(assignment_location) + .with_error_code("E098"), ); } } diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index eba23888b6..791ac96a06 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1236,23 +1236,42 @@ fn ref_assignment() { // Invalid foo REF= foo; refToFoo REF= foo; + referenceToFoo REF= 0; referenceToFoo REF= referenceToFoo; END_FUNCTION ", ); assert_snapshot!(diagnostics, @r###" - error[E001]: Invalid assignment, lhs must be declared with REFERENCE TO + error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` ┌─ :13:13 │ 13 │ foo REF= foo; - │ ^^^^^^^^^^^^ Invalid assignment, lhs must be declared with REFERENCE TO + │ ^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` - error[E001]: Invalid assignment, lhs must be declared with REFERENCE TO + error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` ┌─ :14:13 │ 14 │ refToFoo REF= foo; - │ ^^^^^^^^^^^^^^^^^ Invalid assignment, lhs must be declared with REFERENCE TO + │ ^^^^^^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` - "###) + error[E098]: Invalid assignment, types differ + ┌─ :14:13 + │ + 14 │ refToFoo REF= foo; + │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ + + error[E098]: Invalid assignment, expected a reference + ┌─ :15:33 + │ + 15 │ referenceToFoo REF= 0; + │ ^ Invalid assignment, expected a reference + + error[E098]: Invalid assignment, variable must not be declared with `REFERENCE TO` + ┌─ :16:33 + │ + 16 │ referenceToFoo REF= referenceToFoo; + │ ^^^^^^^^^^^^^^ Invalid assignment, variable must not be declared with `REFERENCE TO` + + "###); } From 75133b9479eafc1f99330fd04d77e490ad7a3182 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Wed, 26 Jun 2024 14:45:37 +0200 Subject: [PATCH 09/20] add is_reference_to field --- compiler/plc_ast/src/ast.rs | 6 ++++- src/index/tests/index_tests.rs | 4 +++ ...generates_inline_pointer_to_pointer-2.snap | 1 + ...g_generates_inline_pointer_to_pointer.snap | 1 + ..._processing_generates_inline_pointers.snap | 1 + ...g_generates_pointer_to_pointer_type-2.snap | 1 + ...ing_generates_pointer_to_pointer_type.snap | 1 + src/index/visitor.rs | 5 +++- src/parser.rs | 7 ++++- .../parse_error_statements_tests.rs | 2 ++ ...r_tests__global_pointer_declaration-2.snap | 1 + ...ser_tests__global_pointer_declaration.snap | 1 + ..._type_parser_tests__pointer_type_test.snap | 1 + ...sts__type_parser_tests__ref_type_test.snap | 1 + src/parser/tests/statement_parser_tests.rs | 1 + src/resolver.rs | 4 ++- src/resolver/generics.rs | 10 ++++--- .../tests/resolve_expressions_tests.rs | 1 + src/tests/adr/vla_adr.rs | 1 + src/typesystem.rs | 8 ++++++ src/validation/variable.rs | 27 ++++++++++++++++--- 21 files changed, 74 insertions(+), 11 deletions(-) diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 8e30b1ac9a..4203809905 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -425,10 +425,12 @@ impl Debug for DataTypeDeclaration { impl DataTypeDeclaration { pub fn temp(&mut self) { if let Self::DataTypeDefinition { - data_type: DataType::PointerType { ref mut auto_deref, .. }, .. + data_type: DataType::PointerType { ref mut auto_deref, ref mut is_reference_to, .. }, + .. } = self { *auto_deref = true; + *is_reference_to = true; } } @@ -497,6 +499,8 @@ pub enum DataType { name: Option, referenced_type: Box, auto_deref: bool, + /// Indicates if the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT` + is_reference_to: bool, }, StringType { name: Option, diff --git a/src/index/tests/index_tests.rs b/src/index/tests/index_tests.rs index b9dc72dc62..2e8eb9dacb 100644 --- a/src/index/tests/index_tests.rs +++ b/src/index/tests/index_tests.rs @@ -1253,6 +1253,7 @@ fn pointer_and_in_out_pointer_should_not_conflict() { name: "__main_x".to_string(), inner_type_name: "INT".to_string(), auto_deref: false, + is_reference_to: false, } ); @@ -1264,6 +1265,7 @@ fn pointer_and_in_out_pointer_should_not_conflict() { name: "__auto_pointer_to_INT".to_string(), inner_type_name: "INT".to_string(), auto_deref: true, + is_reference_to: false, } ); } @@ -1303,6 +1305,7 @@ fn pointer_and_in_out_pointer_should_not_conflict_2() { name: "__main_x".to_string(), inner_type_name: "INT".to_string(), auto_deref: false, + is_reference_to: false, } ); @@ -1314,6 +1317,7 @@ fn pointer_and_in_out_pointer_should_not_conflict_2() { name: "__auto_pointer_to_INT".to_string(), inner_type_name: "INT".to_string(), auto_deref: true, + is_reference_to: false, } ); } diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap index b0b33f5c00..e3895d6f10 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer-2.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "__foo_inline_pointer_", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: Some( diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap index dbe8dfb22c..8acc34ac46 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointer_to_pointer.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: Some( diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap index b1ea8bdafb..be433f8b03 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_inline_pointers.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: Some( diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap index 60c2f8729f..6bd5e157f6 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type-2.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "__pointer_to_pointer", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: None, diff --git a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap index 92218484ad..f8a2939b47 100644 --- a/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap +++ b/src/index/tests/snapshots/rusty__index__tests__index_tests__pre_processing_generates_pointer_to_pointer_type.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: None, diff --git a/src/index/visitor.rs b/src/index/visitor.rs index 9d3bf74410..54d40cc838 100644 --- a/src/index/visitor.rs +++ b/src/index/visitor.rs @@ -273,6 +273,7 @@ fn register_byref_pointer_type_for(index: &mut Index, inner_type_name: &str) -> name: type_name.clone(), inner_type_name: inner_type_name.to_string(), auto_deref: true, + is_reference_to: false, }, nature: TypeNature::Any, location: SourceLocation::internal(), @@ -400,12 +401,13 @@ fn visit_data_type(index: &mut Index, type_declaration: &UserTypeDeclaration) { DataType::ArrayType { name: Some(name), bounds, referenced_type, .. } => { visit_array(bounds, index, scope, referenced_type, name, type_declaration); } - DataType::PointerType { name: Some(name), referenced_type, auto_deref } => { + DataType::PointerType { name: Some(name), referenced_type, auto_deref, is_reference_to } => { let inner_type_name = referenced_type.get_name().expect("named datatype"); let information = DataTypeInformation::Pointer { name: name.clone(), inner_type_name: inner_type_name.into(), auto_deref: *auto_deref, + is_reference_to: *is_reference_to, }; let init = index.get_mut_const_expressions().maybe_add_constant_expression( @@ -574,6 +576,7 @@ fn visit_variable_length_array( location: SourceLocation::undefined(), }), auto_deref: false, + is_reference_to: false, }, location: SourceLocation::undefined(), scope: None, diff --git a/src/parser.rs b/src/parser.rs index 0ec28eeefb..28da3dc563 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -705,7 +705,12 @@ fn parse_pointer_definition( parse_data_type_definition(lexer, None).map(|(decl, initializer)| { ( DataTypeDeclaration::DataTypeDefinition { - data_type: DataType::PointerType { name, referenced_type: Box::new(decl), auto_deref: false }, + data_type: DataType::PointerType { + name, + referenced_type: Box::new(decl), + auto_deref: false, + is_reference_to: false, + }, location: lexer.source_range_factory.create_range(start_pos..lexer.last_range.end), scope: lexer.scope.clone(), }, diff --git a/src/parser/tests/parse_errors/parse_error_statements_tests.rs b/src/parser/tests/parse_errors/parse_error_statements_tests.rs index 5f6fb08a88..635dd03792 100644 --- a/src/parser/tests/parse_errors/parse_error_statements_tests.rs +++ b/src/parser/tests/parse_errors/parse_error_statements_tests.rs @@ -1129,6 +1129,7 @@ fn pointer_type_without_to_test() { location: SourceLocation::undefined(), }), auto_deref: false, + is_reference_to: false, }, location: SourceLocation::undefined(), initializer: None, @@ -1156,6 +1157,7 @@ fn pointer_type_with_wrong_keyword_to_test() { location: SourceLocation::undefined(), }), auto_deref: false, + is_reference_to: false, }, location: SourceLocation::undefined(), initializer: None, diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap index 604b464e1f..b7f3076003 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration-2.snap @@ -11,6 +11,7 @@ Variable { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, }, } diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap index 29dc5d6aea..fcdb7bc49c 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__global_pointer_declaration.snap @@ -11,6 +11,7 @@ Variable { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, }, } diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap index a5d326dd82..576dcbd116 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__pointer_type_test.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: None, diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap index 94c71479f5..332f301817 100644 --- a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__ref_type_test.snap @@ -11,6 +11,7 @@ UserTypeDeclaration { referenced_type: "INT", }, auto_deref: false, + is_reference_to: false, }, initializer: None, scope: None, diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index 76017efd2e..5b64abd5e2 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -323,6 +323,7 @@ fn reference_to_variable() { referenced_type: "DINT", }, auto_deref: true, + is_reference_to: true, }, }, }, diff --git a/src/resolver.rs b/src/resolver.rs index 37b52b84b4..7a173d28a2 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -393,6 +393,7 @@ pub enum StatementAnnotation { argument_type: ArgumentType, /// denotes whether this variable-reference should be automatically dereferenced when accessed is_auto_deref: bool, + // TODO: Maybe is_reference_to }, /// a reference to a function Function { @@ -1881,10 +1882,11 @@ pub(crate) fn add_pointer_type(index: &mut Index, inner_type_name: String) -> St name: new_type_name.clone(), initial_value: None, nature: TypeNature::Any, - information: crate::typesystem::DataTypeInformation::Pointer { + information: DataTypeInformation::Pointer { auto_deref: false, inner_type_name, name: new_type_name.clone(), + is_reference_to: false, }, location: SourceLocation::internal(), }); diff --git a/src/resolver/generics.rs b/src/resolver/generics.rs index 379460b346..6d7861b7e4 100644 --- a/src/resolver/generics.rs +++ b/src/resolver/generics.rs @@ -201,14 +201,18 @@ impl<'i> TypeAnnotator<'i> { .unwrap_or_else(|| member_name) .to_string() } - Some(DataTypeInformation::Pointer { name, inner_type_name, auto_deref: true }) => { + Some(DataTypeInformation::Pointer { name, inner_type_name, auto_deref: true, .. }) => { // This is an auto deref pointer (VAR_IN_OUT or VAR_INPUT {ref}) that points to a // generic. We first resolve the generic type, then create a new pointer type of // the combination let inner_type_name = self.find_or_create_datatype(inner_type_name, generics); let name = format!("{name}__{inner_type_name}"); // TODO: Naming convention (see plc_util/src/convention.rs) - let new_type_info = - DataTypeInformation::Pointer { name: name.clone(), inner_type_name, auto_deref: true }; + let new_type_info = DataTypeInformation::Pointer { + name: name.clone(), + inner_type_name, + auto_deref: true, + is_reference_to: false, + }; // Registers a new pointer type to the index self.annotation_map.new_index.register_type(DataType { diff --git a/src/resolver/tests/resolve_expressions_tests.rs b/src/resolver/tests/resolve_expressions_tests.rs index 44d1537f19..5bfa008ae9 100644 --- a/src/resolver/tests/resolve_expressions_tests.rs +++ b/src/resolver/tests/resolve_expressions_tests.rs @@ -3385,6 +3385,7 @@ fn address_of_is_annotated_correctly() { if let Some(&StatementAnnotation::Value { resulting_type }) = annotations.get(s).as_ref() { assert_eq!( Some(&DataTypeInformation::Pointer { + is_reference_to: false, auto_deref: false, inner_type_name: "INT".to_string(), name: "__POINTER_TO_INT".to_string(), diff --git a/src/tests/adr/vla_adr.rs b/src/tests/adr/vla_adr.rs index e4ba8d7512..29c9816894 100644 --- a/src/tests/adr/vla_adr.rs +++ b/src/tests/adr/vla_adr.rs @@ -104,6 +104,7 @@ fn representation() { name: "__ptr_to___arr_vla_1_dint", inner_type_name: "__arr_vla_1_dint", auto_deref: false, + is_reference_to: false, }, nature: Any, location: SourceLocation { diff --git a/src/typesystem.rs b/src/typesystem.rs index eaead48113..810387c9d8 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -391,6 +391,8 @@ pub enum DataTypeInformation { name: TypeId, inner_type_name: TypeId, auto_deref: bool, + /// Indicates if the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT` + is_reference_to: bool, }, Integer { name: TypeId, @@ -559,6 +561,12 @@ impl DataTypeInformation { } } + /// Returns true if the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT`. + pub fn is_reference_to(&self) -> bool { + + matches!(self, DataTypeInformation::Pointer { is_reference_to: true, .. }) + } + pub fn is_aggregate(&self) -> bool { matches!( self, diff --git a/src/validation/variable.rs b/src/validation/variable.rs index f841b0001f..5c2e7c4a3a 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -1,15 +1,15 @@ -use plc_ast::ast::{ArgumentProperty, Pou, PouType, Variable, VariableBlock, VariableBlockType}; +use plc_ast::ast::{ArgumentProperty, AstNode, Pou, PouType, Variable, VariableBlock, VariableBlockType}; use plc_diagnostics::diagnostics::Diagnostic; -use crate::typesystem::DataTypeInformation; -use crate::{index::const_expressions::ConstExpression, resolver::AnnotationMap}; - use super::{ array::validate_array_assignment, statement::{validate_enum_variant_assignment, visit_statement}, types::{data_type_is_fb_or_class_instance, visit_data_type_declaration}, ValidationContext, Validator, Validators, }; +use crate::index::VariableIndexEntry; +use crate::typesystem::DataTypeInformation; +use crate::{index::const_expressions::ConstExpression, resolver::AnnotationMap}; pub fn visit_variable_block( validator: &mut Validator, @@ -144,6 +144,8 @@ fn validate_variable( if let Some(v_entry) = context.index.find_variable(context.qualifier, &[&variable.name]) { if let Some(initializer) = &variable.initializer { + validate_reference_to_initialization(validator, context, v_entry, &initializer); + // Assume `foo : ARRAY[1..5] OF DINT := [...]`, here the first function call validates the // assignment as a whole whereas the second function call (`visit_statement`) validates the // initializer in case it has further sub-assignments. @@ -208,6 +210,23 @@ fn validate_variable( } } +/// Returns a diagnostic if a `REFERENCE TO` variable was initialized in its declaration +fn validate_reference_to_initialization( + validator: &mut Validator, + context: &ValidationContext, + v_entry: &VariableIndexEntry, + initializer: &&AstNode, +) { + let variable_type = context.index.find_effective_type_by_name(v_entry.get_type_name()); + if variable_type.is_some_and(|ty| ty.get_type_information().is_reference_to()) { + validator.push_diagnostic( + Diagnostic::new("REFERENCE TO variables can not be initialized in their declaration") + .with_location(&initializer.location) + .with_error_code("E098"), + ); + } +} + #[cfg(test)] mod variable_validator_tests { use insta::assert_snapshot; From b0861e581db866b85c281bd9d43197760996483f Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Wed, 26 Jun 2024 14:49:56 +0200 Subject: [PATCH 10/20] Add unit test --- src/resolver.rs | 16 +++++--- .../tests/resolve_expressions_tests.rs | 39 ++++++++++++++----- src/resolver/tests/resolve_literals_tests.rs | 3 +- src/tests/adr/annotated_ast_adr.rs | 9 ++++- src/typesystem.rs | 3 +- src/validation/statement.rs | 10 ++--- .../tests/assignment_validation_tests.rs | 36 ++++++++++------- src/validation/variable.rs | 4 +- tests/integration/cfc/resolver_tests.rs | 1 + ...r_tests__action_variables_annotated-3.snap | 1 + ...r_tests__action_variables_annotated-4.snap | 1 + 11 files changed, 80 insertions(+), 43 deletions(-) diff --git a/src/resolver.rs b/src/resolver.rs index 7a173d28a2..ac5ac50095 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -393,7 +393,8 @@ pub enum StatementAnnotation { argument_type: ArgumentType, /// denotes whether this variable-reference should be automatically dereferenced when accessed is_auto_deref: bool, - // TODO: Maybe is_reference_to + /// denotes whether this variable is declared as `REFERENCE TO` (e.g. `foo : REFERENCE TO DINT`) + is_reference_to: bool, }, /// a reference to a function Function { @@ -435,10 +436,11 @@ impl StatementAnnotation { } pub fn is_auto_deref(&self) -> bool { - match self { - StatementAnnotation::Variable { is_auto_deref, .. } => *is_auto_deref, - _ => false, - } + matches!(self, StatementAnnotation::Variable { is_auto_deref: true, .. }) + } + + pub fn is_reference_to(&self) -> bool { + matches!(self, StatementAnnotation::Variable { is_reference_to: true, .. }) } pub fn data_type(type_name: &str) -> Self { @@ -1636,7 +1638,7 @@ impl<'i> TypeAnnotator<'i> { else { unreachable!("expected a vla reference, but got {statement:#?}"); }; - if let DataTypeInformation::Pointer { inner_type_name, .. } = &self + if let DataTypeInformation::Pointer { inner_type_name, is_reference_to, .. } = &self .index .get_effective_type_or_void_by_name( members.first().expect("internal VLA struct ALWAYS has this member").get_type_name(), @@ -1672,6 +1674,7 @@ impl<'i> TypeAnnotator<'i> { constant: false, argument_type, is_auto_deref: false, + is_reference_to: *is_reference_to, }; self.annotation_map.annotate_type_hint(statement, hint_annotation) } @@ -1947,6 +1950,7 @@ fn to_variable_annotation( constant: v.is_constant() || constant_override, argument_type: v.get_declaration_type(), is_auto_deref, + is_reference_to: v_type.get_type_information().is_reference_to(), } } diff --git a/src/resolver/tests/resolve_expressions_tests.rs b/src/resolver/tests/resolve_expressions_tests.rs index 5bfa008ae9..44872aec2b 100644 --- a/src/resolver/tests/resolve_expressions_tests.rs +++ b/src/resolver/tests/resolve_expressions_tests.rs @@ -162,7 +162,8 @@ fn cast_expressions_of_enum_with_resolves_types() { qualified_name: "MyEnum.a".to_string(), constant: true, argument_type: ArgumentType::ByVal(VariableType::Global), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }) ); @@ -178,7 +179,8 @@ fn cast_expressions_of_enum_with_resolves_types() { qualified_name: "MyEnum.b".to_string(), constant: true, argument_type: ArgumentType::ByVal(VariableType::Global), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }) ); } @@ -1313,6 +1315,7 @@ fn function_expression_resolves_to_the_function_itself_not_its_return_type() { constant: false, is_auto_deref: false, argument_type: ArgumentType::ByVal(VariableType::Return), + is_reference_to: false, }), foo_annotation ); @@ -1574,6 +1577,7 @@ fn qualified_expressions_dont_fallback_to_globals() { constant: false, is_auto_deref: false, argument_type: ArgumentType::ByVal(VariableType::Input), + is_reference_to: false, }), annotations.get(&statements[1]) ); @@ -1815,6 +1819,7 @@ fn method_references_are_resolved() { constant: false, is_auto_deref: false, argument_type: ArgumentType::ByVal(VariableType::Return), + is_reference_to: false, }), annotation ); @@ -2448,7 +2453,8 @@ fn struct_member_explicit_initialization_test() { qualified_name: "myStruct.var1".to_string(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Input), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }), annotations.get(left) ); @@ -2462,7 +2468,8 @@ fn struct_member_explicit_initialization_test() { qualified_name: "myStruct.var2".to_string(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Input), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }), annotations.get(left) ); @@ -2944,6 +2951,7 @@ fn action_body_gets_resolved() { constant: false, is_auto_deref: false, argument_type: ArgumentType::ByVal(VariableType::Local), + is_reference_to: false, }), a ); @@ -3385,7 +3393,7 @@ fn address_of_is_annotated_correctly() { if let Some(&StatementAnnotation::Value { resulting_type }) = annotations.get(s).as_ref() { assert_eq!( Some(&DataTypeInformation::Pointer { - is_reference_to: false, + is_reference_to: false, auto_deref: false, inner_type_name: "INT".to_string(), name: "__POINTER_TO_INT".to_string(), @@ -3469,7 +3477,8 @@ fn call_explicit_parameter_name_is_resolved() { qualified_name: "fb.b".to_string(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Input), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }), annotations.get(b.as_ref()) ); @@ -3480,7 +3489,8 @@ fn call_explicit_parameter_name_is_resolved() { qualified_name: "fb.a".to_string(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Input), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }), annotations.get(a) ); @@ -3702,7 +3712,8 @@ fn function_block_initialization_test() { qualified_name: "TON.PT".into(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Input), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, } ) } else { @@ -3837,7 +3848,8 @@ fn resolve_return_variable_in_nested_call() { qualified_name: "main.main".to_string(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Return), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, } ) } @@ -5082,6 +5094,7 @@ fn annotate_variable_in_parent_class() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5097,6 +5110,7 @@ fn annotate_variable_in_parent_class() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5135,6 +5149,7 @@ fn annotate_variable_in_grandparent_class() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5180,6 +5195,7 @@ fn annotate_variable_in_field() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5237,6 +5253,7 @@ fn annotate_method_in_super() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5252,6 +5269,7 @@ fn annotate_method_in_super() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5267,6 +5285,7 @@ fn annotate_method_in_super() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5282,6 +5301,7 @@ fn annotate_method_in_super() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); @@ -5297,6 +5317,7 @@ fn annotate_method_in_super() { constant: false, argument_type: ArgumentType::ByVal(VariableType::Local,), is_auto_deref: false, + is_reference_to: false, }, annotation.unwrap() ); diff --git a/src/resolver/tests/resolve_literals_tests.rs b/src/resolver/tests/resolve_literals_tests.rs index 559da46845..42d39cd54d 100644 --- a/src/resolver/tests/resolve_literals_tests.rs +++ b/src/resolver/tests/resolve_literals_tests.rs @@ -312,7 +312,8 @@ fn enum_literals_target_are_annotated() { qualified_name: "Color.Red".into(), constant: true, argument_type: ArgumentType::ByVal(crate::index::VariableType::Global), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, }), annotations.get(target) ); diff --git a/src/tests/adr/annotated_ast_adr.rs b/src/tests/adr/annotated_ast_adr.rs index bdfc003e7c..6845bd2163 100644 --- a/src/tests/adr/annotated_ast_adr.rs +++ b/src/tests/adr/annotated_ast_adr.rs @@ -49,7 +49,8 @@ fn references_to_variables_are_annotated() { qualified_name: "prg.a".into(), constant: false, argument_type: ArgumentType::ByVal(VariableType::Local), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, } ); @@ -61,7 +62,8 @@ fn references_to_variables_are_annotated() { qualified_name: "gX".into(), constant: true, argument_type: ArgumentType::ByVal(VariableType::Global), - is_auto_deref: false + is_auto_deref: false, + is_reference_to: false, } ); } @@ -105,6 +107,7 @@ fn different_types_of_annotations() { resulting_type: "SINT".into(), // the variable's type constant: false, // whether this variable is a constant or not is_auto_deref: false, // whether this pointerType should be automatically dereferenced + is_reference_to: false, // whether this pointerType was declared as `REFERENCE TO`, making it auto-deref by default argument_type: ArgumentType::ByVal(VariableType::Input), // the type of declaration }) ); @@ -154,6 +157,7 @@ fn different_types_of_annotations() { resulting_type: "INT".into(), constant: false, is_auto_deref: false, + is_reference_to: false, argument_type: ArgumentType::ByVal(VariableType::Input), }) ); @@ -166,6 +170,7 @@ fn different_types_of_annotations() { resulting_type: "INT".into(), constant: false, is_auto_deref: false, + is_reference_to: false, argument_type: ArgumentType::ByVal(VariableType::Input), }) ); diff --git a/src/typesystem.rs b/src/typesystem.rs index 810387c9d8..8350f8a77c 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -563,8 +563,7 @@ impl DataTypeInformation { /// Returns true if the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT`. pub fn is_reference_to(&self) -> bool { - - matches!(self, DataTypeInformation::Pointer { is_reference_to: true, .. }) + matches!(self, DataTypeInformation::Pointer { is_reference_to: true, .. }) } pub fn is_aggregate(&self) -> bool { diff --git a/src/validation/statement.rs b/src/validation/statement.rs index 02ceacfb06..4c90d37726 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -779,12 +779,8 @@ fn validate_ref_assignment( assignment_location: &SourceLocation, ) { // TODO: Improve error messages? - // TODO: Validate if variable with `REFERENCE TO` is initialized in a VAR block - // TODO: Hmm, I feel like the `is_auto_deref` check alone isn't sufficient? For example - // VAR_IN_OUT is also flagged as auto-deref afaik? - // Assert that the lhs is a variable declared with `REFERENCE TO` - if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_auto_deref) { + if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_reference_to) { validator.push_diagnostic( Diagnostic::new("Invalid assignment, expected a variable declared with `REFERENCE TO`") .with_location(&assignment.left.location) @@ -793,7 +789,7 @@ fn validate_ref_assignment( } // Assert that the rhs is NOT a variable declared with `REFERENCE TO` - if context.annotations.get(&assignment.right).is_some_and(StatementAnnotation::is_auto_deref) { + if context.annotations.get(&assignment.right).is_some_and(StatementAnnotation::is_reference_to) { validator.push_diagnostic( Diagnostic::new("Invalid assignment, variable must not be declared with `REFERENCE TO`") .with_location(&assignment.right.location) @@ -816,7 +812,7 @@ fn validate_ref_assignment( if type_lhs != type_rhs { validator.push_diagnostic( - Diagnostic::new("Invalid assignment, types differ") + Diagnostic::new(format!("Invalid assignment, types differ got {type_lhs:?} and {type_rhs:?}")) .with_location(assignment_location) .with_error_code("E098"), ); diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 791ac96a06..56fa0c9108 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1225,12 +1225,14 @@ fn ref_assignment() { " FUNCTION main VAR - foo : DINT; - refToFoo : REF_TO DINT; - referenceToFoo : REFERENCE TO DINT; + foo : DINT; + refToFoo : REF_TO DINT; + referenceToFoo : REFERENCE TO DINT; + + // Invalid + referenceToFooInitialized : REFERENCE TO DINT := 5; END_VAR - // Valid referenceToFoo REF= foo; // Invalid @@ -1243,34 +1245,40 @@ fn ref_assignment() { ); assert_snapshot!(diagnostics, @r###" + error[E098]: REFERENCE TO variables can not be initialized in their declaration + ┌─ :9:67 + │ + 9 │ referenceToFooInitialized : REFERENCE TO DINT := 5; + │ ^ REFERENCE TO variables can not be initialized in their declaration + error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` - ┌─ :13:13 + ┌─ :15:13 │ - 13 │ foo REF= foo; + 15 │ foo REF= foo; │ ^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` - ┌─ :14:13 + ┌─ :16:13 │ - 14 │ refToFoo REF= foo; + 16 │ refToFoo REF= foo; │ ^^^^^^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` error[E098]: Invalid assignment, types differ - ┌─ :14:13 + ┌─ :16:13 │ - 14 │ refToFoo REF= foo; + 16 │ refToFoo REF= foo; │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ error[E098]: Invalid assignment, expected a reference - ┌─ :15:33 + ┌─ :17:33 │ - 15 │ referenceToFoo REF= 0; + 17 │ referenceToFoo REF= 0; │ ^ Invalid assignment, expected a reference error[E098]: Invalid assignment, variable must not be declared with `REFERENCE TO` - ┌─ :16:33 + ┌─ :18:33 │ - 16 │ referenceToFoo REF= referenceToFoo; + 18 │ referenceToFoo REF= referenceToFoo; │ ^^^^^^^^^^^^^^ Invalid assignment, variable must not be declared with `REFERENCE TO` "###); diff --git a/src/validation/variable.rs b/src/validation/variable.rs index 5c2e7c4a3a..5b53856612 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -1,4 +1,4 @@ -use plc_ast::ast::{ArgumentProperty, AstNode, Pou, PouType, Variable, VariableBlock, VariableBlockType}; +use plc_ast::ast::{ArgumentProperty, Pou, PouType, Variable, VariableBlock, VariableBlockType}; use plc_diagnostics::diagnostics::Diagnostic; use super::{ @@ -144,7 +144,7 @@ fn validate_variable( if let Some(v_entry) = context.index.find_variable(context.qualifier, &[&variable.name]) { if let Some(initializer) = &variable.initializer { - validate_reference_to_initialization(validator, context, v_entry, &initializer); + validate_reference_to_declaration(validator, context, variable, v_entry); // Assume `foo : ARRAY[1..5] OF DINT := [...]`, here the first function call validates the // assignment as a whole whereas the second function call (`visit_statement`) validates the diff --git a/tests/integration/cfc/resolver_tests.rs b/tests/integration/cfc/resolver_tests.rs index f11c866739..dd11728f65 100644 --- a/tests/integration/cfc/resolver_tests.rs +++ b/tests/integration/cfc/resolver_tests.rs @@ -99,6 +99,7 @@ fn function_block_calls_are_annotated_correctly() { Local, ), is_auto_deref: false, + is_reference_to: false, } "###); } diff --git a/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-3.snap b/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-3.snap index 5d46308218..5fdbebac50 100644 --- a/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-3.snap +++ b/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-3.snap @@ -11,5 +11,6 @@ Some( Local, ), is_auto_deref: false, + is_reference_to: false, }, ) diff --git a/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-4.snap b/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-4.snap index 1cb6d633ba..2d8cbaeaf2 100644 --- a/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-4.snap +++ b/tests/integration/cfc/snapshots/tests__integration__cfc__resolver_tests__action_variables_annotated-4.snap @@ -11,5 +11,6 @@ Some( Local, ), is_auto_deref: false, + is_reference_to: false, }, ) From 0e6d595a7ff56b619ee55c1af68803455cfecdf0 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Thu, 27 Jun 2024 12:35:39 +0200 Subject: [PATCH 11/20] Validation --- src/validation/statement.rs | 14 +++-- .../tests/assignment_validation_tests.rs | 48 ++++++++++------ src/validation/variable.rs | 57 +++++++++++++++---- 3 files changed, 85 insertions(+), 34 deletions(-) diff --git a/src/validation/statement.rs b/src/validation/statement.rs index 4c90d37726..93e18a2e6c 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -807,14 +807,18 @@ fn validate_ref_assignment( } // Lastly, assert the type the lhs references matches with the rhs - let type_lhs = context.annotations.get_type(&assignment.left, context.index); - let type_rhs = context.annotations.get_type(&assignment.right, context.index); + let type_lhs = context.annotations.get_type(&assignment.left, context.index).unwrap(); + let type_rhs = context.annotations.get_type(&assignment.right, context.index).unwrap(); if type_lhs != type_rhs { validator.push_diagnostic( - Diagnostic::new(format!("Invalid assignment, types differ got {type_lhs:?} and {type_rhs:?}")) - .with_location(assignment_location) - .with_error_code("E098"), + Diagnostic::new(format!( + "Invalid assignment, types differ got {} and {}", + get_datatype_name_or_slice(&validator.context, type_lhs), + get_datatype_name_or_slice(&validator.context, type_rhs), + )) + .with_location(assignment_location) + .with_error_code("E098"), ); } } diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 56fa0c9108..f6c48c2736 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1230,7 +1230,9 @@ fn ref_assignment() { referenceToFoo : REFERENCE TO DINT; // Invalid - referenceToFooInitialized : REFERENCE TO DINT := 5; + referenceToFooInitializedVariable : REFERENCE TO foo; + referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; + referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; END_VAR referenceToFoo REF= foo; @@ -1245,40 +1247,52 @@ fn ref_assignment() { ); assert_snapshot!(diagnostics, @r###" - error[E098]: REFERENCE TO variables can not be initialized in their declaration - ┌─ :9:67 + error[E098]: Invalid type, reference + ┌─ :9:55 │ - 9 │ referenceToFooInitialized : REFERENCE TO DINT := 5; - │ ^ REFERENCE TO variables can not be initialized in their declaration + 9 │ referenceToFooInitializedVariable : REFERENCE TO foo; + │ ^^^^^^^^^^^^^^^^ Invalid type, reference + + error[E098]: REFERENCE TO variables can not be initialized in their declaration + ┌─ :10:76 + │ + 10 │ referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; + │ ^ REFERENCE TO variables can not be initialized in their declaration + + error[E098]: Invalid type: array, pointer or bit + ┌─ :11:17 + │ + 11 │ referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid type: array, pointer or bit error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` - ┌─ :15:13 + ┌─ :17:13 │ - 15 │ foo REF= foo; + 17 │ foo REF= foo; │ ^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` - ┌─ :16:13 + ┌─ :18:13 │ - 16 │ refToFoo REF= foo; + 18 │ refToFoo REF= foo; │ ^^^^^^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` - error[E098]: Invalid assignment, types differ - ┌─ :16:13 + error[E098]: Invalid assignment, types differ got REF_TO DINT and DINT + ┌─ :18:13 │ - 16 │ refToFoo REF= foo; - │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ + 18 │ refToFoo REF= foo; + │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ got REF_TO DINT and DINT error[E098]: Invalid assignment, expected a reference - ┌─ :17:33 + ┌─ :19:33 │ - 17 │ referenceToFoo REF= 0; + 19 │ referenceToFoo REF= 0; │ ^ Invalid assignment, expected a reference error[E098]: Invalid assignment, variable must not be declared with `REFERENCE TO` - ┌─ :18:33 + ┌─ :20:33 │ - 18 │ referenceToFoo REF= referenceToFoo; + 20 │ referenceToFoo REF= referenceToFoo; │ ^^^^^^^^^^^^^^ Invalid assignment, variable must not be declared with `REFERENCE TO` "###); diff --git a/src/validation/variable.rs b/src/validation/variable.rs index 5b53856612..bd8cc25898 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -143,9 +143,9 @@ fn validate_variable( validate_array_ranges(validator, variable, context); if let Some(v_entry) = context.index.find_variable(context.qualifier, &[&variable.name]) { - if let Some(initializer) = &variable.initializer { - validate_reference_to_declaration(validator, context, variable, v_entry); + validate_reference_to_declaration(validator, context, variable, v_entry); + if let Some(initializer) = &variable.initializer { // Assume `foo : ARRAY[1..5] OF DINT := [...]`, here the first function call validates the // assignment as a whole whereas the second function call (`visit_statement`) validates the // initializer in case it has further sub-assignments. @@ -210,20 +210,53 @@ fn validate_variable( } } +// TODO: Make sure this check happens for REFERENCE TO only /// Returns a diagnostic if a `REFERENCE TO` variable was initialized in its declaration -fn validate_reference_to_initialization( +fn validate_reference_to_declaration( validator: &mut Validator, context: &ValidationContext, - v_entry: &VariableIndexEntry, - initializer: &&AstNode, + variable: &Variable, + variable_entry: &VariableIndexEntry, ) { - let variable_type = context.index.find_effective_type_by_name(v_entry.get_type_name()); - if variable_type.is_some_and(|ty| ty.get_type_information().is_reference_to()) { - validator.push_diagnostic( - Diagnostic::new("REFERENCE TO variables can not be initialized in their declaration") - .with_location(&initializer.location) - .with_error_code("E098"), - ); + let Some(variable_type) = context.index.find_effective_type_by_name(variable_entry.get_type_name()) + else { + return; + }; + + if variable_type.get_type_information().is_reference_to() { + let DataTypeInformation::Pointer { inner_type_name, .. } = variable_type.get_type_information() + else { + unreachable!("`REFERENCE TO` is defined as a pointer, hence this must exist") + }; + + // Check if there is an initalizer + if let Some(ref initializer) = variable.initializer { + if variable_type.get_type_information().is_reference_to() { + validator.push_diagnostic( + Diagnostic::new("REFERENCE TO variables can not be initialized in their declaration") + .with_location(&initializer.location) + .with_error_code("E098"), + ); + } + } + + // Reference + if context.index.find_member(context.qualifier.unwrap_or_default(), &inner_type_name).is_some() { + validator.push_diagnostic( + Diagnostic::new("Invalid type, reference") + .with_location(&variable_type.location) + .with_error_code("E098"), + ); + } + + let inner_type = context.index.find_effective_type_by_name(&inner_type_name); + if inner_type.is_some_and(|ty| ty.is_array() || ty.is_pointer() || ty.is_bit()) { + validator.push_diagnostic( + Diagnostic::new("Invalid type: array, pointer or bit ") + .with_location(&variable.location) + .with_error_code("E098"), + ); + } } } From 65e32ff31fca0dbe71cc6760370c65864ba1660a Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Fri, 28 Jun 2024 11:19:30 +0200 Subject: [PATCH 12/20] Polish --- compiler/plc_ast/src/ast.rs | 13 +--- .../src/diagnostics/diagnostics_registry.rs | 1 + .../src/diagnostics/error_codes/E099.md | 1 + err.conf | 3 - src/index/visitor.rs | 1 - src/lexer/tokens.rs | 2 +- src/parser.rs | 34 +++++----- src/parser/tests/statement_parser_tests.rs | 4 +- src/resolver.rs | 1 - src/typesystem.rs | 2 +- src/validation/statement.rs | 9 ++- .../tests/assignment_validation_tests.rs | 4 +- src/validation/variable.rs | 65 +++++++++---------- 13 files changed, 59 insertions(+), 81 deletions(-) create mode 100644 compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md delete mode 100644 err.conf diff --git a/compiler/plc_ast/src/ast.rs b/compiler/plc_ast/src/ast.rs index 4203809905..20ce291232 100644 --- a/compiler/plc_ast/src/ast.rs +++ b/compiler/plc_ast/src/ast.rs @@ -423,17 +423,6 @@ impl Debug for DataTypeDeclaration { } impl DataTypeDeclaration { - pub fn temp(&mut self) { - if let Self::DataTypeDefinition { - data_type: DataType::PointerType { ref mut auto_deref, ref mut is_reference_to, .. }, - .. - } = self - { - *auto_deref = true; - *is_reference_to = true; - } - } - pub fn get_name(&self) -> Option<&str> { match self { DataTypeDeclaration::DataTypeReference { referenced_type, .. } => Some(referenced_type.as_str()), @@ -499,7 +488,7 @@ pub enum DataType { name: Option, referenced_type: Box, auto_deref: bool, - /// Indicates if the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT` + /// Denotes whether the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT` is_reference_to: bool, }, StringType { diff --git a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs index 82c01ac29e..874f1c4eb1 100644 --- a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs +++ b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs @@ -200,6 +200,7 @@ lazy_static! { E096, Warning, include_str!("./error_codes/E096.md"), // Integer Condition E097, Error, include_str!("./error_codes/E097.md"), // Invalid Array Range E098, Error, include_str!("./error_codes/E098.md"), // Invalid `REF=` assignment + E099, Error, include_str!("./error_codes/E099.md"), // Invalid `REFERENCE TO` declaration ); } diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md new file mode 100644 index 0000000000..693a9040ab --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md @@ -0,0 +1 @@ +# Invalid `REFERENCE TO` declaration \ No newline at end of file diff --git a/err.conf b/err.conf deleted file mode 100644 index 04a0b3c5af..0000000000 --- a/err.conf +++ /dev/null @@ -1,3 +0,0 @@ -{ -"warning": ["E065", "E037"] -} diff --git a/src/index/visitor.rs b/src/index/visitor.rs index 54d40cc838..f0ecd18a1e 100644 --- a/src/index/visitor.rs +++ b/src/index/visitor.rs @@ -415,7 +415,6 @@ fn visit_data_type(index: &mut Index, type_declaration: &UserTypeDeclaration) { name, scope.clone(), ); - // TODO: Here index.register_type(typesystem::DataType { name: name.to_string(), initial_value: init, diff --git a/src/lexer/tokens.rs b/src/lexer/tokens.rs index c6fc2769af..d7c72a5394 100644 --- a/src/lexer/tokens.rs +++ b/src/lexer/tokens.rs @@ -159,7 +159,7 @@ pub enum Token { #[token("=>")] KeywordOutputAssignment, - #[token("REF=")] + #[token("REF=", ignore(case))] KeywordReferenceAssignment, #[token("(")] diff --git a/src/parser.rs b/src/parser.rs index 28da3dc563..f113e44549 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -676,9 +676,9 @@ fn parse_data_type_definition( if expect_keyword_to(lexer).is_some() { lexer.advance(); } - parse_pointer_definition(lexer, name, start_pos) + parse_pointer_definition(lexer, name, start_pos, false) } else if lexer.try_consume(&KeywordRef) { - parse_pointer_definition(lexer, name, lexer.last_range.start) + parse_pointer_definition(lexer, name, lexer.last_range.start, false) } else if lexer.try_consume(&KeywordParensOpen) { //enum without datatype parse_enum_type_definition(lexer, name) @@ -701,6 +701,7 @@ fn parse_pointer_definition( lexer: &mut ParseSession, name: Option, start_pos: usize, + is_reference_to: bool, ) -> Option<(DataTypeDeclaration, Option)> { parse_data_type_definition(lexer, None).map(|(decl, initializer)| { ( @@ -708,8 +709,8 @@ fn parse_pointer_definition( data_type: DataType::PointerType { name, referenced_type: Box::new(decl), - auto_deref: false, - is_reference_to: false, + auto_deref: is_reference_to, + is_reference_to, }, location: lexer.source_range_factory.create_range(start_pos..lexer.last_range.end), scope: lexer.scope.clone(), @@ -1114,21 +1115,15 @@ fn parse_variable_line(lexer: &mut ParseSession) -> Vec { // create variables with the same data type for each of the names let mut variables = vec![]; - if lexer.try_consume(&KeywordReferenceTo) { - let (mut data_type, initializer) = - parse_pointer_definition(lexer, None, lexer.last_range.start).unwrap(); - data_type.temp(); - for (name, range) in var_names { - variables.push(Variable { - name, - data_type_declaration: data_type.clone(), - location: lexer.source_range_factory.create_range(range), - initializer: initializer.clone(), - address: address.clone(), - }); - } - lexer.advance(); - } else if let Some((data_type, initializer)) = parse_full_data_type_definition(lexer, None) { + let parse_definition_opt = if lexer.try_consume(&KeywordReferenceTo) { + parse_pointer_definition(lexer, None, lexer.last_range.start, true) + } else { + parse_full_data_type_definition(lexer, None) + }; + + lexer.try_consume(&KeywordSemicolon); + + if let Some((data_type, initializer)) = parse_definition_opt { for (name, range) in var_names { variables.push(Variable { name, @@ -1139,6 +1134,7 @@ fn parse_variable_line(lexer: &mut ParseSession) -> Vec { }); } } + variables } diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index 5b64abd5e2..52903ad235 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -264,7 +264,7 @@ fn empty_parameter_assignments_in_call_statement() { } #[test] -fn reference_assignment_is_parsed() { +fn ref_assignment() { let result = &parse("PROGRAM main x REF= y END_PROGRAM").0.implementations[0]; insta::assert_debug_snapshot!(result.statements, @r###" [ @@ -291,7 +291,7 @@ fn reference_assignment_is_parsed() { } #[test] -fn reference_to_variable() { +fn reference_to_declaration() { let (result, diagnostics) = parse( r" FUNCTION foo diff --git a/src/resolver.rs b/src/resolver.rs index ac5ac50095..75aba1083a 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -1940,7 +1940,6 @@ fn to_variable_annotation( // real auto-deref pointer (inner_type_name.clone(), AUTO_DEREF) } - // TODO: Autoderef here _ => (v_type.get_name().to_string(), NO_DEREF), }; diff --git a/src/typesystem.rs b/src/typesystem.rs index 8350f8a77c..d34a189c0d 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -391,7 +391,7 @@ pub enum DataTypeInformation { name: TypeId, inner_type_name: TypeId, auto_deref: bool, - /// Indicates if the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT` + /// Denotes whether the variable was declared as `REFERENCE TO`, e.g. `foo : REFERENCE TO DINT` is_reference_to: bool, }, Integer { diff --git a/src/validation/statement.rs b/src/validation/statement.rs index 93e18a2e6c..a251a3be40 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -769,16 +769,15 @@ fn validate_call_by_ref(validator: &mut Validator, param: &VariableIndexEntry, a } } -// For `foo REF= bar` to be valid we need to check if -// - the left-hand side is a reference declared with the `REFERENCE TO` keyword and -// - the right-hand side is a lvalue +// TODO: Improve error messages? +/// Checks if `REF=` assignments are correct, specifically if the left-hand side is a reference declared +/// as `REFERENCE TO` and the right hand side is a lvalue of the same type that is being referenced. fn validate_ref_assignment( context: &ValidationContext, validator: &mut Validator, assignment: &Assignment, assignment_location: &SourceLocation, ) { - // TODO: Improve error messages? // Assert that the lhs is a variable declared with `REFERENCE TO` if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_reference_to) { validator.push_diagnostic( @@ -813,7 +812,7 @@ fn validate_ref_assignment( if type_lhs != type_rhs { validator.push_diagnostic( Diagnostic::new(format!( - "Invalid assignment, types differ got {} and {}", + "Invalid assignment, types differ (got {} and {})", get_datatype_name_or_slice(&validator.context, type_lhs), get_datatype_name_or_slice(&validator.context, type_rhs), )) diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index f6c48c2736..363aacbfc9 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1277,11 +1277,11 @@ fn ref_assignment() { 18 │ refToFoo REF= foo; │ ^^^^^^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` - error[E098]: Invalid assignment, types differ got REF_TO DINT and DINT + error[E098]: Invalid assignment, types differ (got REF_TO DINT and DINT) ┌─ :18:13 │ 18 │ refToFoo REF= foo; - │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ got REF_TO DINT and DINT + │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ (got REF_TO DINT and DINT) error[E098]: Invalid assignment, expected a reference ┌─ :19:33 diff --git a/src/validation/variable.rs b/src/validation/variable.rs index bd8cc25898..ba83006fbb 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -210,52 +210,49 @@ fn validate_variable( } } -// TODO: Make sure this check happens for REFERENCE TO only -/// Returns a diagnostic if a `REFERENCE TO` variable was initialized in its declaration +/// Returns a diagnostic if a `REFERENCE TO` variable is incorrectly declared (or initialized). fn validate_reference_to_declaration( validator: &mut Validator, context: &ValidationContext, variable: &Variable, variable_entry: &VariableIndexEntry, ) { - let Some(variable_type) = context.index.find_effective_type_by_name(variable_entry.get_type_name()) - else { - return; - }; + if let Some(variable_type) = context.index.find_effective_type_by_name(variable_entry.get_type_name()) { + if variable_type.get_type_information().is_reference_to() { + let DataTypeInformation::Pointer { inner_type_name, .. } = variable_type.get_type_information() + else { + unreachable!("`REFERENCE TO` is defined as a pointer, hence this must exist") + }; - if variable_type.get_type_information().is_reference_to() { - let DataTypeInformation::Pointer { inner_type_name, .. } = variable_type.get_type_information() - else { - unreachable!("`REFERENCE TO` is defined as a pointer, hence this must exist") - }; + // Assert that no initializers are present in the `REFERENCE TO` declaration + if let Some(ref initializer) = variable.initializer { + if variable_type.get_type_information().is_reference_to() { + validator.push_diagnostic( + Diagnostic::new("REFERENCE TO variables can not be initialized in their declaration") + .with_location(&initializer.location) + .with_error_code("E099"), + ); + } + } - // Check if there is an initalizer - if let Some(ref initializer) = variable.initializer { - if variable_type.get_type_information().is_reference_to() { + // Assert that the referenced type is no variable reference + if context.index.find_member(context.qualifier.unwrap_or_default(), &inner_type_name).is_some() { validator.push_diagnostic( - Diagnostic::new("REFERENCE TO variables can not be initialized in their declaration") - .with_location(&initializer.location) - .with_error_code("E098"), + Diagnostic::new("Invalid type, reference") + .with_location(&variable_type.location) + .with_error_code("E099"), ); } - } - // Reference - if context.index.find_member(context.qualifier.unwrap_or_default(), &inner_type_name).is_some() { - validator.push_diagnostic( - Diagnostic::new("Invalid type, reference") - .with_location(&variable_type.location) - .with_error_code("E098"), - ); - } - - let inner_type = context.index.find_effective_type_by_name(&inner_type_name); - if inner_type.is_some_and(|ty| ty.is_array() || ty.is_pointer() || ty.is_bit()) { - validator.push_diagnostic( - Diagnostic::new("Invalid type: array, pointer or bit ") - .with_location(&variable.location) - .with_error_code("E098"), - ); + // Lastly assert that the referenced type is no array, pointer or bit + let inner_type = context.index.find_effective_type_by_name(&inner_type_name); + if inner_type.is_some_and(|ty| ty.is_array() || ty.is_pointer() || ty.is_bit()) { + validator.push_diagnostic( + Diagnostic::new("Invalid type: array, pointer or bit ") + .with_location(&variable.location) + .with_error_code("E099"), + ); + } } } } From 04a38790ba29595645b4048202c2781133a9de3a Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Fri, 28 Jun 2024 13:07:28 +0200 Subject: [PATCH 13/20] further (failing) tests --- src/validation/statement.rs | 38 +++++----- .../tests/assignment_validation_tests.rs | 73 +++++++++++-------- src/validation/variable.rs | 23 ++++-- tests/correctness/pointers.rs | 70 ++++++++++++++++++ 4 files changed, 146 insertions(+), 58 deletions(-) diff --git a/src/validation/statement.rs b/src/validation/statement.rs index a251a3be40..efd8471e13 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -778,23 +778,23 @@ fn validate_ref_assignment( assignment: &Assignment, assignment_location: &SourceLocation, ) { - // Assert that the lhs is a variable declared with `REFERENCE TO` - if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_reference_to) { - validator.push_diagnostic( - Diagnostic::new("Invalid assignment, expected a variable declared with `REFERENCE TO`") - .with_location(&assignment.left.location) - .with_error_code("E098"), - ); - } - - // Assert that the rhs is NOT a variable declared with `REFERENCE TO` - if context.annotations.get(&assignment.right).is_some_and(StatementAnnotation::is_reference_to) { - validator.push_diagnostic( - Diagnostic::new("Invalid assignment, variable must not be declared with `REFERENCE TO`") - .with_location(&assignment.right.location) - .with_error_code("E098"), - ); - } + // // Assert that the lhs is a variable declared with `REFERENCE TO` + // if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_reference_to) { + // validator.push_diagnostic( + // Diagnostic::new("Invalid assignment, expected a variable declared with `REFERENCE TO`") + // .with_location(&assignment.left.location) + // .with_error_code("E098"), + // ); + // } + + // // Assert that the rhs is NOT a variable declared with `REFERENCE TO` + // if context.annotations.get(&assignment.right).is_some_and(StatementAnnotation::is_reference_to) { + // validator.push_diagnostic( + // Diagnostic::new("Invalid assignment, variable must not be declared with `REFERENCE TO`") + // .with_location(&assignment.right.location) + // .with_error_code("E098"), + // ); + // } // Assert that the rhs is a variable that can be referenced if !assignment.right.is_reference() { @@ -813,8 +813,8 @@ fn validate_ref_assignment( validator.push_diagnostic( Diagnostic::new(format!( "Invalid assignment, types differ (got {} and {})", - get_datatype_name_or_slice(&validator.context, type_lhs), - get_datatype_name_or_slice(&validator.context, type_rhs), + get_datatype_name_or_slice(validator.context, type_lhs), + get_datatype_name_or_slice(validator.context, type_rhs), )) .with_location(assignment_location) .with_error_code("E098"), diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 363aacbfc9..70c78b8820 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1223,6 +1223,15 @@ fn void_assignment_validation() { fn ref_assignment() { let diagnostics = parse_and_validate_buffered( " + TYPE + AliasedDINT : DINT; + AliasedArray : ARRAY[1..5] OF DINT; + END_TYPE + + VAR_GLOBAL + fooGlobal : DINT; + END_VAR + FUNCTION main VAR foo : DINT; @@ -1231,6 +1240,9 @@ fn ref_assignment() { // Invalid referenceToFooInitializedVariable : REFERENCE TO foo; + referenceToFooGlobalVariable : REFERENCE TO fooGlobal; + referenceToFooAliasedDINTType : REFERENCE TO AliasedDINT; + referenceToFooAliasedArrayType : REFERENCE TO AliasedArray; referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; END_VAR @@ -1247,53 +1259,52 @@ fn ref_assignment() { ); assert_snapshot!(diagnostics, @r###" - error[E098]: Invalid type, reference - ┌─ :9:55 - │ - 9 │ referenceToFooInitializedVariable : REFERENCE TO foo; - │ ^^^^^^^^^^^^^^^^ Invalid type, reference - - error[E098]: REFERENCE TO variables can not be initialized in their declaration - ┌─ :10:76 + error[E099]: Invalid type, reference + ┌─ :18:55 │ - 10 │ referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; - │ ^ REFERENCE TO variables can not be initialized in their declaration + 18 │ referenceToFooInitializedVariable : REFERENCE TO foo; + │ ^^^^^^^^^^^^^^^^ Invalid type, reference - error[E098]: Invalid type: array, pointer or bit - ┌─ :11:17 + error[E099]: Invalid type, reference + ┌─ :19:55 │ - 11 │ referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; + 19 │ referenceToFooGlobalVariable : REFERENCE TO fooGlobal; + │ ^^^^^^^^^^^^^^^^^^^^^^ Invalid type, reference + + error[E099]: Invalid type: array, pointer or bit + ┌─ :21:17 + │ + 4 │ AliasedArray : ARRAY[1..5] OF DINT; + │ ------------ see also + · + 21 │ referenceToFooAliasedArrayType : REFERENCE TO AliasedArray; │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid type: array, pointer or bit - error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` - ┌─ :17:13 + error[E099]: REFERENCE TO variables can not be initialized in their declaration + ┌─ :22:76 │ - 17 │ foo REF= foo; - │ ^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` + 22 │ referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; + │ ^ REFERENCE TO variables can not be initialized in their declaration - error[E098]: Invalid assignment, expected a variable declared with `REFERENCE TO` - ┌─ :18:13 + error[E099]: Invalid type: array, pointer or bit + ┌─ :23:17 │ - 18 │ refToFoo REF= foo; - │ ^^^^^^^^ Invalid assignment, expected a variable declared with `REFERENCE TO` + 23 │ referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -------------------------------- see also + │ │ + │ Invalid type: array, pointer or bit error[E098]: Invalid assignment, types differ (got REF_TO DINT and DINT) - ┌─ :18:13 + ┌─ :30:13 │ - 18 │ refToFoo REF= foo; + 30 │ refToFoo REF= foo; │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ (got REF_TO DINT and DINT) error[E098]: Invalid assignment, expected a reference - ┌─ :19:33 + ┌─ :31:33 │ - 19 │ referenceToFoo REF= 0; + 31 │ referenceToFoo REF= 0; │ ^ Invalid assignment, expected a reference - error[E098]: Invalid assignment, variable must not be declared with `REFERENCE TO` - ┌─ :20:33 - │ - 20 │ referenceToFoo REF= referenceToFoo; - │ ^^^^^^^^^^^^^^ Invalid assignment, variable must not be declared with `REFERENCE TO` - "###); } diff --git a/src/validation/variable.rs b/src/validation/variable.rs index ba83006fbb..7164674744 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -236,7 +236,11 @@ fn validate_reference_to_declaration( } // Assert that the referenced type is no variable reference - if context.index.find_member(context.qualifier.unwrap_or_default(), &inner_type_name).is_some() { + let qualifier = context.qualifier.unwrap_or_default(); + let var_local = context.index.find_member(qualifier, inner_type_name).is_some(); + let var_global = context.index.find_global_variable(inner_type_name).is_some(); + + if var_local || var_global { validator.push_diagnostic( Diagnostic::new("Invalid type, reference") .with_location(&variable_type.location) @@ -245,13 +249,16 @@ fn validate_reference_to_declaration( } // Lastly assert that the referenced type is no array, pointer or bit - let inner_type = context.index.find_effective_type_by_name(&inner_type_name); - if inner_type.is_some_and(|ty| ty.is_array() || ty.is_pointer() || ty.is_bit()) { - validator.push_diagnostic( - Diagnostic::new("Invalid type: array, pointer or bit ") - .with_location(&variable.location) - .with_error_code("E099"), - ); + let inner_type = context.index.find_effective_type_by_name(inner_type_name); + if let Some(ty) = inner_type { + if ty.is_array() || ty.is_pointer() || ty.is_bit() { + validator.push_diagnostic( + Diagnostic::new("Invalid type: array, pointer or bit ") + .with_location(&variable.location) + .with_secondary_location(&ty.location) + .with_error_code("E099"), + ); + } } } } diff --git a/tests/correctness/pointers.rs b/tests/correctness/pointers.rs index 53bad14337..323ec44e60 100644 --- a/tests/correctness/pointers.rs +++ b/tests/correctness/pointers.rs @@ -277,3 +277,73 @@ fn reference_to_assignment() { let res: i32 = compile_and_run(function, &mut MainType::default()); assert_eq!(5, res); } + +#[test] +#[ignore = "Currently does not work, track in an issue"] +fn reference_to_variable_referencing_other_reference_to_variable() { + let function = r" + FUNCTION main : DINT + VAR + foo : REFERENCE TO DINT; + bar : REFERENCE TO DINT; + qux : DINT; + END_VAR + + foo REF= bar; + bar REF= qux; + qux := 5; + + main := foo; // foo -> bar -> qux + END_FUNCTION + "; + + let res: i32 = compile_and_run(function, &mut MainType::default()); + assert_eq!(5, res); +} + +#[test] +fn reference_to_variable_referencing_itself() { + let function = r" + FUNCTION main : DINT + VAR + foo : REFERENCE TO DINT; + bar : REFERENCE TO DINT; + qux : DINT; + END_VAR + + foo REF= bar; + bar REF= qux; + + bar REF= bar; + qux := 5; + + main := bar; // bar (-> bar) -> qux + END_FUNCTION + "; + + let res: i32 = compile_and_run(function, &mut MainType::default()); + assert_eq!(5, res); +} + +#[test] +fn reference_to_variable_referencing_struct() { + let function = r" + TYPE Transaction : STRUCT + id : DINT; + amount : DINT; + message : STRING; + END_STRUCT END_TYPE + + FUNCTION main : DINT + VAR + txn : Transaction := (id := 1, amount := 5, message := 'whats up'); + refTxn : REFERENCE TO Transaction; + END_VAR + + main := refTxn.amount; + END_FUNCTION + "; + + let res: i32 = compile_and_run(function, &mut MainType::default()); + assert_eq!(5, res); +} From a991a8c053673db3532c7002cae258d30bb65fc8 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Mon, 1 Jul 2024 10:28:01 +0200 Subject: [PATCH 14/20] polish --- .../src/diagnostics/error_codes/E098.md | 7 ++--- .../src/diagnostics/error_codes/E099.md | 9 ++++++- src/codegen/generators/statement_generator.rs | 11 ++++---- src/codegen/tests/statement_codegen_test.rs | 24 +++++++++++------ src/typesystem.rs | 2 +- src/validation/statement.rs | 21 +-------------- .../tests/assignment_validation_tests.rs | 27 +++++++++---------- ...ts__constant_fb_instances_are_illegal.snap | 10 +++---- src/validation/variable.rs | 14 +++++----- 9 files changed, 60 insertions(+), 65 deletions(-) diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md index 8262561bdb..a170638835 100644 --- a/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md @@ -1,8 +1,7 @@ # Invalid REF= assignment -For `REF=` to be valid, the left-hand side of the assignment must be a variable declared -with the `REFERENCE TO` keyword and the right hand side must be a variable of the type -that is being referenced. +`REF=` assignments are considered valid if the left-hand side of the assignment is a variable declared +with `REFERENCE TO` and the right-hand side is a variable of the type that is being referenced. For example assignments such as the following are invalid @@ -10,10 +9,12 @@ For example assignments such as the following are invalid VAR foo : DINT; bar : DINT; + qux : SINT; refFoo : REFERENCE TO DINT; END_VAR refFoo REF= 5; // `5` is not a variable foo REF= bar; // `foo` is not declared with `REFERENCE TO` +refFoo REF= qux; // `refFoo` and `qux` have different types, DINT vs SINT refFoo REF= refFoo; // pointing to oneself doesn't make sense ``` \ No newline at end of file diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md index 693a9040ab..3096bd82f1 100644 --- a/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md @@ -1 +1,8 @@ -# Invalid `REFERENCE TO` declaration \ No newline at end of file +# Invalid `REFERENCE TO` declaration + +`REFERENCE TO` variable declarations are considered valid if the referenced type is not a +* array, e.g. `foo : REFERENCE TO ARRAY[1..5] OF DINT` +* pointer, e.g. `foo : REFERENCE TO REF_TO DINT` +* reference, e.g. `foo : REFERENCE TO bar` + +and furthermore are not initialized in their declaration, e.g. `foo : REFERENCE TO DINT := bar`. \ No newline at end of file diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index 67e9736914..c18b45cd1f 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -236,12 +236,13 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { } } - /// Generates IR for a `REF=` assignment, which is syntactic sugar for a `REF(...)` call on the - /// right side of an assignment. Specifically `foo REF= bar` and `foo := REF(bar)` are the same. + /// Generates IR for a `REF=` assignment, which is syntactic sugar for an assignment where the + /// right-hand side is wrapped in a `REF(...)` call. Specifically `foo REF= bar` and + /// `foo := REF(bar)` are the same. /// - /// Note: Though somewhat similar to the [`generate_assignment_statement`] function, we can't - /// directly call it here because the left side of a `REF=` assignment is flagged as auto-deref - /// but for `REF=` assignment we don't want (and can't) deref without generating incorrect IR.:w + /// Note: Although somewhat similar to the [`generate_assignment_statement`] function, we can't + /// apply the code here because the left side of a `REF=` assignment is flagged as auto-deref. + /// For `REF=` assignments we don't want (and can't) deref without generating incorrect IR.:w pub fn generate_ref_assignment(&self, left: &AstNode, right: &AstNode) -> Result<(), Diagnostic> { let exp = self.create_expr_generator(); let ref_builtin = self.index.get_builtin_function("REF").expect("REF must exist"); diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index 6dc77f01c0..164f94f319 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -217,33 +217,41 @@ fn ref_assignment() { #[test] fn reference_to_assignment() { - let result = codegen( + let auto_deref = codegen( r#" FUNCTION main VAR a : REFERENCE TO DINT; - b : REF_TO DINT; END_VAR a := 5; - b^ := 5; END_FUNCTION "#, ); - insta::assert_snapshot!(result, @r###" + let manual_deref = codegen( + r#" + FUNCTION main + VAR + a : REF_TO DINT; + END_VAR + a^ := 5; + END_FUNCTION + "#, + ); + + // We want to assert that `a := 5` and `a^ := 5` yield identical IR + assert_eq!(auto_deref, manual_deref); + // + insta::assert_snapshot!(auto_deref, @r###" ; ModuleID = 'main' source_filename = "main" define void @main() section "fn-$RUSTY$main:v" { entry: %a = alloca i32*, align 8 - %b = alloca i32*, align 8 store i32* null, i32** %a, align 8 - store i32* null, i32** %b, align 8 %deref = load i32*, i32** %a, align 8 store i32 5, i32* %deref, align 4 - %deref1 = load i32*, i32** %b, align 8 - store i32 5, i32* %deref1, align 4 ret void } "###); diff --git a/src/typesystem.rs b/src/typesystem.rs index d34a189c0d..3e19f5665d 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -96,7 +96,7 @@ mod tests; #[derive(Debug, Clone)] pub struct DataType { pub name: String, - /// the initial value defined on the TYPE-declration + /// the initial value defined on the TYPE-declaration pub initial_value: Option, pub information: DataTypeInformation, pub nature: TypeNature, diff --git a/src/validation/statement.rs b/src/validation/statement.rs index efd8471e13..a7fe21ce9c 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -769,7 +769,6 @@ fn validate_call_by_ref(validator: &mut Validator, param: &VariableIndexEntry, a } } -// TODO: Improve error messages? /// Checks if `REF=` assignments are correct, specifically if the left-hand side is a reference declared /// as `REFERENCE TO` and the right hand side is a lvalue of the same type that is being referenced. fn validate_ref_assignment( @@ -778,24 +777,6 @@ fn validate_ref_assignment( assignment: &Assignment, assignment_location: &SourceLocation, ) { - // // Assert that the lhs is a variable declared with `REFERENCE TO` - // if !context.annotations.get(&assignment.left).is_some_and(StatementAnnotation::is_reference_to) { - // validator.push_diagnostic( - // Diagnostic::new("Invalid assignment, expected a variable declared with `REFERENCE TO`") - // .with_location(&assignment.left.location) - // .with_error_code("E098"), - // ); - // } - - // // Assert that the rhs is NOT a variable declared with `REFERENCE TO` - // if context.annotations.get(&assignment.right).is_some_and(StatementAnnotation::is_reference_to) { - // validator.push_diagnostic( - // Diagnostic::new("Invalid assignment, variable must not be declared with `REFERENCE TO`") - // .with_location(&assignment.right.location) - // .with_error_code("E098"), - // ); - // } - // Assert that the rhs is a variable that can be referenced if !assignment.right.is_reference() { validator.push_diagnostic( @@ -812,7 +793,7 @@ fn validate_ref_assignment( if type_lhs != type_rhs { validator.push_diagnostic( Diagnostic::new(format!( - "Invalid assignment, types differ (got {} and {})", + "Invalid assignment, types {} and {} differ", get_datatype_name_or_slice(validator.context, type_lhs), get_datatype_name_or_slice(validator.context, type_rhs), )) diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 70c78b8820..6062221fae 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1218,9 +1218,8 @@ fn void_assignment_validation() { "###) } -// TODO: Think of an edge-case here, some variable that has the auto-deref trait on which #[test] -fn ref_assignment() { +fn reference_to_variables_and_ref_assignments() { let diagnostics = parse_and_validate_buffered( " TYPE @@ -1259,46 +1258,46 @@ fn ref_assignment() { ); assert_snapshot!(diagnostics, @r###" - error[E099]: Invalid type, reference + error[E099]: REFERENCE TO variables can not reference other variables ┌─ :18:55 │ 18 │ referenceToFooInitializedVariable : REFERENCE TO foo; - │ ^^^^^^^^^^^^^^^^ Invalid type, reference + │ ^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables - error[E099]: Invalid type, reference + error[E099]: REFERENCE TO variables can not reference other variables ┌─ :19:55 │ 19 │ referenceToFooGlobalVariable : REFERENCE TO fooGlobal; - │ ^^^^^^^^^^^^^^^^^^^^^^ Invalid type, reference + │ ^^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables - error[E099]: Invalid type: array, pointer or bit + error[E099]: REFERENCE TO variables can not reference arrays, pointers or bits ┌─ :21:17 │ 4 │ AliasedArray : ARRAY[1..5] OF DINT; │ ------------ see also · 21 │ referenceToFooAliasedArrayType : REFERENCE TO AliasedArray; - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid type: array, pointer or bit + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference arrays, pointers or bits - error[E099]: REFERENCE TO variables can not be initialized in their declaration + error[E099]: Initializations of REFERENCE TO variables are disallowed ┌─ :22:76 │ 22 │ referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; - │ ^ REFERENCE TO variables can not be initialized in their declaration + │ ^ Initializations of REFERENCE TO variables are disallowed - error[E099]: Invalid type: array, pointer or bit + error[E099]: REFERENCE TO variables can not reference arrays, pointers or bits ┌─ :23:17 │ 23 │ referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -------------------------------- see also │ │ - │ Invalid type: array, pointer or bit + │ REFERENCE TO variables can not reference arrays, pointers or bits - error[E098]: Invalid assignment, types differ (got REF_TO DINT and DINT) + error[E098]: Invalid assignment, types REF_TO DINT and DINT differ ┌─ :30:13 │ 30 │ refToFoo REF= foo; - │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types differ (got REF_TO DINT and DINT) + │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types REF_TO DINT and DINT differ error[E098]: Invalid assignment, expected a reference ┌─ :31:33 diff --git a/src/validation/tests/snapshots/rusty__validation__tests__variable_validation_tests__constant_fb_instances_are_illegal.snap b/src/validation/tests/snapshots/rusty__validation__tests__variable_validation_tests__constant_fb_instances_are_illegal.snap index 52d6277b77..a6062071b6 100644 --- a/src/validation/tests/snapshots/rusty__validation__tests__variable_validation_tests__constant_fb_instances_are_illegal.snap +++ b/src/validation/tests/snapshots/rusty__validation__tests__variable_validation_tests__constant_fb_instances_are_illegal.snap @@ -2,16 +2,14 @@ source: src/validation/tests/variable_validation_tests.rs expression: "&diagnostics" --- -error[E035]: Invalid constant y - Functionblock- and Class-instances cannot be delcared constant +error[E035]: Invalid constant y, FUNCTION_BLOCK- and CLASS-instances cannot be declared constant ┌─ :15:13 │ 15 │ y : MyFb; - │ ^ Invalid constant y - Functionblock- and Class-instances cannot be delcared constant + │ ^ Invalid constant y, FUNCTION_BLOCK- and CLASS-instances cannot be declared constant -error[E035]: Invalid constant z - Functionblock- and Class-instances cannot be delcared constant +error[E035]: Invalid constant z, FUNCTION_BLOCK- and CLASS-instances cannot be declared constant ┌─ :16:13 │ 16 │ z : cls; - │ ^ Invalid constant z - Functionblock- and Class-instances cannot be delcared constant - - + │ ^ Invalid constant z, FUNCTION_BLOCK- and CLASS-instances cannot be declared constant diff --git a/src/validation/variable.rs b/src/validation/variable.rs index 7164674744..fc9cfd388d 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -200,7 +200,7 @@ fn validate_variable( { validator.push_diagnostic( Diagnostic::new(format!( - "Invalid constant {} - Functionblock- and Class-instances cannot be delcared constant", + "Invalid constant {}, FUNCTION_BLOCK- and CLASS-instances cannot be declared constant", v_entry.get_name() )) .with_error_code("E035") @@ -228,7 +228,7 @@ fn validate_reference_to_declaration( if let Some(ref initializer) = variable.initializer { if variable_type.get_type_information().is_reference_to() { validator.push_diagnostic( - Diagnostic::new("REFERENCE TO variables can not be initialized in their declaration") + Diagnostic::new("Initializations of REFERENCE TO variables are disallowed") .with_location(&initializer.location) .with_error_code("E099"), ); @@ -237,12 +237,12 @@ fn validate_reference_to_declaration( // Assert that the referenced type is no variable reference let qualifier = context.qualifier.unwrap_or_default(); - let var_local = context.index.find_member(qualifier, inner_type_name).is_some(); - let var_global = context.index.find_global_variable(inner_type_name).is_some(); + let inner_ty_is_local_var = context.index.find_member(qualifier, inner_type_name).is_some(); + let inner_ty_is_global_var = context.index.find_global_variable(inner_type_name).is_some(); - if var_local || var_global { + if inner_ty_is_local_var || inner_ty_is_global_var { validator.push_diagnostic( - Diagnostic::new("Invalid type, reference") + Diagnostic::new("REFERENCE TO variables can not reference other variables") .with_location(&variable_type.location) .with_error_code("E099"), ); @@ -253,7 +253,7 @@ fn validate_reference_to_declaration( if let Some(ty) = inner_type { if ty.is_array() || ty.is_pointer() || ty.is_bit() { validator.push_diagnostic( - Diagnostic::new("Invalid type: array, pointer or bit ") + Diagnostic::new("REFERENCE TO variables can not reference arrays, pointers or bits") .with_location(&variable.location) .with_secondary_location(&ty.location) .with_error_code("E099"), From 64b759a6a8459b33c6cb6c44cd59e2b692afbc6d Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Mon, 1 Jul 2024 11:02:04 +0200 Subject: [PATCH 15/20] convert correctness tests to lit --- src/validation/statement.rs | 28 +++-- .../tests/assignment_validation_tests.rs | 15 +-- src/validation/variable.rs | 81 +++++++------- tests/correctness/pointers.rs | 105 ------------------ .../single/pointer/ref_assignment_operator.st | 11 ++ ...referenceto_variable_referencing_itself.st | 17 +++ ..._referencing_other_referenceto_variable.st | 15 +++ ...referenceto_variable_referencing_struct.st | 17 +++ 8 files changed, 127 insertions(+), 162 deletions(-) create mode 100644 tests/lit/single/pointer/ref_assignment_operator.st create mode 100644 tests/lit/single/pointer/referenceto_variable_referencing_itself.st create mode 100644 tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st create mode 100644 tests/lit/single/pointer/referenceto_variable_referencing_struct.st diff --git a/src/validation/statement.rs b/src/validation/statement.rs index a7fe21ce9c..0825a75428 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -777,20 +777,26 @@ fn validate_ref_assignment( assignment: &Assignment, assignment_location: &SourceLocation, ) { - // Assert that the rhs is a variable that can be referenced - if !assignment.right.is_reference() { - validator.push_diagnostic( - Diagnostic::new("Invalid assignment, expected a reference") - .with_location(&assignment.right.location) - .with_error_code("E098"), - ); - } + let mut assert_reference = |node: &AstNode| { + if !node.is_reference() { + validator.push_diagnostic( + Diagnostic::new("Invalid assignment, expected a reference") + .with_location(&node.location) + .with_error_code("E098"), + ); + } + }; + + assert_reference(&assignment.left); + assert_reference(&assignment.right); // Lastly, assert the type the lhs references matches with the rhs - let type_lhs = context.annotations.get_type(&assignment.left, context.index).unwrap(); - let type_rhs = context.annotations.get_type(&assignment.right, context.index).unwrap(); + let type_lhs = context.annotations.get_type_or_void(&assignment.left, context.index); + let type_rhs = context.annotations.get_type_or_void(&assignment.right, context.index); + let type_info_lhs = context.index.find_elementary_pointer_type(type_lhs.get_type_information()); + let type_info_rhs = context.index.find_elementary_pointer_type(type_rhs.get_type_information()); - if type_lhs != type_rhs { + if type_info_lhs != type_info_rhs { validator.push_diagnostic( Diagnostic::new(format!( "Invalid assignment, types {} and {} differ", diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 6062221fae..7b3af090c6 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1246,11 +1246,12 @@ fn reference_to_variables_and_ref_assignments() { referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; END_VAR - referenceToFoo REF= foo; + refToFoo REF= foo; + referenceToFoo REF= foo; // Invalid + 1 REF= foo; foo REF= foo; - refToFoo REF= foo; referenceToFoo REF= 0; referenceToFoo REF= referenceToFoo; END_FUNCTION @@ -1293,16 +1294,16 @@ fn reference_to_variables_and_ref_assignments() { │ │ │ REFERENCE TO variables can not reference arrays, pointers or bits - error[E098]: Invalid assignment, types REF_TO DINT and DINT differ + error[E098]: Invalid assignment, expected a reference ┌─ :30:13 │ - 30 │ refToFoo REF= foo; - │ ^^^^^^^^^^^^^^^^^ Invalid assignment, types REF_TO DINT and DINT differ + 30 │ 1 REF= foo; + │ ^ Invalid assignment, expected a reference error[E098]: Invalid assignment, expected a reference - ┌─ :31:33 + ┌─ :32:33 │ - 31 │ referenceToFoo REF= 0; + 32 │ referenceToFoo REF= 0; │ ^ Invalid assignment, expected a reference "###); diff --git a/src/validation/variable.rs b/src/validation/variable.rs index fc9cfd388d..558a0a266c 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -217,49 +217,52 @@ fn validate_reference_to_declaration( variable: &Variable, variable_entry: &VariableIndexEntry, ) { - if let Some(variable_type) = context.index.find_effective_type_by_name(variable_entry.get_type_name()) { - if variable_type.get_type_information().is_reference_to() { - let DataTypeInformation::Pointer { inner_type_name, .. } = variable_type.get_type_information() - else { - unreachable!("`REFERENCE TO` is defined as a pointer, hence this must exist") - }; + let Some(variable_ty) = context.index.find_effective_type_by_name(variable_entry.get_type_name()) else { + return; + }; - // Assert that no initializers are present in the `REFERENCE TO` declaration - if let Some(ref initializer) = variable.initializer { - if variable_type.get_type_information().is_reference_to() { - validator.push_diagnostic( - Diagnostic::new("Initializations of REFERENCE TO variables are disallowed") - .with_location(&initializer.location) - .with_error_code("E099"), - ); - } - } + if !variable_ty.get_type_information().is_reference_to() { + return; + } - // Assert that the referenced type is no variable reference - let qualifier = context.qualifier.unwrap_or_default(); - let inner_ty_is_local_var = context.index.find_member(qualifier, inner_type_name).is_some(); - let inner_ty_is_global_var = context.index.find_global_variable(inner_type_name).is_some(); + let Some(inner_ty_name) = variable_ty.get_type_information().get_inner_pointer_type_name() else { + unreachable!("`REFERENCE TO` is defined as a pointer, hence this must exist") + }; - if inner_ty_is_local_var || inner_ty_is_global_var { - validator.push_diagnostic( - Diagnostic::new("REFERENCE TO variables can not reference other variables") - .with_location(&variable_type.location) - .with_error_code("E099"), - ); - } + // Assert that no initializers are present in the `REFERENCE TO` declaration + if let Some(ref initializer) = variable.initializer { + if variable_ty.get_type_information().is_reference_to() { + validator.push_diagnostic( + Diagnostic::new("Initializations of REFERENCE TO variables are disallowed") + .with_location(&initializer.location) + .with_error_code("E099"), + ); + } + } - // Lastly assert that the referenced type is no array, pointer or bit - let inner_type = context.index.find_effective_type_by_name(inner_type_name); - if let Some(ty) = inner_type { - if ty.is_array() || ty.is_pointer() || ty.is_bit() { - validator.push_diagnostic( - Diagnostic::new("REFERENCE TO variables can not reference arrays, pointers or bits") - .with_location(&variable.location) - .with_secondary_location(&ty.location) - .with_error_code("E099"), - ); - } - } + // Assert that the referenced type is no variable reference + let qualifier = context.qualifier.unwrap_or_default(); + let inner_ty_is_local_var = context.index.find_member(qualifier, inner_ty_name).is_some(); + let inner_ty_is_global_var = context.index.find_global_variable(inner_ty_name).is_some(); + + if inner_ty_is_local_var || inner_ty_is_global_var { + validator.push_diagnostic( + Diagnostic::new("REFERENCE TO variables can not reference other variables") + .with_location(&variable_ty.location) + .with_error_code("E099"), + ); + } + + // Lastly assert that the referenced type is no array, pointer or bit + let inner_type = context.index.find_effective_type_by_name(inner_ty_name); + if let Some(ty) = inner_type { + if ty.is_array() || ty.is_pointer() || ty.is_bit() { + validator.push_diagnostic( + Diagnostic::new("REFERENCE TO variables can not reference arrays, pointers or bits") + .with_location(&variable.location) + .with_secondary_location(&ty.location) + .with_error_code("E099"), + ); } } } diff --git a/tests/correctness/pointers.rs b/tests/correctness/pointers.rs index 21844b175e..49fc28bc0c 100644 --- a/tests/correctness/pointers.rs +++ b/tests/correctness/pointers.rs @@ -242,108 +242,3 @@ fn value_behind_function_block_pointer_is_assigned_to_correctly() { assert!(!maintype.a); assert!(maintype.b); } - -#[test] -fn reference_assignment() { - let function = r" - FUNCTION main : DINT - VAR - a : REF_TO DINT; - b : DINT := 5; - END_VAR - - a REF= b; - main := a^; - END_FUNCTION - "; - - let res: i32 = compile_and_run(function.to_string(), &mut MainType::default()); - assert_eq!(5, res); -} - -#[test] -fn reference_to_assignment() { - let function = r" - FUNCTION main : DINT - VAR - a : REFERENCE TO DINT; - b : DINT := 5; - END_VAR - a REF= b; - main := a; - END_FUNCTION - "; - - let res: i32 = compile_and_run(function, &mut MainType::default()); - assert_eq!(5, res); -} - -#[test] -fn reference_to_variable_referencing_other_reference_to_variable() { - let function = r" - FUNCTION main : DINT - VAR - foo : REFERENCE TO DINT; - bar : REFERENCE TO DINT; - qux : DINT; - END_VAR - - bar REF= qux; - foo REF= bar; - qux := 5; - - main := foo; // foo -> bar -> qux - END_FUNCTION - "; - - let res: i32 = compile_and_run(function, &mut MainType::default()); - assert_eq!(5, res); -} - -#[test] -fn reference_to_variable_referencing_itself() { - let function = r" - FUNCTION main : DINT - VAR - foo : REFERENCE TO DINT; - bar : REFERENCE TO DINT; - qux : DINT; - END_VAR - - foo REF= bar; - bar REF= qux; - - bar REF= bar; - qux := 5; - - main := bar; // bar (-> bar) -> qux - END_FUNCTION - "; - - let res: i32 = compile_and_run(function, &mut MainType::default()); - assert_eq!(5, res); -} - -#[test] -fn reference_to_variable_referencing_struct() { - let function = r" - TYPE Transaction : STRUCT - id : DINT; - amount : DINT; - message : STRING; - END_STRUCT END_TYPE - - FUNCTION main : DINT - VAR - txn : Transaction := (id := 1, amount := 5, message := 'whats up'); - refTxn : REFERENCE TO Transaction; - END_VAR - - refTxn REF= txn; - main := refTxn.amount; - END_FUNCTION - "; - - let res: i32 = compile_and_run(function, &mut MainType::default()); - assert_eq!(5, res); -} diff --git a/tests/lit/single/pointer/ref_assignment_operator.st b/tests/lit/single/pointer/ref_assignment_operator.st new file mode 100644 index 0000000000..e51f760b39 --- /dev/null +++ b/tests/lit/single/pointer/ref_assignment_operator.st @@ -0,0 +1,11 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +// CHECK: 5 +FUNCTION main : DINT + VAR + a : REF_TO DINT; + b : DINT := 5; + END_VAR + a REF= b; + + printf('%d$N', a^); +END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_itself.st b/tests/lit/single/pointer/referenceto_variable_referencing_itself.st new file mode 100644 index 0000000000..5c3d2701a0 --- /dev/null +++ b/tests/lit/single/pointer/referenceto_variable_referencing_itself.st @@ -0,0 +1,17 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +// CHECK: 5 +FUNCTION main : DINT + VAR + foo : REFERENCE TO DINT; + bar : REFERENCE TO DINT; + qux : DINT; + END_VAR + + foo REF= bar; + bar REF= qux; + + bar REF= bar; + qux := 5; + + printf('%d$N', bar); // bar (-> bar) -> qux +END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st b/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st new file mode 100644 index 0000000000..8ce3b5a0fd --- /dev/null +++ b/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st @@ -0,0 +1,15 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +// CHECK: 5 +FUNCTION main : DINT + VAR + foo : REFERENCE TO DINT; + bar : REFERENCE TO DINT; + qux : DINT; + END_VAR + + bar REF= qux; + foo REF= bar; + qux := 5; + + printf('%d$N', foo); // foo -> bar -> qux +END_FUNCTION diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_struct.st b/tests/lit/single/pointer/referenceto_variable_referencing_struct.st new file mode 100644 index 0000000000..40876d33d4 --- /dev/null +++ b/tests/lit/single/pointer/referenceto_variable_referencing_struct.st @@ -0,0 +1,17 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +// CHECK: 5 +TYPE Transaction : STRUCT + id : DINT; + amount : DINT; + message : STRING; +END_STRUCT END_TYPE + +FUNCTION main : DINT + VAR + txn : Transaction := (id := 1, amount := 5, message := 'whats up'); + refTxn : REFERENCE TO Transaction; + END_VAR + + refTxn REF= txn; + printf('%d$N', refTxn.amount); +END_FUNCTION \ No newline at end of file From 3f74eac0d72dd29e3323dd1d91e0ca8a783b5a0d Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Wed, 3 Jul 2024 10:52:07 +0200 Subject: [PATCH 16/20] Feedback --- src/codegen/generators/statement_generator.rs | 4 +- src/codegen/tests/statement_codegen_test.rs | 54 +++- src/index.rs | 1 + src/parser/tests/statement_parser_tests.rs | 2 +- src/validation/statement.rs | 67 +++- .../tests/assignment_validation_tests.rs | 289 ++++++++++++++---- src/validation/variable.rs | 13 - .../referenceto_variable_referencing_array.st | 14 + ...referenceto_variable_referencing_struct.st | 4 + 9 files changed, 361 insertions(+), 87 deletions(-) create mode 100644 tests/lit/single/pointer/referenceto_variable_referencing_array.st diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index c18b45cd1f..7c053ddf93 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -242,13 +242,13 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { /// /// Note: Although somewhat similar to the [`generate_assignment_statement`] function, we can't /// apply the code here because the left side of a `REF=` assignment is flagged as auto-deref. - /// For `REF=` assignments we don't want (and can't) deref without generating incorrect IR.:w + /// For `REF=` assignments we don't want (and can't) deref without generating incorrect IR. pub fn generate_ref_assignment(&self, left: &AstNode, right: &AstNode) -> Result<(), Diagnostic> { let exp = self.create_expr_generator(); let ref_builtin = self.index.get_builtin_function("REF").expect("REF must exist"); let AstStatement::ReferenceExpr(data) = &left.stmt else { - unreachable!("should be covered by a validation? The left-hand side must be a reference") + unreachable!("should be covered by a validation") }; let left_ptr_val = { diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index 164f94f319..9f8a4db6d0 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -241,7 +241,7 @@ fn reference_to_assignment() { // We want to assert that `a := 5` and `a^ := 5` yield identical IR assert_eq!(auto_deref, manual_deref); - // + insta::assert_snapshot!(auto_deref, @r###" ; ModuleID = 'main' source_filename = "main" @@ -256,3 +256,55 @@ fn reference_to_assignment() { } "###); } + +#[test] +fn reference_to_string_assignment() { + let auto_deref = codegen( + r#" + FUNCTION main + VAR + a : REFERENCE TO STRING; + END_VAR + + a := 'hello'; + END_FUNCTION + "#, + ); + + let manual_deref = codegen( + r#" + FUNCTION main + VAR + a : REF_TO STRING; + END_VAR + + a^ := 'hello'; + END_FUNCTION + "#, + ); + + // We want to assert that `a := 'hello'` and `a^ := 'hello'` yield identical IR + assert_eq!(auto_deref, manual_deref); + + insta::assert_snapshot!(auto_deref, @r###" + ; ModuleID = 'main' + source_filename = "main" + + @utf08_literal_0 = private unnamed_addr constant [6 x i8] c"hello\00" + + define void @main() section "fn-$RUSTY$main:v" { + entry: + %a = alloca [81 x i8]*, align 8 + store [81 x i8]* null, [81 x i8]** %a, align 8 + %deref = load [81 x i8]*, [81 x i8]** %a, align 8 + %0 = bitcast [81 x i8]* %deref to i8* + call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 1 %0, i8* align 1 getelementptr inbounds ([6 x i8], [6 x i8]* @utf08_literal_0, i32 0, i32 0), i32 6, i1 false) + ret void + } + + ; Function Attrs: argmemonly nofree nounwind willreturn + declare void @llvm.memcpy.p0i8.p0i8.i32(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i32, i1 immarg) #0 + + attributes #0 = { argmemonly nofree nounwind willreturn } + "###); +} diff --git a/src/index.rs b/src/index.rs index cb0fdd9837..474217b662 100644 --- a/src/index.rs +++ b/src/index.rs @@ -1130,6 +1130,7 @@ impl Index { if segments.is_empty() { return None; } + //For the first element, if the context does not contain that element, it is possible that the element is also a global variable let init = match context { Some(context) => self diff --git a/src/parser/tests/statement_parser_tests.rs b/src/parser/tests/statement_parser_tests.rs index 52903ad235..c1b1c51d6e 100644 --- a/src/parser/tests/statement_parser_tests.rs +++ b/src/parser/tests/statement_parser_tests.rs @@ -1,6 +1,6 @@ use crate::{ parser::tests::{empty_stmt, ref_to}, - test_utils::tests::parse, + test_utils::tests::{parse, parse_and_validate_buffered}, typesystem::DINT_TYPE, }; use insta::assert_snapshot; diff --git a/src/validation/statement.rs b/src/validation/statement.rs index 0825a75428..2115972180 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -14,7 +14,9 @@ use plc_ast::{ use plc_diagnostics::diagnostics::Diagnostic; use plc_source::source_location::SourceLocation; +use super::{array::validate_array_assignment, ValidationContext, Validator, Validators}; use crate::index::ImplementationType; +use crate::typesystem::VOID_TYPE; use crate::validation::statement::helper::{get_datatype_name_or_slice, get_literal_int_or_const_expr_value}; use crate::{ builtins::{self, BuiltIn}, @@ -27,8 +29,6 @@ use crate::{ }, }; -use super::{array::validate_array_assignment, ValidationContext, Validator, Validators}; - macro_rules! visit_all_statements { ($validator:expr, $context:expr, $last:expr ) => { visit_statement($validator, $last, $context); @@ -777,24 +777,59 @@ fn validate_ref_assignment( assignment: &Assignment, assignment_location: &SourceLocation, ) { - let mut assert_reference = |node: &AstNode| { - if !node.is_reference() { - validator.push_diagnostic( - Diagnostic::new("Invalid assignment, expected a reference") - .with_location(&node.location) - .with_error_code("E098"), - ); - } - }; - - assert_reference(&assignment.left); - assert_reference(&assignment.right); - - // Lastly, assert the type the lhs references matches with the rhs let type_lhs = context.annotations.get_type_or_void(&assignment.left, context.index); let type_rhs = context.annotations.get_type_or_void(&assignment.right, context.index); let type_info_lhs = context.index.find_elementary_pointer_type(type_lhs.get_type_information()); let type_info_rhs = context.index.find_elementary_pointer_type(type_rhs.get_type_information()); + let annotation_lhs = context.annotations.get(&assignment.left); + + // Assert that the right-hand side is a reference + if !assignment.right.is_reference() { + validator.push_diagnostic( + Diagnostic::new("Invalid assignment, expected a reference") + .with_location(&assignment.right.location) + .with_error_code("E098"), + ); + } + + // Assert that the left-hand side is a valid pointer-reference + if !annotation_lhs.is_some_and(StatementAnnotation::is_reference_to) && !type_lhs.is_pointer() { + validator.push_diagnostic( + Diagnostic::new("Invalid assignment, expected a pointer reference") + .with_location(&assignment.left.location) + .with_error_code("E098"), + ) + } + + if type_info_lhs.is_array() && type_info_rhs.is_array() { + let mut messages = Vec::new(); + + let len_lhs = type_info_lhs.get_array_length(context.index).unwrap_or_default(); + let len_rhs = type_info_rhs.get_array_length(context.index).unwrap_or_default(); + + if len_lhs < len_rhs { + messages.push(format!("Invalid assignment, array lengths {len_lhs} and {len_rhs} differ")); + } + + let inner_ty_name_lhs = type_info_lhs.get_inner_array_type_name().unwrap_or(VOID_TYPE); + let inner_ty_name_rhs = type_info_rhs.get_inner_array_type_name().unwrap_or(VOID_TYPE); + let inner_ty_lhs = context.index.find_effective_type_by_name(inner_ty_name_lhs); + let inner_ty_rhs = context.index.find_effective_type_by_name(inner_ty_name_rhs); + + if inner_ty_lhs != inner_ty_rhs { + messages.push(format!( + "Invalid assignment, array types {inner_ty_name_lhs} and {inner_ty_name_rhs} differ" + )); + } + + for message in messages { + validator.push_diagnostic( + Diagnostic::new(message).with_location(assignment_location).with_error_code("E098"), + ) + } + + return; + } if type_info_lhs != type_info_rhs { validator.push_diagnostic( diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 7b3af090c6..9c6aabf681 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1219,92 +1219,273 @@ fn void_assignment_validation() { } #[test] -fn reference_to_variables_and_ref_assignments() { +fn ref_assignments() { let diagnostics = parse_and_validate_buffered( " - TYPE - AliasedDINT : DINT; - AliasedArray : ARRAY[1..5] OF DINT; - END_TYPE + FUNCTION main + VAR + localINT : INT; + localDINT : DINT; + localSTRING : STRING; + + localRefTo : REF_TO DINT; + localReferenceTo : REFERENCE TO DINT; + END_VAR + + localRefTo REF= localDINT; + localReferenceTo REF= localDINT; + + // The following are invalid + 1 REF= localDINT; + localINT REF= localDINT; + localRefTo REF= 1; + localReferenceTo REF= 1; + + localReferenceTo REF= localINT; + localReferenceTo REF= localSTRING; + localReferenceTo REF= 'howdy'; + END_FUNCTION + ", + ); + + assert_snapshot!(diagnostics, @r###" + error[E098]: Invalid assignment, expected a pointer reference + ┌─ :16:13 + │ + 16 │ 1 REF= localDINT; + │ ^ Invalid assignment, expected a pointer reference + + error[E098]: Invalid assignment, expected a pointer reference + ┌─ :17:13 + │ + 17 │ localINT REF= localDINT; + │ ^^^^^^^^ Invalid assignment, expected a pointer reference + + error[E098]: Invalid assignment, types INT and DINT differ + ┌─ :17:13 + │ + 17 │ localINT REF= localDINT; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types INT and DINT differ + + error[E098]: Invalid assignment, expected a reference + ┌─ :18:38 + │ + 18 │ localRefTo REF= 1; + │ ^ Invalid assignment, expected a reference + + error[E098]: Invalid assignment, expected a reference + ┌─ :19:38 + │ + 19 │ localReferenceTo REF= 1; + │ ^ Invalid assignment, expected a reference + error[E098]: Invalid assignment, types DINT and INT differ + ┌─ :21:13 + │ + 21 │ localReferenceTo REF= localINT; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types DINT and INT differ + + error[E098]: Invalid assignment, types DINT and STRING differ + ┌─ :22:13 + │ + 22 │ localReferenceTo REF= localSTRING; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types DINT and STRING differ + + error[E098]: Invalid assignment, expected a reference + ┌─ :23:38 + │ + 23 │ localReferenceTo REF= 'howdy'; + │ ^^^^^^^ Invalid assignment, expected a reference + + error[E098]: Invalid assignment, types DINT and STRING differ + ┌─ :23:13 + │ + 23 │ localReferenceTo REF= 'howdy'; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types DINT and STRING differ + + "###); +} + +#[test] +fn ref_assignment_with_global_local_variables_and_aliased_types() { + let diagnostics = parse_and_validate_buffered( + " VAR_GLOBAL fooGlobal : DINT; END_VAR FUNCTION main VAR - foo : DINT; - refToFoo : REF_TO DINT; - referenceToFoo : REFERENCE TO DINT; - - // Invalid - referenceToFooInitializedVariable : REFERENCE TO foo; - referenceToFooGlobalVariable : REFERENCE TO fooGlobal; - referenceToFooAliasedDINTType : REFERENCE TO AliasedDINT; - referenceToFooAliasedArrayType : REFERENCE TO AliasedArray; - referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; - referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; + fooLocal : DINT; + referenceToFooFirstOfHisName : REFERENCE TO DINT; + referenceToFooSecondOfHisName : REFERENCE TO DINT; + referenceToAlias : REFERENCE TO AliasedDINT; + + intLocal : INT; + stringLocal : STRING; + + // Invalid, types should be referenced rather than literals or variables + invalidA : REFERENCE TO fooLocal; + invalidB : REFERENCE TO fooGlobal; + invalidC : REFERENCE TO DINT := 5; END_VAR - refToFoo REF= foo; - referenceToFoo REF= foo; + referenceToFooFirstOfHisName REF= fooLocal; + referenceToFooFirstOfHisName REF= fooGlobal; + referenceToFooFirstOfHisName REF= referenceToFooFirstOfHisName; // Valid, albeit questionable + referenceToFooFirstOfHisName REF= referenceToFooSecondOfHisName; - // Invalid - 1 REF= foo; - foo REF= foo; - referenceToFoo REF= 0; - referenceToFoo REF= referenceToFoo; + // Invalid, type mismatch + referenceToFooFirstOfHisName REF= intLocal; + referenceToFooFirstOfHisName REF= stringLocal; END_FUNCTION ", ); assert_snapshot!(diagnostics, @r###" error[E099]: REFERENCE TO variables can not reference other variables - ┌─ :18:55 + ┌─ :17:28 │ - 18 │ referenceToFooInitializedVariable : REFERENCE TO foo; - │ ^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables + 17 │ invalidA : REFERENCE TO fooLocal; + │ ^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables error[E099]: REFERENCE TO variables can not reference other variables - ┌─ :19:55 + ┌─ :18:28 │ - 19 │ referenceToFooGlobalVariable : REFERENCE TO fooGlobal; - │ ^^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables + 18 │ invalidB : REFERENCE TO fooGlobal; + │ ^^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables - error[E099]: REFERENCE TO variables can not reference arrays, pointers or bits - ┌─ :21:17 + error[E099]: Initializations of REFERENCE TO variables are disallowed + ┌─ :19:49 │ - 4 │ AliasedArray : ARRAY[1..5] OF DINT; - │ ------------ see also - · - 21 │ referenceToFooAliasedArrayType : REFERENCE TO AliasedArray; - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference arrays, pointers or bits + 19 │ invalidC : REFERENCE TO DINT := 5; + │ ^ Initializations of REFERENCE TO variables are disallowed - error[E099]: Initializations of REFERENCE TO variables are disallowed - ┌─ :22:76 + error[E098]: Invalid assignment, types DINT and INT differ + ┌─ :28:13 │ - 22 │ referenceToFooInitializedLiteral : REFERENCE TO DINT := 5; - │ ^ Initializations of REFERENCE TO variables are disallowed + 28 │ referenceToFooFirstOfHisName REF= intLocal; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types DINT and INT differ - error[E099]: REFERENCE TO variables can not reference arrays, pointers or bits - ┌─ :23:17 + error[E098]: Invalid assignment, types DINT and STRING differ + ┌─ :29:13 │ - 23 │ referenceToFooInitializedArray : REFERENCE TO ARRAY[1..5] OF DINT; - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -------------------------------- see also - │ │ - │ REFERENCE TO variables can not reference arrays, pointers or bits + 29 │ referenceToFooFirstOfHisName REF= stringLocal; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types DINT and STRING differ - error[E098]: Invalid assignment, expected a reference - ┌─ :30:13 + "###); +} + +#[test] +fn ref_assignment_with_reference_to_array_variable() { + let diagnostics = parse_and_validate_buffered( + " + FUNCTION main + VAR + arrSTRING : ARRAY[1..6] OF STRING; + arrDINT : ARRAY[1..5] OF DINT; + arrReferenceDINT : REFERENCE TO ARRAY[1..5] OF DINT; + END_VAR + + arrReferenceDINT REF= arrDINT; + arrReferenceDINT REF= arrSTRING; + END_FUNCTION + ", + ); + + assert_snapshot!(diagnostics, @r###" + error[E098]: Invalid assignment, array lengths 5 and 6 differ + ┌─ :10:13 │ - 30 │ 1 REF= foo; - │ ^ Invalid assignment, expected a reference + 10 │ arrReferenceDINT REF= arrSTRING; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, array lengths 5 and 6 differ - error[E098]: Invalid assignment, expected a reference - ┌─ :32:33 + error[E098]: Invalid assignment, array types DINT and STRING differ + ┌─ :10:13 + │ + 10 │ arrReferenceDINT REF= arrSTRING; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, array types DINT and STRING differ + + "###); +} + +#[test] +fn ref_assignment_with_reference_to_string_variable() { + let diagnostics = parse_and_validate_buffered( + " + FUNCTION main + VAR + localCHAR : CHAR; + localSTRING : STRING; + localWSTRING : WSTRING; + referenceToString : REFERENCE TO STRING; + END_VAR + + referenceToString REF= localCHAR; + referenceToString REF= localSTRING; + referenceToString REF= localWSTRING; + END_FUNCTION + ", + ); + + assert_snapshot!(diagnostics, @r###" + error[E098]: Invalid assignment, types STRING and CHAR differ + ┌─ :10:13 + │ + 10 │ referenceToString REF= localCHAR; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types STRING and CHAR differ + + error[E098]: Invalid assignment, types STRING and WSTRING differ + ┌─ :12:13 │ - 32 │ referenceToFoo REF= 0; - │ ^ Invalid assignment, expected a reference + 12 │ referenceToString REF= localWSTRING; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment, types STRING and WSTRING differ + + "###); +} + +#[test] +fn invalid_reference_to_declaration() { + let diagnostics = parse_and_validate_buffered( + r" + FUNCTION foo + VAR + bar : ARRAY[1..5] OF REFERENCE TO DINT; + baz : REFERENCE TO REFERENCE TO DINT; + qux : REF_TO REFERENCE TO DINT; + END_VAR + END_FUNCTION + ", + ); + + insta::assert_snapshot!(diagnostics, @r###" + error[E007]: Unexpected token: expected DataTypeDefinition but found KeywordReferenceTo + ┌─ :4:38 + │ + 4 │ bar : ARRAY[1..5] OF REFERENCE TO DINT; + │ ^^^^^^^^^^^^ Unexpected token: expected DataTypeDefinition but found KeywordReferenceTo + + error[E007]: Unexpected token: expected KeywordSemicolon but found 'REFERENCE TO DINT' + ┌─ :4:38 + │ + 4 │ bar : ARRAY[1..5] OF REFERENCE TO DINT; + │ ^^^^^^^^^^^^^^^^^ Unexpected token: expected KeywordSemicolon but found 'REFERENCE TO DINT' + + error[E007]: Unexpected token: expected DataTypeDefinition but found KeywordReferenceTo + ┌─ :5:36 + │ + 5 │ baz : REFERENCE TO REFERENCE TO DINT; + │ ^^^^^^^^^^^^ Unexpected token: expected DataTypeDefinition but found KeywordReferenceTo + + error[E007]: Unexpected token: expected KeywordEndVar but found 'REFERENCE TO DINT; + qux : REF_TO REFERENCE TO DINT;' + ┌─ :5:36 + │ + 5 │ baz : REFERENCE TO REFERENCE TO DINT; + │ ╭────────────────────────────────────^ + 6 │ │ qux : REF_TO REFERENCE TO DINT; + │ ╰───────────────────────────────────────────────^ Unexpected token: expected KeywordEndVar but found 'REFERENCE TO DINT; + qux : REF_TO REFERENCE TO DINT;' "###); } diff --git a/src/validation/variable.rs b/src/validation/variable.rs index 558a0a266c..ed07481734 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -252,19 +252,6 @@ fn validate_reference_to_declaration( .with_error_code("E099"), ); } - - // Lastly assert that the referenced type is no array, pointer or bit - let inner_type = context.index.find_effective_type_by_name(inner_ty_name); - if let Some(ty) = inner_type { - if ty.is_array() || ty.is_pointer() || ty.is_bit() { - validator.push_diagnostic( - Diagnostic::new("REFERENCE TO variables can not reference arrays, pointers or bits") - .with_location(&variable.location) - .with_secondary_location(&ty.location) - .with_error_code("E099"), - ); - } - } } #[cfg(test)] diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_array.st b/tests/lit/single/pointer/referenceto_variable_referencing_array.st new file mode 100644 index 0000000000..9daffc5b2b --- /dev/null +++ b/tests/lit/single/pointer/referenceto_variable_referencing_array.st @@ -0,0 +1,14 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +// CHECK: 1, 2, 3 +FUNCTION main: DINT + VAR + arr : ARRAY[1..3] OF DINT; + refArr : REFERENCE TO ARRAY[1..3] OF DINT; + END_VAR + + refArr REF= arr; + arr[1] := 3; + arr[2] := 2; + arr[3] := 1; + printf('%d, %d, %d$N', refArr[3], refArr[2], refArr[1]); +END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_struct.st b/tests/lit/single/pointer/referenceto_variable_referencing_struct.st index 40876d33d4..1fb2002245 100644 --- a/tests/lit/single/pointer/referenceto_variable_referencing_struct.st +++ b/tests/lit/single/pointer/referenceto_variable_referencing_struct.st @@ -1,5 +1,7 @@ // RUN: (%COMPILE %s && %RUN) | %CHECK %s +// CHECK: 1 // CHECK: 5 +// CHECK: whats up TYPE Transaction : STRUCT id : DINT; amount : DINT; @@ -13,5 +15,7 @@ FUNCTION main : DINT END_VAR refTxn REF= txn; + printf('%d$N', refTxn.id); printf('%d$N', refTxn.amount); + printf('%s$N', REF(refTxn.message)); END_FUNCTION \ No newline at end of file From 892f67604a936632f2c38d028c70875b9ee6ed04 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Thu, 4 Jul 2024 12:13:33 +0200 Subject: [PATCH 17/20] Update E099.md --- .../src/diagnostics/error_codes/E099.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md index 3096bd82f1..d327378250 100644 --- a/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E099.md @@ -1,8 +1,8 @@ # Invalid `REFERENCE TO` declaration -`REFERENCE TO` variable declarations are considered valid if the referenced type is not a -* array, e.g. `foo : REFERENCE TO ARRAY[1..5] OF DINT` -* pointer, e.g. `foo : REFERENCE TO REF_TO DINT` -* reference, e.g. `foo : REFERENCE TO bar` +`REFERENCE TO` variable declarations are considered valid if the referenced type is not of the following form +* `foo : REFERENCE TO REFERENCE TO (* ... *)` +* `foo : ARRAY[...] OF REFERENCE TO (* ... *)` +* `foo : REF_TO REFERENCE TO (* ... *)` -and furthermore are not initialized in their declaration, e.g. `foo : REFERENCE TO DINT := bar`. \ No newline at end of file +Furthermore `REFERENCE_TO` variables must not be initialized in their declaration, e.g. `foo : REFERENCE TO DINT := bar`. \ No newline at end of file From 39da06447ac43541b079c5fa2e02d947a3ae8daf Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Thu, 4 Jul 2024 12:20:09 +0200 Subject: [PATCH 18/20] Add TODO for non-blocking issue --- src/validation/tests/assignment_validation_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs index 9c6aabf681..219dc6872d 100644 --- a/src/validation/tests/assignment_validation_tests.rs +++ b/src/validation/tests/assignment_validation_tests.rs @@ -1444,6 +1444,8 @@ fn ref_assignment_with_reference_to_string_variable() { "###); } +// TODO(volsa): Improve the error messages here; these are the default messages returned by the parser +// without any modifications. #[test] fn invalid_reference_to_declaration() { let diagnostics = parse_and_validate_buffered( From 180e51b6e38df07a32d572214025f73dd885159e Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Tue, 9 Jul 2024 13:41:42 +0200 Subject: [PATCH 19/20] Feedback --- .../src/diagnostics/error_codes/E098.md | 7 ++--- .../single/pointer/ref_assignment_operator.st | 2 +- .../pointer/referenceto_variable_autoderef.st | 28 +++++++++++++++++++ .../referenceto_variable_referencing_array.st | 3 +- ...referenceto_variable_referencing_itself.st | 2 +- ..._referencing_other_referenceto_variable.st | 2 +- ...referenceto_variable_referencing_struct.st | 9 ++++-- ...erenceto_variable_uninitialized_address.st | 22 +++++++++++++++ 8 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 tests/lit/single/pointer/referenceto_variable_autoderef.st create mode 100644 tests/lit/single/pointer/referenceto_variable_uninitialized_address.st diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md index a170638835..f550271947 100644 --- a/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E098.md @@ -1,7 +1,7 @@ # Invalid REF= assignment -`REF=` assignments are considered valid if the left-hand side of the assignment is a variable declared -with `REFERENCE TO` and the right-hand side is a variable of the type that is being referenced. +`REF=` assignments are considered valid if the left-hand side of the assignment is a pointer variable +and the right-hand side is a variable of the type that is being referenced. For example assignments such as the following are invalid @@ -14,7 +14,6 @@ VAR END_VAR refFoo REF= 5; // `5` is not a variable -foo REF= bar; // `foo` is not declared with `REFERENCE TO` +foo REF= bar; // `foo` is not a pointer refFoo REF= qux; // `refFoo` and `qux` have different types, DINT vs SINT -refFoo REF= refFoo; // pointing to oneself doesn't make sense ``` \ No newline at end of file diff --git a/tests/lit/single/pointer/ref_assignment_operator.st b/tests/lit/single/pointer/ref_assignment_operator.st index e51f760b39..72f5a9f97c 100644 --- a/tests/lit/single/pointer/ref_assignment_operator.st +++ b/tests/lit/single/pointer/ref_assignment_operator.st @@ -1,5 +1,4 @@ // RUN: (%COMPILE %s && %RUN) | %CHECK %s -// CHECK: 5 FUNCTION main : DINT VAR a : REF_TO DINT; @@ -7,5 +6,6 @@ FUNCTION main : DINT END_VAR a REF= b; + // CHECK: 5 printf('%d$N', a^); END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_autoderef.st b/tests/lit/single/pointer/referenceto_variable_autoderef.st new file mode 100644 index 0000000000..400b2ffc56 --- /dev/null +++ b/tests/lit/single/pointer/referenceto_variable_autoderef.st @@ -0,0 +1,28 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +FUNCTION main: DINT + VAR + foo : REFERENCE TO DINT; + bar : DINT; + baz : DINT; + qux : DINT; + END_VAR + + foo REF= bar; + bar := 1; + baz := 2; + qux := 2; + + // CHECK: 2 + bar := bar + foo; // bar + bar => 1 + 1 + printf('%d$N', foo); + + // CHECK: 4 + baz := baz + foo; // baz + foo => baz + bar => 2 + 2 + foo REF= baz; + printf('%d$N', foo); + + // CHECK: 6 + qux := qux + foo; // qux + foo => qux + baz => 2 + 4 + foo REF= qux; + printf('%d$N', foo); +END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_array.st b/tests/lit/single/pointer/referenceto_variable_referencing_array.st index 9daffc5b2b..f3e255a388 100644 --- a/tests/lit/single/pointer/referenceto_variable_referencing_array.st +++ b/tests/lit/single/pointer/referenceto_variable_referencing_array.st @@ -1,5 +1,4 @@ // RUN: (%COMPILE %s && %RUN) | %CHECK %s -// CHECK: 1, 2, 3 FUNCTION main: DINT VAR arr : ARRAY[1..3] OF DINT; @@ -10,5 +9,7 @@ FUNCTION main: DINT arr[1] := 3; arr[2] := 2; arr[3] := 1; + + // CHECK: 1, 2, 3 printf('%d, %d, %d$N', refArr[3], refArr[2], refArr[1]); END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_itself.st b/tests/lit/single/pointer/referenceto_variable_referencing_itself.st index 5c3d2701a0..c108e8917e 100644 --- a/tests/lit/single/pointer/referenceto_variable_referencing_itself.st +++ b/tests/lit/single/pointer/referenceto_variable_referencing_itself.st @@ -1,5 +1,4 @@ // RUN: (%COMPILE %s && %RUN) | %CHECK %s -// CHECK: 5 FUNCTION main : DINT VAR foo : REFERENCE TO DINT; @@ -13,5 +12,6 @@ FUNCTION main : DINT bar REF= bar; qux := 5; + // CHECK: 5 printf('%d$N', bar); // bar (-> bar) -> qux END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st b/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st index 8ce3b5a0fd..9b739965c9 100644 --- a/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st +++ b/tests/lit/single/pointer/referenceto_variable_referencing_other_referenceto_variable.st @@ -1,5 +1,4 @@ // RUN: (%COMPILE %s && %RUN) | %CHECK %s -// CHECK: 5 FUNCTION main : DINT VAR foo : REFERENCE TO DINT; @@ -11,5 +10,6 @@ FUNCTION main : DINT foo REF= bar; qux := 5; + // CHECK: 5 printf('%d$N', foo); // foo -> bar -> qux END_FUNCTION diff --git a/tests/lit/single/pointer/referenceto_variable_referencing_struct.st b/tests/lit/single/pointer/referenceto_variable_referencing_struct.st index 1fb2002245..77e6502ce3 100644 --- a/tests/lit/single/pointer/referenceto_variable_referencing_struct.st +++ b/tests/lit/single/pointer/referenceto_variable_referencing_struct.st @@ -1,7 +1,4 @@ // RUN: (%COMPILE %s && %RUN) | %CHECK %s -// CHECK: 1 -// CHECK: 5 -// CHECK: whats up TYPE Transaction : STRUCT id : DINT; amount : DINT; @@ -15,7 +12,13 @@ FUNCTION main : DINT END_VAR refTxn REF= txn; + + // CHECK: 1 printf('%d$N', refTxn.id); + + // CHECK: 5 printf('%d$N', refTxn.amount); + + // CHECK: whats up printf('%s$N', REF(refTxn.message)); END_FUNCTION \ No newline at end of file diff --git a/tests/lit/single/pointer/referenceto_variable_uninitialized_address.st b/tests/lit/single/pointer/referenceto_variable_uninitialized_address.st new file mode 100644 index 0000000000..744b96050f --- /dev/null +++ b/tests/lit/single/pointer/referenceto_variable_uninitialized_address.st @@ -0,0 +1,22 @@ +// RUN: (%COMPILE %s && %RUN) | %CHECK %s +FUNCTION main : DINT + VAR + foo : REFERENCE TO DINT; + bar : REFERENCE TO DINT; + qux : DINT; + END_VAR + + foo REF= bar; + + // CHECK-NOT: ^5$ + printf('%d$N', foo); // We expect some random value here because foo isn't pointing to any variable yet + + // CHECK-NOT: ^5$ + printf('%d$N', bar); // Same as above + + bar REF= qux; + qux := 5; + + // CHECK: 5 + printf('%d$N', bar); // We expect 5 here because bar references qux, and qux has a value of 5 +END_FUNCTION \ No newline at end of file From 1ccc09d4eb463f37c844ddd7ebcf1b0652f738a5 Mon Sep 17 00:00:00 2001 From: Volkan Date: Tue, 9 Jul 2024 15:18:45 +0200 Subject: [PATCH 20/20] Delete tests/lit/single/pointer/referenceto_variable_uninitialized_address.st --- ...erenceto_variable_uninitialized_address.st | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 tests/lit/single/pointer/referenceto_variable_uninitialized_address.st diff --git a/tests/lit/single/pointer/referenceto_variable_uninitialized_address.st b/tests/lit/single/pointer/referenceto_variable_uninitialized_address.st deleted file mode 100644 index 744b96050f..0000000000 --- a/tests/lit/single/pointer/referenceto_variable_uninitialized_address.st +++ /dev/null @@ -1,22 +0,0 @@ -// RUN: (%COMPILE %s && %RUN) | %CHECK %s -FUNCTION main : DINT - VAR - foo : REFERENCE TO DINT; - bar : REFERENCE TO DINT; - qux : DINT; - END_VAR - - foo REF= bar; - - // CHECK-NOT: ^5$ - printf('%d$N', foo); // We expect some random value here because foo isn't pointing to any variable yet - - // CHECK-NOT: ^5$ - printf('%d$N', bar); // Same as above - - bar REF= qux; - qux := 5; - - // CHECK: 5 - printf('%d$N', bar); // We expect 5 here because bar references qux, and qux has a value of 5 -END_FUNCTION \ No newline at end of file