fix(validation): prevent array-bound panics and diagnose non-constant bounds

compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs

@@ -231,7 +231,7 @@ lazy_static! {
         E128,   Error,      include_str!("./error_codes/E128.md"),  // Invalid nested property assignment target
         E129,   Error,      include_str!("./error_codes/E129.md"),  // Direct interface reference call
         E130,   Error,      include_str!("./error_codes/E130.md"),  // Array size exceeds supported limit
-        // XXX: Use E117 when introducing a new error, it has been removed from the registry (also delete me afterwards)
+        E117,   Error,      include_str!("./error_codes/E117.md"),  // Non-constant array boundary
     );
 }
 

compiler/plc_diagnostics/src/diagnostics/error_codes/E117.md

@@ -0,0 +1,7 @@
+# Non-constant array boundary
+
+Array boundaries must be compile-time constants.
+
+Examples of invalid boundaries:
+- Local variables declared without `CONSTANT`
+- Expressions that are not compile-time evaluable constants

src/typesystem.rs

@@ -752,8 +752,12 @@ impl DataTypeInformation {
             DataTypeInformation::Array { inner_type_name, dimensions, .. } => {
                 let inner_type = index.get_type_information_or_void(inner_type_name);
                 let inner_size = inner_type.get_size_recursive(index, seen)?.bits();
-                let element_count: u32 =
-                    dimensions.iter().map(|dim| dim.get_length(index).unwrap()).product();
+                let element_count: u32 = dimensions
+                    .iter()
+                    .map(|dim| dim.get_length(index).map_err(|err| anyhow::anyhow!(err)))
+                    .collect::<Result<Vec<_>>>()?
+                    .into_iter()
+                    .product();
                 Ok(Bytes::from_bits(inner_size * element_count))
             }
             DataTypeInformation::Pointer { .. } => Ok(Bytes::from_bits(POINTER_SIZE)),

src/validation/tests/array_validation_test.rs

@@ -206,6 +206,28 @@ fn array_bounds_with_const_integer_variables_are_valid() {
     assert_snapshot!(diagnostics, @"");
 }
 
+#[test]
+fn array_bounds_must_be_constants() {
+    let diagnostics = parse_and_validate_buffered(
+        "
+        FUNCTION main : DINT
+        VAR
+            x : INT := 5;
+            arr : ARRAY[1..x] OF INT;
+        END_VAR
+        END_FUNCTION
+        ",
+    );
+
+    assert_snapshot!(diagnostics, @r"
+    error[E117]: Only constants are allowed as array boundaries
+      ┌─ <internal>:5:28
+      │
+    5 │             arr : ARRAY[1..x] OF INT;
+      │                            ^ Only constants are allowed as array boundaries
+    ");
+}
+
 #[test]
 fn array_bounds_with_parenthesized_integer_literals_are_valid() {
     let diagnostics = parse_and_validate_buffered(

src/validation/types.rs

@@ -69,8 +69,8 @@ pub fn visit_data_type<T: AnnotationMap>(
                         {
                             validator.push_diagnostic(
                                 Diagnostic::new("Invalid reference to declaration. Arrays of automatically dereferenced references are not allowed.")
-                                .with_error_code("E099")
-                                .with_location(location),
+                                    .with_error_code("E099")
+                                    .with_location(location),
                             );
                         }
                     };

src/validation/variable.rs

@@ -300,6 +300,54 @@ where
     }
 }
 
+fn validate_array_bounds_are_constant<T: AnnotationMap>(
+    validator: &mut Validator,
+    variable: &Variable,
+    context: &ValidationContext<T>,
+) {
+    let ty_name = variable.data_type_declaration.get_name().unwrap_or_default();
+    let ty_info = context.index.get_effective_type_or_void_by_name(ty_name).get_type_information();
+
+    if !ty_info.is_array() {
+        return;
+    }
+
+    let mut types = vec![];
+    ty_info.get_inner_array_types(&mut types, context.index);
+
+    for ty in types {
+        let DataTypeInformation::Array { dimensions, .. } = ty else {
+            unreachable!("`get_inner_types()` only operates on Arrays");
+        };
+
+        for expr in dimensions.iter().flat_map(|it| {
+            [
+                it.start_offset.as_const_expression(context.index),
+                it.end_offset.as_const_expression(context.index),
+            ]
+        }) {
+            let Some(expr) = expr else { continue };
+            let Some(reference_name) = expr.get_flat_reference_name() else { continue };
+
+            let qualifier = context.qualifier.unwrap_or_default();
+            let is_non_constant_variable = context
+                .index
+                .find_member(qualifier, reference_name)
+                .map(|it| !it.is_constant())
+                .or_else(|| context.index.find_global_variable(reference_name).map(|it| !it.is_constant()))
+                .unwrap_or(false);
+
+            if is_non_constant_variable {
+                validator.push_diagnostic(
+                    Diagnostic::new("Only constants are allowed as array boundaries")
+                        .with_error_code("E117")
+                        .with_location(expr.get_location()),
+                );
+            }
+        }
+    }
+}
+
 fn unresolved_constant_diagnostic_message(variable: &Variable, type_name: &str) -> String {
     if variable.initializer.as_ref().is_some_and(|it| matches!(it.get_stmt(), AstStatement::DefaultValue(_)))
     {
@@ -319,6 +367,7 @@ fn validate_variable<T: AnnotationMap>(
 ) {
     validate_variable_redeclaration(validator, variable, context);
 
+    validate_array_bounds_are_constant(validator, variable, context);
     validate_array_ranges(validator, variable, context);
     validate_array_size(validator, variable, context);