From 5bfd80becde16a7769c9de2e040c1b68aaf0d933 Mon Sep 17 00:00:00 2001 From: flavioBachmann Date: Thu, 14 Jul 2022 07:08:34 +0000 Subject: [PATCH 1/3] validate Pointer incl. tests --- src/diagnostics.rs | 9 +++ src/typesystem.rs | 2 + src/validation/stmt_validator.rs | 27 ++++++- .../tests/statement_validation_tests.rs | 79 +++++++++++++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index b892d8077c..e2602c8aa2 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -79,6 +79,7 @@ pub enum ErrNo { type__invalid_nature, type__unknown_nature, type__unresolved_generic, + type__incompatible_size, //codegen related codegen__general, @@ -513,6 +514,14 @@ impl Diagnostic { } } + pub fn incompatible_type_size(nature: &str, size: u32, error: &str, location: SourceRange) -> Diagnostic { + Diagnostic::SyntaxError { + message: format!("The type {} {} is too small to {} Pointer",nature, size, error), + range: location, + err_no: ErrNo::type__incompatible_size, + } + } + pub fn link_error(error: &str) -> Diagnostic { Diagnostic::GeneralError { err_no: ErrNo::linker__generic_error, diff --git a/src/typesystem.rs b/src/typesystem.rs index 56d8f9edfe..f578fe30a0 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -27,6 +27,7 @@ pub type NativeDwordType = u32; pub type NativeLwordType = u64; pub type NativeRealType = f32; pub type NativeLrealType = f64; +pub type NativePointerType = usize; //TODO should we change this to usize? pub const U1_SIZE: u32 = 1; @@ -39,6 +40,7 @@ pub const LINT_SIZE: u32 = NativeLintType::BITS as u32; pub const REAL_SIZE: u32 = (size_of::() * 8) as u32; pub const LREAL_SIZE: u32 = (size_of::() * 8) as u32; pub const DATE_TIME_SIZE: u32 = 64; +pub const POINTER_SIZE: u32 = NativePointerType::BITS as u32; pub const U1_TYPE: &str = "__U1"; /// used internally for forced casts to u1 diff --git a/src/validation/stmt_validator.rs b/src/validation/stmt_validator.rs index 6ffe5b3e31..3ad825e3e8 100644 --- a/src/validation/stmt_validator.rs +++ b/src/validation/stmt_validator.rs @@ -8,7 +8,7 @@ use crate::{ typesystem::{ DataType, DataTypeInformation, Dimension, BOOL_TYPE, DATE_AND_TIME_TYPE, DATE_TYPE, DINT_TYPE, INT_TYPE, LINT_TYPE, LREAL_TYPE, SINT_TYPE, STRING_TYPE, TIME_OF_DAY_TYPE, - TIME_TYPE, UDINT_TYPE, UINT_TYPE, ULINT_TYPE, USINT_TYPE, VOID_TYPE, WSTRING_TYPE, + TIME_TYPE, UDINT_TYPE, UINT_TYPE, ULINT_TYPE, USINT_TYPE, VOID_TYPE, WSTRING_TYPE, POINTER_SIZE, }, Diagnostic, }; @@ -137,6 +137,31 @@ impl StatementValidator { .get_type_or_void(right, context.index) .get_type_information(); + //check if Datatype can hold a Pointer (u64) + if r_effective_type.is_pointer() && + !l_effective_type.is_pointer() && + l_effective_type.get_size() < POINTER_SIZE + { + self.diagnostics.push(Diagnostic::incompatible_type_size( + l_effective_type.get_name(), + l_effective_type.get_size(), + "hold a", + statement.get_location(), + )); + } + //check if size allocated to Pointer is standart pointer size (u64) + else if l_effective_type.is_pointer() && + !r_effective_type.is_pointer() && + r_effective_type.get_size() < POINTER_SIZE + { + self.diagnostics.push(Diagnostic::incompatible_type_size( + r_effective_type.get_name(), + r_effective_type.get_size(), + "to be stored in a", + statement.get_location(), + )); + } + // valid assignments -> char := literalString, char := char // check if we assign to a character variable -> char := .. if l_effective_type.is_character() { diff --git a/src/validation/tests/statement_validation_tests.rs b/src/validation/tests/statement_validation_tests.rs index 3f834d255c..0902ec05f4 100644 --- a/src/validation/tests/statement_validation_tests.rs +++ b/src/validation/tests/statement_validation_tests.rs @@ -1,6 +1,85 @@ use crate::test_utils::tests::parse_and_validate; use crate::Diagnostic; +#[test] +fn assign_pointer_to_too_small_type_result_in_an_error() { + //GIVEN assignment statements to DWORD + //WHEN it is validated + let diagnostics: Vec = parse_and_validate( + " + PROGRAM FOO + VAR + ptr : POINTER TO INT; + address : DWORD; + END_VAR + + address := 16#DEAD_BEEF; + address := ptr; //should throw error as address is too small to store full pointer + END_PROGRAM + ", + ); + + //THEN assignment with different type sizes are reported + assert_eq!( + diagnostics, + vec![ + Diagnostic::incompatible_type_size("DWORD",32,"hold a",(204..218).into()), + ] + ); +} + +#[test] +fn assign_too_small_type_to_pointer_result_in_an_error() { + //GIVEN assignment statements to pointer + //WHEN it is validated + let diagnostics: Vec = parse_and_validate( + " + PROGRAM FOO + VAR + ptr : POINTER TO INT; + address : DWORD; + END_VAR + + address := 16#DEAD_BEEF; + ptr := address; //should throw error as address is too small to store full pointer + END_PROGRAM + ", + ); + + //THEN assignment with different type sizes are reported + assert_eq!( + diagnostics, + vec![ + Diagnostic::incompatible_type_size("DWORD",32,"to be stored in a",(204..218).into()), + ] + ); +} + +#[test] +fn assign_pointer_to_lword() { + //GIVEN assignment statements to lword + //WHEN it is validated + let diagnostics: Vec = parse_and_validate( + " + PROGRAM FOO + VAR + ptr : POINTER TO INT; + address : LWORD; + END_VAR + + address := 16#DEAD_BEEF; + address := ptr; //should throw error as address is too small to store full pointer + END_PROGRAM + ", + ); + + //THEN every assignment is valid + assert_eq!( + diagnostics, + vec![] + ); +} + #[test] fn assignment_to_constants_result_in_an_error() { // GIVEN assignment statements to constants, some to writable variables From e9f6a1642a35dd0c5e33a3b55a684b7c0d95f13c Mon Sep 17 00:00:00 2001 From: flavioBachmann Date: Thu, 14 Jul 2022 07:55:27 +0000 Subject: [PATCH 2/3] using cargo clippy for coding style as well as cargo fmt --- src/diagnostics.rs | 16 +++++++++---- src/validation/stmt_validator.rs | 21 +++++++++-------- .../tests/statement_validation_tests.rs | 23 +++++++++++-------- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index e2602c8aa2..b26e1a4bac 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -514,10 +514,18 @@ impl Diagnostic { } } - pub fn incompatible_type_size(nature: &str, size: u32, error: &str, location: SourceRange) -> Diagnostic { - Diagnostic::SyntaxError { - message: format!("The type {} {} is too small to {} Pointer",nature, size, error), - range: location, + pub fn incompatible_type_size( + nature: &str, + size: u32, + error: &str, + location: SourceRange, + ) -> Diagnostic { + Diagnostic::SyntaxError { + message: format!( + "The type {} {} is too small to {} Pointer", + nature, size, error + ), + range: location, err_no: ErrNo::type__incompatible_size, } } diff --git a/src/validation/stmt_validator.rs b/src/validation/stmt_validator.rs index 3ad825e3e8..5e3098518c 100644 --- a/src/validation/stmt_validator.rs +++ b/src/validation/stmt_validator.rs @@ -7,8 +7,9 @@ use crate::{ resolver::{AnnotationMap, StatementAnnotation}, typesystem::{ DataType, DataTypeInformation, Dimension, BOOL_TYPE, DATE_AND_TIME_TYPE, DATE_TYPE, - DINT_TYPE, INT_TYPE, LINT_TYPE, LREAL_TYPE, SINT_TYPE, STRING_TYPE, TIME_OF_DAY_TYPE, - TIME_TYPE, UDINT_TYPE, UINT_TYPE, ULINT_TYPE, USINT_TYPE, VOID_TYPE, WSTRING_TYPE, POINTER_SIZE, + DINT_TYPE, INT_TYPE, LINT_TYPE, LREAL_TYPE, POINTER_SIZE, SINT_TYPE, STRING_TYPE, + TIME_OF_DAY_TYPE, TIME_TYPE, UDINT_TYPE, UINT_TYPE, ULINT_TYPE, USINT_TYPE, VOID_TYPE, + WSTRING_TYPE, }, Diagnostic, }; @@ -138,21 +139,21 @@ impl StatementValidator { .get_type_information(); //check if Datatype can hold a Pointer (u64) - if r_effective_type.is_pointer() && - !l_effective_type.is_pointer() && - l_effective_type.get_size() < POINTER_SIZE - { + if r_effective_type.is_pointer() + && !l_effective_type.is_pointer() + && l_effective_type.get_size() < POINTER_SIZE + { self.diagnostics.push(Diagnostic::incompatible_type_size( l_effective_type.get_name(), l_effective_type.get_size(), "hold a", statement.get_location(), )); - } + } //check if size allocated to Pointer is standart pointer size (u64) - else if l_effective_type.is_pointer() && - !r_effective_type.is_pointer() && - r_effective_type.get_size() < POINTER_SIZE + else if l_effective_type.is_pointer() + && !r_effective_type.is_pointer() + && r_effective_type.get_size() < POINTER_SIZE { self.diagnostics.push(Diagnostic::incompatible_type_size( r_effective_type.get_name(), diff --git a/src/validation/tests/statement_validation_tests.rs b/src/validation/tests/statement_validation_tests.rs index 0902ec05f4..74cc01e140 100644 --- a/src/validation/tests/statement_validation_tests.rs +++ b/src/validation/tests/statement_validation_tests.rs @@ -22,9 +22,12 @@ fn assign_pointer_to_too_small_type_result_in_an_error() { //THEN assignment with different type sizes are reported assert_eq!( diagnostics, - vec![ - Diagnostic::incompatible_type_size("DWORD",32,"hold a",(204..218).into()), - ] + vec![Diagnostic::incompatible_type_size( + "DWORD", + 32, + "hold a", + (204..218).into() + ),] ); } @@ -49,9 +52,12 @@ fn assign_too_small_type_to_pointer_result_in_an_error() { //THEN assignment with different type sizes are reported assert_eq!( diagnostics, - vec![ - Diagnostic::incompatible_type_size("DWORD",32,"to be stored in a",(204..218).into()), - ] + vec![Diagnostic::incompatible_type_size( + "DWORD", + 32, + "to be stored in a", + (204..218).into() + ),] ); } @@ -74,10 +80,7 @@ fn assign_pointer_to_lword() { ); //THEN every assignment is valid - assert_eq!( - diagnostics, - vec![] - ); + assert_eq!(diagnostics, vec![]); } #[test] From 66bc397263fdf8862a2f898fe1b7a7d243382484 Mon Sep 17 00:00:00 2001 From: Flavio Schuricht Date: Thu, 14 Jul 2022 13:21:32 +0000 Subject: [PATCH 3/3] multi-type declarations incl. tests --- src/index/tests/instance_resolver_tests.rs | 1 + ...global_struct_variables_are_retrieved.snap | 22 +++---- src/parser.rs | 45 +++++++------ .../parse_error_containers_tests.rs | 7 +- ..._parser_tests__multi_type_declaration.snap | 64 +++++++++++++++++++ src/parser/tests/type_parser_tests.rs | 17 +++++ 6 files changed, 123 insertions(+), 33 deletions(-) create mode 100644 src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__multi_type_declaration.snap diff --git a/src/index/tests/instance_resolver_tests.rs b/src/index/tests/instance_resolver_tests.rs index b2dd94b3c8..24cef3d0f2 100644 --- a/src/index/tests/instance_resolver_tests.rs +++ b/src/index/tests/instance_resolver_tests.rs @@ -87,6 +87,7 @@ fn nested_global_struct_variables_are_retrieved() { TYPE str : STRUCT a,b : str2; END_STRUCT + END_TYPE TYPE str2 : STRUCT c,d : DINT; END_STRUCT diff --git a/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__nested_global_struct_variables_are_retrieved.snap b/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__nested_global_struct_variables_are_retrieved.snap index e9ad383fb8..2dc2854a35 100644 --- a/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__nested_global_struct_variables_are_retrieved.snap +++ b/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__nested_global_struct_variables_are_retrieved.snap @@ -24,7 +24,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 154..158, + range: 167..171, }, }, ), @@ -83,7 +83,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 91..92, + range: 104..105, }, }, ), @@ -114,7 +114,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 93..94, + range: 106..107, }, }, ), @@ -173,7 +173,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 91..92, + range: 104..105, }, }, ), @@ -204,7 +204,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 93..94, + range: 106..107, }, }, ), @@ -229,7 +229,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 182..254, + range: 195..267, }, }, ), @@ -257,7 +257,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 215..219, + range: 228..232, }, }, ), @@ -322,7 +322,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 91..92, + range: 104..105, }, }, ), @@ -356,7 +356,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 93..94, + range: 106..107, }, }, ), @@ -421,7 +421,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 91..92, + range: 104..105, }, }, ), @@ -455,7 +455,7 @@ expression: "index.find_instances().collect::>>()" linkage: Internal, binding: None, source_location: SourceRange { - range: 93..94, + range: 106..107, }, }, ), diff --git a/src/parser.rs b/src/parser.rs index 5b8df237af..7de094c85a 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -63,8 +63,9 @@ pub fn parse(mut lexer: ParseSession, lnk: LinkageType) -> ParsedAst { unit.implementations.append(&mut actions); } KeywordType => { - if let Some(unit_type) = parse_type(&mut lexer) { - unit.types.push(unit_type); + let unit_type = parse_type(&mut lexer); + for utype in unit_type { + unit.types.push(utype); } } KeywordEndActions | End => return (unit, lexer.diagnostics), @@ -535,26 +536,32 @@ fn parse_action( } // TYPE ... END_TYPE -fn parse_type(lexer: &mut ParseSession) -> Option { +fn parse_type(lexer: &mut ParseSession) -> Vec { lexer.advance(); // consume the TYPE - let start = lexer.location().get_start(); - let name = lexer.slice_and_advance(); - lexer.consume_or_report(KeywordColon); - let result = parse_full_data_type_definition(lexer, Some(name)); + parse_any_in_region(lexer, vec![KeywordEndType], |lexer| { + let mut declarations = vec![]; + while !lexer.closes_open_region(&lexer.token) { + let start = lexer.location().get_start(); + let name = lexer.slice_and_advance(); + lexer.consume_or_report(KeywordColon); - if let Some((DataTypeDeclaration::DataTypeDefinition { data_type, .. }, initializer)) = result { - let end = lexer.last_range.end; - lexer.consume_or_report(KeywordEndType); - Some(UserTypeDeclaration { - data_type, - initializer, - location: (start..end).into(), - scope: lexer.scope.clone(), - }) - } else { - None - } + let result = parse_full_data_type_definition(lexer, Some(name)); + + if let Some((DataTypeDeclaration::DataTypeDefinition { data_type, .. }, initializer)) = + result + { + let end = lexer.last_range.end; + declarations.push(UserTypeDeclaration { + data_type, + initializer, + location: (start..end).into(), + scope: lexer.scope.clone(), + }); + } + } + declarations + }) } type DataTypeWithInitializer = (DataTypeDeclaration, Option); diff --git a/src/parser/tests/parse_errors/parse_error_containers_tests.rs b/src/parser/tests/parse_errors/parse_error_containers_tests.rs index a2d8ea5d7a..7bafba463b 100644 --- a/src/parser/tests/parse_errors/parse_error_containers_tests.rs +++ b/src/parser/tests/parse_errors/parse_error_containers_tests.rs @@ -290,10 +290,11 @@ fn test_unexpected_type_declaration_error_message() { ), Diagnostic::unexpected_token_found( "KeywordSemicolon", - "'PROGRAM\n END_PROGRAM\n END_TYPE'", - (29..85).into(), + "'PROGRAM\n END_PROGRAM'", + (29..64).into(), ), - Diagnostic::unexpected_token_found("KeywordSemicolon", "''", (90..90).into(),), + Diagnostic::missing_token("[KeywordSemicolon]", (77..85).into(),), + Diagnostic::unexpected_token_found("KeywordSemicolon", "'END_TYPE'", (77..85).into(),), ], diagnostics ); diff --git a/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__multi_type_declaration.snap b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__multi_type_declaration.snap new file mode 100644 index 0000000000..9921451e74 --- /dev/null +++ b/src/parser/tests/snapshots/rusty__parser__tests__type_parser_tests__multi_type_declaration.snap @@ -0,0 +1,64 @@ +--- +source: src/parser/tests/type_parser_tests.rs +assertion_line: 18 +expression: result +--- +CompilationUnit { + global_vars: [], + units: [], + implementations: [], + types: [ + UserTypeDeclaration { + data_type: StructType { + name: Some( + "Point2D", + ), + variables: [ + Variable { + name: "x", + data_type: DataTypeReference { + referenced_type: "INT", + }, + }, + Variable { + name: "y", + data_type: DataTypeReference { + referenced_type: "INT", + }, + }, + ], + }, + initializer: None, + scope: None, + }, + UserTypeDeclaration { + data_type: StructType { + name: Some( + "Point3D", + ), + variables: [ + Variable { + name: "x", + data_type: DataTypeReference { + referenced_type: "INT", + }, + }, + Variable { + name: "y", + data_type: DataTypeReference { + referenced_type: "INT", + }, + }, + Variable { + name: "z", + data_type: DataTypeReference { + referenced_type: "INT", + }, + }, + ], + }, + initializer: None, + scope: None, + }, + ], +} diff --git a/src/parser/tests/type_parser_tests.rs b/src/parser/tests/type_parser_tests.rs index 2456ddd380..1dfd165534 100644 --- a/src/parser/tests/type_parser_tests.rs +++ b/src/parser/tests/type_parser_tests.rs @@ -1,6 +1,23 @@ use crate::{ast::*, parser::AstStatement::LiteralInteger, test_utils::tests::parse, Diagnostic}; use pretty_assertions::*; +#[test] +fn multi_type_declaration() { + let (result, ..) = parse( + r#" + TYPE + Point2D : STRUCT + x,y : INT; + END_STRUCT + Point3D : STRUCT + x,y,z : INT; + END_STRUCT + END_TYPE + "#, + ); + insta::assert_debug_snapshot!(result); +} + #[test] fn simple_struct_type_can_be_parsed() { let (result, ..) = parse(