diff --git a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs index 1bc9e5b03db..49208375474 100644 --- a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs +++ b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs @@ -242,6 +242,7 @@ lazy_static! { E138, Error, include_str!("./error_codes/E138.md"), // Reserved keyword used as a name E139, Error, include_str!("./error_codes/E139.md"), // Linker invocation failed (spawn / cmdline) E140, Error, include_str!("./error_codes/E140.md"), // ':=' used for an output parameter + E141, Error, include_str!("./error_codes/E141.md"), // Member access on a non-auto-deref pointer base ); } diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E141.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E141.md new file mode 100644 index 00000000000..6f1edabfcc9 --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E141.md @@ -0,0 +1,44 @@ +# Member access on a non-auto-deref pointer base + +Member access (`base.member`) requires `base` to be a struct, function-block, or class instance — not a pointer to one. The compiler will not implicitly dereference pointers when they appear on the left of a `.` (auto-dereferencing applies only to `REFERENCE TO` / alias pointers). + +This rule covers three cases that share the same underlying shape — the value of `base` is a pointer, not the pointee: + +1. `THIS` is `POINTER TO `. Use `THIS^.member`. +2. `SUPER` is `POINTER TO ` (before dereferencing). Use `SUPER^.member`. +3. A user-declared `POINTER TO ...` variable. Use `^.member`. + +## Example of invalid use + +```iecst +FUNCTION_BLOCK fb + VAR + a : DINT; + END_VAR +END_FUNCTION_BLOCK + +FUNCTION_BLOCK other + VAR + p_fb : POINTER TO fb; + END_VAR + METHOD m : DINT + p_fb.a := 1; // Error: Cannot access `a` on `POINTER TO fb` + THIS.b := 2; // Error: Cannot access `b` on `POINTER TO other` (THIS is a pointer) + END_METHOD +END_FUNCTION_BLOCK +``` + +## Example of valid use + +```iecst +FUNCTION_BLOCK other + VAR + p_fb : POINTER TO fb; + b : DINT; + END_VAR + METHOD m : DINT + p_fb^.a := 1; // Dereference the pointer first + THIS^.b := 2; // Dereference THIS first + END_METHOD +END_FUNCTION_BLOCK +``` diff --git a/src/validation.rs b/src/validation.rs index 6429701757a..4bef9ded5f1 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -110,7 +110,15 @@ pub struct Validator<'a> { impl Validators for Validator<'_> { fn push_diagnostic(&mut self, diagnostic: Diagnostic) { - self.diagnostics.push(diagnostic); + // Skip exact duplicates: lowering passes occasionally clone an AST + // node into multiple positions in the post-lowered tree (e.g. the + // receiver of a method call ends up both in the call's operator + // chain and in its synthesised `self` argument), which causes the + // same statement-level diagnostic to be raised more than once for + // the same source location. + if !self.diagnostics.contains(&diagnostic) { + self.diagnostics.push(diagnostic); + } } fn take_diagnostics(&mut self) -> Vec { std::mem::take(&mut self.diagnostics) diff --git a/src/validation/statement.rs b/src/validation/statement.rs index 2718f70c588..ac70e4670e5 100644 --- a/src/validation/statement.rs +++ b/src/validation/statement.rs @@ -218,14 +218,51 @@ fn validate_reference_expression( .with_location(m.get_location()) .with_error_code("E119"), ); - } else if (base.is_super() || base.has_super_metadata()) - && !(base.is_super_deref() || base.has_super_metadata_deref()) - { - validator.push_diagnostic( - Diagnostic::new("`SUPER` must be dereferenced to access its members.") + // Only suppress follow-on cascades when the enclosing POU + // actually extends something. Otherwise let + // `validate_member_access` recurse so the SUPER-leaf + // check still emits "Invalid use of `SUPER`. Usage is + // only allowed within a POU that directly extends + // another POU" -- that's orthogonal signal worth + // keeping. + let qualifier_extends = context + .qualifier + .and_then(|q| context.index.find_pou(q)) + .and_then(|pou| match pou { + PouIndexEntry::Method { parent_name, .. } + | PouIndexEntry::Action { parent_name, .. } => { + context.index.find_pou(parent_name) + } + other => Some(other), + }) + .and_then(|pou| pou.get_super_class()) + .is_some(); + if qualifier_extends { + return; + } + } else if let Some(base_type) = context.annotations.get_type(base, context.index) { + // Member access through a non-auto-deref pointer needs an + // explicit `^`. This subsumes THIS, SUPER, and + // user-declared `POINTER TO ...` bases, which all share + // the same underlying shape (the value of the base is a + // pointer, not the pointee). + let info = base_type.get_type_information(); + if info.is_pointer() && !info.is_auto_deref() { + let inner = info.get_inner_name(); + // Use the source slice for the member name so the + // diagnostic shows what the user wrote (e.g. `prop`) + // rather than any synthetic name introduced by an + // earlier lowering pass (e.g. `__get_prop`). + let m_name = validator.context.slice(&m.get_location()); + validator.push_diagnostic( + Diagnostic::new(format!( + "Cannot access `{m_name}` on `POINTER TO {inner}`; dereference with `^` first" + )) .with_location(m.get_location()) - .with_error_code("E119"), - ); + .with_error_code("E141"), + ); + return; + } } } diff --git a/src/validation/tests/super_keyword_validation_tests.rs b/src/validation/tests/super_keyword_validation_tests.rs index 5518fe301d8..bbd53bf8d17 100644 --- a/src/validation/tests/super_keyword_validation_tests.rs +++ b/src/validation/tests/super_keyword_validation_tests.rs @@ -314,12 +314,6 @@ fn super_accessor_cannot_be_accessed_from_outside_of_its_pou() { 16 │ fb.SUPER.x := 2; │ ^^^^^ Invalid use of `SUPER`. Usage is only allowed within a POU that directly extends another POU. - error[E119]: `SUPER` must be dereferenced to access its members. - ┌─ :16:22 - │ - 16 │ fb.SUPER.x := 2; - │ ^ `SUPER` must be dereferenced to access its members. - error[E119]: `SUPER` is not allowed in member-access position. ┌─ :17:16 │ @@ -995,12 +989,6 @@ fn invalid_super_dereferencing_patterns() { 16 │ SUPER^.SUPER.x := 40; │ ^^^^^ `SUPER` is not allowed in member-access position. - error[E119]: `SUPER` must be dereferenced to access its members. - ┌─ :16:26 - │ - 16 │ SUPER^.SUPER.x := 40; - │ ^ `SUPER` must be dereferenced to access its members. - warning[E049]: Illegal access to private member parent.x ┌─ :16:26 │ @@ -1288,12 +1276,12 @@ fn super_dereferencing_with_method_calls() { "#, ); - assert_snapshot!(diagnostics, @r" - error[E119]: `SUPER` must be dereferenced to access its members. + assert_snapshot!(diagnostics, @" + error[E141]: Cannot access `get_value` on `POINTER TO parent`; dereference with `^` first ┌─ :19:28 │ 19 │ x := SUPER.get_value(); // Trying to call method on pointer - │ ^^^^^^^^^ `SUPER` must be dereferenced to access its members. + │ ^^^^^^^^^ Cannot access `get_value` on `POINTER TO parent`; dereference with `^` first error[E037]: Invalid assignment: cannot assign 'get_value' to 'parent' ┌─ :20:17 @@ -1327,18 +1315,18 @@ fn super_without_deref_accessing_members() { "#, ); - assert_snapshot!(diagnostics, @r" - error[E119]: `SUPER` must be dereferenced to access its members. + assert_snapshot!(diagnostics, @" + error[E141]: Cannot access `x` on `POINTER TO parent`; dereference with `^` first ┌─ :11:19 │ 11 │ SUPER.x := 20; // Should be SUPER^.x - │ ^ `SUPER` must be dereferenced to access its members. + │ ^ Cannot access `x` on `POINTER TO parent`; dereference with `^` first - error[E119]: `SUPER` must be dereferenced to access its members. + error[E141]: Cannot access `ptr` on `POINTER TO parent`; dereference with `^` first ┌─ :14:19 │ 14 │ SUPER.ptr^ := 30; // Should be SUPER^.ptr^ - │ ^^^ `SUPER` must be dereferenced to access its members. + │ ^^^ Cannot access `ptr` on `POINTER TO parent`; dereference with `^` first warning[E049]: Illegal access to private member parent.ptr ┌─ :17:20 @@ -1386,18 +1374,18 @@ fn super_with_property_access_errors() { "#, ); - assert_snapshot!(diagnostics, @r" - error[E119]: `SUPER` must be dereferenced to access its members. + assert_snapshot!(diagnostics, @" + error[E141]: Cannot access `prop` on `POINTER TO parent`; dereference with `^` first ┌─ :21:19 │ 21 │ SUPER.prop := 10; // Should be SUPER^.prop - │ ^^^^ `SUPER` must be dereferenced to access its members. + │ ^^^^ Cannot access `prop` on `POINTER TO parent`; dereference with `^` first - error[E119]: `SUPER` must be dereferenced to access its members. + error[E141]: Cannot access `prop` on `POINTER TO parent`; dereference with `^` first ┌─ :22:24 │ 22 │ x := SUPER.prop; // Should be SUPER^.prop - │ ^^^^ `SUPER` must be dereferenced to access its members. + │ ^^^^ Cannot access `prop` on `POINTER TO parent`; dereference with `^` first error[E007]: Properties cannot be called like functions. Remove `()` ┌─ :25:13 diff --git a/src/validation/tests/this_keyword_validation_tests.rs b/src/validation/tests/this_keyword_validation_tests.rs index da47ecab363..f36fda1b58c 100644 --- a/src/validation/tests/this_keyword_validation_tests.rs +++ b/src/validation/tests/this_keyword_validation_tests.rs @@ -448,3 +448,34 @@ fn this_calling_functionblock_body_from_method_is_ok() { ); assert!(diagnostics.is_empty()); } + +#[test] +fn this_member_access_without_deref_is_an_error() { + // Regression test for #1728: `THIS.` (without `^`) used to slip + // through validation and panic in codegen with + // `Builder error: GEP pointee is not a struct`. It must surface as E120. + let diagnostics = parse_and_validate_buffered( + r#" + INTERFACE IAnimal + METHOD legs : DINT + END_METHOD + END_INTERFACE + + FUNCTION_BLOCK fb + VAR + a : IAnimal; + END_VAR + METHOD m : DINT + THIS.a.legs(); + END_METHOD + END_FUNCTION_BLOCK + "#, + ); + assert_snapshot!(diagnostics, @" + error[E141]: Cannot access `a` on `POINTER TO fb`; dereference with `^` first + ┌─ :12:22 + │ + 12 │ THIS.a.legs(); + │ ^ Cannot access `a` on `POINTER TO fb`; dereference with `^` first + "); +} diff --git a/tests/lit/single/oop/this_member_access_without_deref_errors.st b/tests/lit/single/oop/this_member_access_without_deref_errors.st new file mode 100644 index 00000000000..5224e6da20b --- /dev/null +++ b/tests/lit/single/oop/this_member_access_without_deref_errors.st @@ -0,0 +1,24 @@ +// RUN: rm -f %T/%basename_t.out +// RUN: not %COMPILE --error-format clang %s 2>&1 | %CHECK %s +// RUN: test ! -e %T/%basename_t.out + +// Regression test for #1728: `THIS.` without an explicit `^` deref +// must be rejected by validation, not silently passed through to codegen +// (where it used to crash with "GEP pointee is not a struct"). + +// CHECK: {{.*}}this_member_access_without_deref_errors.st:{{[0-9]+}}:{{[0-9]+}}:{{.*}}error[E141]: Cannot access `a` on `POINTER TO fb`; dereference with `^` first +// CHECK: error: Compilation aborted due to critical errors + +INTERFACE IAnimal + METHOD legs : DINT + END_METHOD +END_INTERFACE + +FUNCTION_BLOCK fb +VAR + a : IAnimal; +END_VAR +METHOD m : DINT + THIS.a.legs(); +END_METHOD +END_FUNCTION_BLOCK