feat: add additional pointer and type validations (#1341)

mhasel · web-flow · commit 867e5d9bbe53 · 2024-10-18T10:28:15.000+02:00
* fix: stack-overflow on pointer-validation

* feat: add additional pointer and type validations
diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E052.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E052.md
@@ -1 +1,3 @@
 # Unknown type
+
+Use of undeclared type-identifier.
diff --git a/src/index.rs b/src/index.rs
@@ -1628,6 +1628,9 @@ impl Index {
         if let DataTypeInformation::Pointer { inner_type_name, .. } = initial_type {
             if let Some(ty) = self.find_effective_type_info(inner_type_name) {
                 return self.find_elementary_pointer_type(self.find_intrinsic_type(ty));
+            } else {
+                // the inner type can't be found, return VOID as placeholder
+                return self.get_void_type().get_type_information();
             }
         }
 
diff --git a/src/index/const_expressions.rs b/src/index/const_expressions.rs
@@ -88,7 +88,7 @@ impl ConstExpression {
 #[derive(Debug, PartialEq, Clone)]
 pub struct InitData {
     pub initializer: Option<AstNode>,
-    pub target_type_name: String,
+    pub target_type_name: Option<String>,
     pub scope: Option<String>,
     pub lhs: Option<String>,
 }
@@ -102,7 +102,7 @@ impl InitData {
     ) -> Self {
         InitData {
             initializer: initializer.cloned(),
-            target_type_name: target_type.unwrap_or_default().to_string(),
+            target_type_name: target_type.map(|it| it.into()),
             scope: scope.map(|it| it.into()),
             lhs: target.map(|it| it.into()),
         }
diff --git a/src/lowering/initializers.rs b/src/lowering/initializers.rs
@@ -58,7 +58,7 @@ impl<'lwr> Init<'lwr> for Initializers {
             .for_each(|(scope, data)| {
                 assignments.maybe_insert_initializer(
                     &scope,
-                    Some(data.lhs.as_ref().unwrap_or(&data.target_type_name)),
+                    data.lhs.as_ref().or(data.target_type_name.as_ref()).map(|it| it.as_str()),
                     &data.initializer,
                 );
             });
diff --git a/src/parser.rs b/src/parser.rs
@@ -746,6 +746,7 @@ fn parse_pointer_definition(
         (
             DataTypeDeclaration::DataTypeDefinition {
                 data_type: DataType::PointerType { name, referenced_type: Box::new(decl), auto_deref },
+                // FIXME: this currently includes the initializer in the sourcelocation, resulting in 'REF_TO A := B' when creating a slice
                 location: lexer.source_range_factory.create_range(start_pos..lexer.last_range.end),
                 scope: lexer.scope.clone(),
             },
@@ -772,14 +773,15 @@ fn parse_type_reference_type_definition(
         None
     };
 
-    let initial_value =
+    let end = lexer.last_range.end;
+
+    let initial_value: Option<AstNode> =
         if lexer.try_consume(&KeywordAssignment) || lexer.try_consume(&KeywordReferenceAssignment) {
             Some(parse_expression(lexer))
         } else {
             None
         };
 
-    let end = lexer.last_range.end;
     if name.is_some() || bounds.is_some() {
         let data_type = match bounds {
             Some(AstNode { stmt: AstStatement::ExpressionList(expressions), id, location }) => {
diff --git a/src/resolver/tests/const_resolver_tests.rs b/src/resolver/tests/const_resolver_tests.rs
@@ -1311,7 +1311,7 @@ fn ref_initializer_is_marked_as_resolve_later() {
 
     assert_eq!(init.scope, Some("foo".into()));
     assert_eq!(init.lhs, Some("ps".into()));
-    assert_eq!(init.target_type_name, "__foo_ps".to_string());
+    assert_eq!(init.target_type_name, Some("__foo_ps".to_string()));
 }
 
 #[test]
@@ -1338,7 +1338,7 @@ fn adr_initializer_is_marked_as_resolve_later() {
 
     assert_eq!(init.scope, Some("foo".into()));
     assert_eq!(init.lhs, Some("ps".into()));
-    assert_eq!(init.target_type_name, "__foo_ps".to_string());
+    assert_eq!(init.target_type_name, Some("__foo_ps".to_string()));
 }
 
 #[test]
@@ -1368,13 +1368,13 @@ fn alias_initializer_is_marked_as_resolve_later() {
 
     assert_eq!(init.scope, Some("foo".into()));
     assert_eq!(init.lhs, Some("ps1".into()));
-    assert_eq!(init.target_type_name, "__foo_ps1".to_string());
+    assert_eq!(init.target_type_name, Some("__foo_ps1".to_string()));
 
     let Some(UnresolvableKind::Address(ref init)) = unresolvable[1].kind else { panic!() };
 
     assert_eq!(init.scope, Some("foo".into()));
     assert_eq!(init.lhs, Some("ps2".into()));
-    assert_eq!(init.target_type_name, "__foo_ps2".to_string());
+    assert_eq!(init.target_type_name, Some("__foo_ps2".to_string()));
 }
 
 #[test]
@@ -1401,5 +1401,5 @@ fn reference_to_initializer_is_marked_as_resolve_later() {
 
     assert_eq!(init.scope, Some("foo".into()));
     assert_eq!(init.lhs, Some("ps".into()));
-    assert_eq!(init.target_type_name, "__foo_ps".to_string());
+    assert_eq!(init.target_type_name, Some("__foo_ps".to_string()));
 }
diff --git a/src/validation/tests/assignment_validation_tests.rs b/src/validation/tests/assignment_validation_tests.rs
@@ -1355,6 +1355,24 @@ fn ref_assignment_with_global_local_variables_and_aliased_types() {
     18 │                 invalidB : REFERENCE TO fooGlobal;
        │                            ^^^^^^^^^^^^^^^^^^^^^^ REFERENCE TO variables can not reference other variables
 
+    error[E052]: Unknown type: AliasedDINT
+       ┌─ <internal>:11:64
+       │
+    11 │                 referenceToAlias                : REFERENCE TO AliasedDINT;
+       │                                                                ^^^^^^^^^^^ Unknown type: AliasedDINT
+
+    error[E052]: Unknown type: fooLocal
+       ┌─ <internal>:17:41
+       │
+    17 │                 invalidA : REFERENCE TO fooLocal;
+       │                                         ^^^^^^^^ Unknown type: fooLocal
+
+    error[E052]: Unknown type: fooGlobal
+       ┌─ <internal>:18:41
+       │
+    18 │                 invalidB : REFERENCE TO fooGlobal;
+       │                                         ^^^^^^^^^ Unknown type: fooGlobal
+
     error[E037]: Invalid assignment: cannot assign 'INT' to 'DINT'
        ┌─ <internal>:28:13
        │
diff --git a/src/validation/tests/reference_resolve_tests.rs b/src/validation/tests/reference_resolve_tests.rs
@@ -176,9 +176,6 @@ fn resolve_function_members_via_qualifier() {
     let diagnostics = parse_and_validate_buffered(
         "
             PROGRAM prg
-                VAR
-                    s : MyStruct;
-                END_VAR
                 foo(a := 1, b := 2, c := 3);    (* ok *)
                 foo.a; (* not ok *)
                 foo.b; (* not ok *)
diff --git a/src/validation/tests/snapshots/rusty__validation__tests__reference_resolve_tests__resolve_function_members_via_qualifier.snap b/src/validation/tests/snapshots/rusty__validation__tests__reference_resolve_tests__resolve_function_members_via_qualifier.snap
@@ -3,21 +3,19 @@ source: src/validation/tests/reference_resolve_tests.rs
 expression: "&diagnostics"
 ---
 error[E048]: Could not resolve reference to a
-  ┌─ <internal>:7:21
+  ┌─ <internal>:4:21
   │
-7 │                 foo.a; (* not ok *)
+4 │                 foo.a; (* not ok *)
   │                     ^ Could not resolve reference to a
 
 error[E048]: Could not resolve reference to b
-  ┌─ <internal>:8:21
+  ┌─ <internal>:5:21
   │
-8 │                 foo.b; (* not ok *)
+5 │                 foo.b; (* not ok *)
   │                     ^ Could not resolve reference to b
 
 error[E048]: Could not resolve reference to c
-  ┌─ <internal>:9:21
+  ┌─ <internal>:6:21
   │
-9 │                 foo.c; (* not ok *)
+6 │                 foo.c; (* not ok *)
   │                     ^ Could not resolve reference to c
-
-
diff --git a/src/validation/tests/snapshots/rusty__validation__tests__statement_validation_tests__program_implicit_downcast.snap b/src/validation/tests/snapshots/rusty__validation__tests__statement_validation_tests__program_implicit_downcast.snap
@@ -3,37 +3,37 @@ source: src/validation/tests/statement_validation_tests.rs
 expression: "&diagnostics"
 ---
 warning[E067]: Implicit downcast from 'LINT' to 'INT'.
-   ┌─ <internal>:11:17
+   ┌─ <internal>:10:17
    │
-11 │                 var1_lint, // downcast
+10 │                 var1_lint, // downcast
    │                 ^^^^^^^^^ Implicit downcast from 'LINT' to 'INT'.
 
 warning[E067]: Implicit downcast from 'LWORD' to 'DWORD'.
-   ┌─ <internal>:12:17
+   ┌─ <internal>:11:17
    │
-12 │                 var_lword, // downcast
+11 │                 var_lword, // downcast
    │                 ^^^^^^^^^ Implicit downcast from 'LWORD' to 'DWORD'.
 
 warning[E067]: Implicit downcast from 'INT' to 'SINT'.
-   ┌─ <internal>:14:17
+   ┌─ <internal>:13:17
    │
-14 │                 INT#var1_lint, // downcast
+13 │                 INT#var1_lint, // downcast
    │                 ^^^^^^^^^^^^^ Implicit downcast from 'INT' to 'SINT'.
 
 warning[E067]: Implicit downcast from 'LINT' to 'INT'.
-   ┌─ <internal>:15:17
+   ┌─ <internal>:14:17
    │
-15 │                 var2_lint, // downcast
+14 │                 var2_lint, // downcast
    │                 ^^^^^^^^^ Implicit downcast from 'LINT' to 'INT'.
 
 error[E037]: Invalid assignment: cannot assign 'WSTRING' to 'STRING'
-   ┌─ <internal>:16:17
+   ┌─ <internal>:15:17
    │
-16 │                 var_wstr, // invalid
+15 │                 var_wstr, // invalid
    │                 ^^^^^^^^ Invalid assignment: cannot assign 'WSTRING' to 'STRING'
 
 warning[E067]: Implicit downcast from 'LINT' to 'DINT'.
-   ┌─ <internal>:17:17
+   ┌─ <internal>:16:17
    │
-17 │                 var1_lint // downcast
+16 │                 var1_lint // downcast
    │                 ^^^^^^^^^ Implicit downcast from 'LINT' to 'DINT'.
diff --git a/src/validation/tests/statement_validation_tests.rs b/src/validation/tests/statement_validation_tests.rs
@@ -989,7 +989,6 @@ fn program_implicit_downcast() {
         r#"
         PROGRAM main
         VAR
-            fb: fb_t;
             var1_lint, var2_lint : LINT;
             var_real             : REAL;
             var_lword            : LWORD;
diff --git a/src/validation/tests/variable_validation_tests.rs b/src/validation/tests/variable_validation_tests.rs
@@ -719,6 +719,117 @@ fn unresolved_references_to_const_builtins_in_initializer_are_reported() {
     "###);
 }
 
+#[test]
+fn unknown_types_are_reported() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        VAR_GLOBAL
+            a: undefined;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E052]: Unknown type: undefined
+      ┌─ <internal>:3:16
+      │
+    3 │             a: undefined;
+      │                ^^^^^^^^^ Unknown type: undefined
+
+    "###);
+}
+
+#[test]
+fn aliases_are_not_reported_as_unknown_types() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        TYPE myDeclaredType : DINT; END_TYPE
+        VAR_GLOBAL
+            a: myDeclaredType;
+        END_VAR
+        "#,
+    );
+
+    assert!(diagnostics.is_empty())
+}
+
+#[test]
+fn aliasing_to_undeclared_type_is_an_error() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        TYPE myDeclaredType : undeclaredType; END_TYPE
+        VAR_GLOBAL
+            a: myDeclaredType;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E052]: Unknown type: myDeclaredType
+      ┌─ <internal>:4:16
+      │
+    4 │             a: myDeclaredType;
+      │                ^^^^^^^^^^^^^^ Unknown type: myDeclaredType
+
+    "###);
+}
+
+#[test]
+fn trying_to_initialize_a_pointer_of_unknown_type_is_reported() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        VAR_GLOBAL
+            a: undefined;
+        END_VAR
+        FUNCTION_BLOCK foo
+        VAR
+            bar : REF_TO undefined := REF(a);
+        END_VAR
+        END_FUNCTION_BLOCK
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E052]: Unknown type: undefined
+      ┌─ <internal>:7:26
+      │
+    7 │             bar : REF_TO undefined := REF(a);
+      │                          ^^^^^^^^^ Unknown type: undefined
+
+    error[E052]: Unknown type: undefined
+      ┌─ <internal>:3:16
+      │
+    3 │             a: undefined;
+      │                ^^^^^^^^^ Unknown type: undefined
+
+    "###);
+}
+
+#[test]
+fn trying_to_initialize_a_pointer_with_builtin_ref_with_type_mismatch_leads_to_error() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        VAR_GLOBAL
+            a: DINT;
+        END_VAR
+        FUNCTION_BLOCK foo
+        VAR
+            bar : REF_TO STRING := REF(a);
+        END_VAR
+        END_FUNCTION_BLOCK
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E037]: Invalid assignment: cannot assign 'DINT' to 'REF_TO STRING := REF(a)'
+      ┌─ <internal>:7:36
+      │
+    7 │             bar : REF_TO STRING := REF(a);
+      │                                    ^^^^^^ Invalid assignment: cannot assign 'DINT' to 'REF_TO STRING := REF(a)'
+
+    "###);
+}
+
 #[test]
 fn unconfigured_template_variables_are_validated() {
     let diagnostics = parse_and_validate_buffered(
diff --git a/src/validation/types.rs b/src/validation/types.rs
@@ -15,8 +15,15 @@ pub fn visit_data_type_declaration<T: AnnotationMap>(
     declaration: &DataTypeDeclaration,
     context: &ValidationContext<T>,
 ) {
-    if let DataTypeDeclaration::DataTypeDefinition { data_type, location, .. } = declaration {
-        visit_data_type(validator, data_type, location, context);
+    match declaration {
+        DataTypeDeclaration::DataTypeReference { referenced_type, location } => {
+            if context.index.find_effective_type_by_name(referenced_type).is_none() {
+                validator.push_diagnostic(Diagnostic::unknown_type(referenced_type, location.into()));
+            };
+        }
+        DataTypeDeclaration::DataTypeDefinition { data_type, location, .. } => {
+            visit_data_type(validator, data_type, location, context)
+        }
     }
 }
 
@@ -39,6 +46,9 @@ pub fn visit_data_type<T: AnnotationMap>(
         DataType::VarArgs { referenced_type: Some(referenced_type), .. } => {
             visit_data_type_declaration(validator, referenced_type.as_ref(), context);
         }
+        DataType::PointerType { referenced_type, .. } => {
+            visit_data_type_declaration(validator, referenced_type.as_ref(), context);
+        }
         _ => {}
     }
 }
diff --git a/src/validation/variable.rs b/src/validation/variable.rs

Original file line number	Diff line number	Diff line change
`@@ -1 +1,3 @@`
`1`	`1`	`# Unknown type`
	`2`	`+`
	`3`	`+Use of undeclared type-identifier.`
Original file line number	Diff line number	Diff line change
`@@ -1628,6 +1628,9 @@ impl Index {`
`1628`	`1628`	`if let DataTypeInformation::Pointer { inner_type_name, .. } = initial_type {`
`1629`	`1629`	`if let Some(ty) = self.find_effective_type_info(inner_type_name) {`
`1630`	`1630`	`return self.find_elementary_pointer_type(self.find_intrinsic_type(ty));`
	`1631`	`+ } else {`
	`1632`	`+ // the inner type can't be found, return VOID as placeholder`
	`1633`	`+ return self.get_void_type().get_type_information();`
`1631`	`1634`	`}`
`1632`	`1635`	`}`
`1633`	`1636`