Conversation
When arrays are assigned, if the multiplication factor is non-constant, it will be lowered into for loops. This changes the parse of (xx)(yy) to be treated as multipliers instead of call statements
# Conflicts: # compiler/plc_lowering/src/array_lowering.rs # compiler/plc_lowering/src/tests/array_lowering_tests.rs # src/codegen/generators/expression_generator.rs # src/codegen/generators/llvm.rs # src/codegen/tests/oop_tests/super_tests.rs # tests/lit/ir_tests/external_fb_array_no_init_globals.st
mhasel
left a comment
mhasel
left a comment
There was a problem hiding this comment.
It seems like validation is silently skipping bounds-checking.
FUNCTION main : DINT
VAR
n : DINT := 10;
arr : ARRAY [0..4] OF DINT := [(n)(42)];
END_VAR
END_FUNCTION
emits the following IR:
%tmpVar = add i32 0, %load_n ; 0 + 10
%tmpVar1 = sub i32 %tmpVar, 1 ; = 9
; loop: __literal_idx from 0 to 9
This is undefined behaviour and could potentially corrupt the stack.
| return make_array_element_access(lhs, offset_val as i64, array_info, id_provider); | ||
| } | ||
|
|
||
| // Non-literal offset — only supported for single-dim arrays. |
There was a problem hiding this comment.
This comment says only single-dim arrays are supported, but we don't check if we are dealing with a multi-dimensional array here.
This example silently falls through here and we fail during codegen:
FUNCTION main : DINT
VAR
n : DINT := 4;
arr : ARRAY [0..1, 0..1] OF DINT := [(n)(42)];
END_VAR
END_FUNCTION
error[E045]: Expected array access with 2 dimensions, found 1
Compilation aborted due to critical errors.
Hint: You can use `plc explain <ErrorCode>` for more information
There was a problem hiding this comment.
we can probably figure that out in constructors but not for other cases since this is a runtime value
| let stmts = find_impl_stmts(&project, "main"); | ||
| assert!(!has_literal_array(stmts), "Mixed variable multiplier should be lowered"); | ||
| assert!(has_for_loop(stmts), "Variable multiplier segment should produce a FOR loop"); | ||
| // n := 5 init + 2 individual element assignments + return assignment = 4 |
There was a problem hiding this comment.
i will replace these tests with snapshots, i think it will be easier for us to parse the tests then later
|
On hold, the feature is a nice to have but not worth the time to make everything work at the moment, we could probably pick this up again when we have more time to handle edgecases (validating bounds on constructors for example) |
2 participants
When arrays are assigned, if the multiplication factor is non-constant, it will be lowered into for loops.
This changes the parse of (xx)(yy) to be treated as multipliers instead of call statements.
This bases on PR #1633 and extends the literal assignments to also be non-constant values