From c66ffc762da2da84504095da58b5335814df860a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 06:08:19 +0000 Subject: [PATCH 1/4] Initial plan From 82b2917b3d13fa40ae721ad09f5656ba45e3e7f0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 06:24:33 +0000 Subject: [PATCH 2/4] Allow redefinition of non-physical vars (#12521) and private interface impl (#12268) Co-authored-by: Simn <634365+Simn@users.noreply.github.com> --- src/typing/typeloadCheck.ml | 2 +- src/typing/typeloadFields.ml | 30 +++++- tests/misc/eval/projects/Issue12521/Main.hx | 54 +++++++++++ .../projects/Issue12521/compile-fail.hxml | 2 + .../Issue12521/compile-fail.hxml.stderr | 8 ++ tests/unit/src/unit/TestMain.hx | 1 + tests/unit/src/unit/TestRedefinition.hx | 91 +++++++++++++++++++ tests/unit/src/unit/issues/Issue12268.hx | 21 +++++ 8 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 tests/misc/eval/projects/Issue12521/Main.hx create mode 100644 tests/misc/eval/projects/Issue12521/compile-fail.hxml create mode 100644 tests/misc/eval/projects/Issue12521/compile-fail.hxml.stderr create mode 100644 tests/unit/src/unit/TestRedefinition.hx create mode 100644 tests/unit/src/unit/issues/Issue12268.hx diff --git a/src/typing/typeloadCheck.ml b/src/typing/typeloadCheck.ml index 0ba15b68565..887cd113eec 100644 --- a/src/typing/typeloadCheck.ml +++ b/src/typing/typeloadCheck.ml @@ -427,7 +427,7 @@ module Inheritance = struct | MethDynamic -> 1 | MethMacro -> 2 in - if (has_class_field_flag f CfPublic) && not (has_class_field_flag f2 CfPublic) && not (Meta.has Meta.CompilerGenerated f.cf_meta) then + if (has_class_field_flag f CfPublic) && not (has_class_field_flag f2 CfPublic) && not (Meta.has Meta.CompilerGenerated f.cf_meta) && (has_class_flag c CExtern) then display_error com ("Field " ^ f.cf_name ^ " should be public as requested by " ^ s_type_path intf.cl_path) p else if not (unify_kind uctx f2.cf_kind f.cf_kind) || not (match f.cf_kind, f2.cf_kind with Var _ , Var _ -> true | Method m1, Method m2 -> mkind m1 = mkind m2 | _ -> false) then display_error com ("Field " ^ f.cf_name ^ " has different property access than in " ^ s_type_path intf.cl_path ^ ": " ^ s_kind f2.cf_kind ^ " should be " ^ s_kind f.cf_kind) p diff --git a/src/typing/typeloadFields.ml b/src/typing/typeloadFields.ml index ea333eb8882..b3e0e42cbc9 100644 --- a/src/typing/typeloadFields.ml +++ b/src/typing/typeloadFields.ml @@ -733,7 +733,35 @@ module TypeBinding = struct if not fctx.is_static && not cctx.is_lib then begin match get_declared cf.cf_name c.cl_super with | None -> () | Some (csup,_) -> - display_error ctx.com ("Redefinition of variable " ^ cf.cf_name ^ " in subclass is not allowed. Previously declared at " ^ (s_type_path csup.cl_path) ) cf.cf_name_pos + let error_redefinition () = + display_error ctx.com ("Redefinition of variable " ^ cf.cf_name ^ " in subclass is not allowed. Previously declared at " ^ (s_type_path csup.cl_path)) cf.cf_name_pos + in + (try + let cf_parent = PMap.find cf.cf_name csup.cl_fields in + if ( + has_class_field_flag cf_parent CfPublic + && not (has_class_field_flag cf CfPublic) + ) then + display_error ctx.com ("Variable " ^ cf.cf_name ^ " has less visibility (public/private) than superclass one") cf.cf_pos + else if is_physical_var_field cf_parent then + error_redefinition () + else begin + let is_narrowing parent_acc child_acc = match parent_acc, child_acc with + | AccCall, (AccPrivateCall | AccNever) -> true + | AccPrivateCall, AccNever -> true + | _ -> false + in + match cf.cf_kind, cf_parent.cf_kind with + | Var child_v, Var parent_v -> + if is_narrowing parent_v.v_read child_v.v_read then + display_error ctx.com ("Cannot narrow read access of " ^ cf.cf_name ^ " in subclass") cf.cf_name_pos + else if is_narrowing parent_v.v_write child_v.v_write then + display_error ctx.com ("Cannot narrow write access of " ^ cf.cf_name ^ " in subclass") cf.cf_name_pos + | _ -> + error_redefinition () + end + with Not_found -> + error_redefinition ()) end let bind_var_expression ctx_f cctx fctx cf e = diff --git a/tests/misc/eval/projects/Issue12521/Main.hx b/tests/misc/eval/projects/Issue12521/Main.hx new file mode 100644 index 00000000000..674c059f303 --- /dev/null +++ b/tests/misc/eval/projects/Issue12521/Main.hx @@ -0,0 +1,54 @@ +class PhysicalParent { + public var x:Int; + public function new() {} +} + +class PhysicalPropertyParent { + @:isVar public var x(get, set):Int; + public function new() {} + function get_x() return x; + function set_x(v) return x = v; +} + +class RedefinePhysical extends PhysicalParent { + public var x:Int; +} + +class RedefinePhysicalProperty extends PhysicalPropertyParent { + @:isVar public var x(get, set):Int; +} + +class RedefinePhysicalPropAsProperty extends PhysicalPropertyParent { + public var x(get, set):Int; +} + +class RedefinePhysicalAsProperty extends PhysicalParent { + public var x(get, set):Int; + function get_x() return 0; + function set_x(v) return v; +} + +class NonPhysicalParent { + public var x(get, set):Int; + public function new() {} + function get_x() return 0; + function set_x(v) return v; +} + +class NarrowRead extends NonPhysicalParent { + public var x(never, set):Int; +} + +class NarrowWrite extends NonPhysicalParent { + public var x(get, never):Int; +} + +class NarrowWrite2 extends NonPhysicalParent { + public var x(get, private set):Int; +} + +class PrivateVar extends NonPhysicalParent { + var x(get, set):Int; +} + +function main() {} diff --git a/tests/misc/eval/projects/Issue12521/compile-fail.hxml b/tests/misc/eval/projects/Issue12521/compile-fail.hxml new file mode 100644 index 00000000000..7d61297e2cb --- /dev/null +++ b/tests/misc/eval/projects/Issue12521/compile-fail.hxml @@ -0,0 +1,2 @@ +-main Main +--interp diff --git a/tests/misc/eval/projects/Issue12521/compile-fail.hxml.stderr b/tests/misc/eval/projects/Issue12521/compile-fail.hxml.stderr new file mode 100644 index 00000000000..898d144cef6 --- /dev/null +++ b/tests/misc/eval/projects/Issue12521/compile-fail.hxml.stderr @@ -0,0 +1,8 @@ +Main.hx:51: characters 2-22 : Variable x has less visibility (public/private) than superclass one +Main.hx:47: characters 13-14 : Cannot narrow write access of x in subclass +Main.hx:43: characters 13-14 : Cannot narrow write access of x in subclass +Main.hx:39: characters 13-14 : Cannot narrow read access of x in subclass +Main.hx:26: characters 13-14 : Redefinition of variable x in subclass is not allowed. Previously declared at PhysicalParent +Main.hx:22: characters 13-14 : Redefinition of variable x in subclass is not allowed. Previously declared at PhysicalPropertyParent +Main.hx:18: characters 21-22 : Redefinition of variable x in subclass is not allowed. Previously declared at PhysicalPropertyParent +Main.hx:14: characters 13-14 : Redefinition of variable x in subclass is not allowed. Previously declared at PhysicalParent diff --git a/tests/unit/src/unit/TestMain.hx b/tests/unit/src/unit/TestMain.hx index 8cc773e8283..6d31721e1dd 100644 --- a/tests/unit/src/unit/TestMain.hx +++ b/tests/unit/src/unit/TestMain.hx @@ -106,6 +106,7 @@ function main() { #if (!flash && !hl && !cppia) new TestCoroutines(), #end + new TestRedefinition(), // new TestUnspecified(), ]; diff --git a/tests/unit/src/unit/TestRedefinition.hx b/tests/unit/src/unit/TestRedefinition.hx new file mode 100644 index 00000000000..4154495ac0a --- /dev/null +++ b/tests/unit/src/unit/TestRedefinition.hx @@ -0,0 +1,91 @@ +package unit; + +abstract class Dispatcher { + public var scheduler(get, never):Float; + + abstract function get_scheduler():Float; +} + +class ConcreteDispatcher extends Dispatcher { + @:isVar public var scheduler(get, null):Int; + + public function new(v:Int) { + this.scheduler = v; + } + + function get_scheduler() { + return scheduler; + } +} + +class ParentWithNonPhysical { + public function new() {} + + public var name(get, private set):String; + + function get_name():String + return "parent"; + + function set_name(v:String):String + return "parent"; +} + +class ChildWidened extends ParentWithNonPhysical { + @:isVar public var name(get, set):String; + + public function new(v:String) { + super(); + this.name = v; + } + + override function get_name():String + return name; + + override function set_name(v:String):String + return name = v; +} + +class ChildWidenedNonPhysical extends ParentWithNonPhysical { + public var name(get, set):String; + + public function new() { + super(); + } + + override function get_name():String + return "child"; + + override function set_name(v:String):String + return v; +} + +class TestRedefinition extends Test { + // cppia doesn't support redefining non-physical vars from a parent class + #if !cppia + public function testDispatcher() { + final dispatcher = new ConcreteDispatcher(123); + eq(dispatcher.scheduler, 123); + final generalDispatcher:Dispatcher = dispatcher; + eq(generalDispatcher.scheduler, 123.0); + } + + public function testWidening() { + final child = new ChildWidened("child"); + eq(child.name, "child"); + child.name = "new child"; + eq(child.name, "new child"); + + final parent:ParentWithNonPhysical = child; + eq(parent.name, "new child"); + } + + public function testWideningNonPhysical() { + final child = new ChildWidenedNonPhysical(); + eq(child.name, "child"); + child.name = "new child"; + + final parent:ParentWithNonPhysical = child; + eq(parent.name, "child"); + } + #end +} diff --git a/tests/unit/src/unit/issues/Issue12268.hx b/tests/unit/src/unit/issues/Issue12268.hx new file mode 100644 index 00000000000..65e12e3a03c --- /dev/null +++ b/tests/unit/src/unit/issues/Issue12268.hx @@ -0,0 +1,21 @@ +package unit.issues; + +interface I12268 { + function f():Void; +} + +class C12268 implements I12268 { + public function new() {} + + function f() {} +} + +class Issue12268 extends Test { + public function test() { + var c = new C12268(); + var i:I12268 = c; + var ok = true; + i.f(); + t(ok); + } +} From dc53c440879526b8a68c725e59b43d085b9c5d02 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 06:26:13 +0000 Subject: [PATCH 3/4] Improve tests: add return value assertion and non-physical setter verification Co-authored-by: Simn <634365+Simn@users.noreply.github.com> --- tests/unit/src/unit/TestRedefinition.hx | 2 ++ tests/unit/src/unit/issues/Issue12268.hx | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/unit/src/unit/TestRedefinition.hx b/tests/unit/src/unit/TestRedefinition.hx index 4154495ac0a..8a1a0e67b38 100644 --- a/tests/unit/src/unit/TestRedefinition.hx +++ b/tests/unit/src/unit/TestRedefinition.hx @@ -83,6 +83,8 @@ class TestRedefinition extends Test { final child = new ChildWidenedNonPhysical(); eq(child.name, "child"); child.name = "new child"; + // Non-physical setter has no storage, so the value is unchanged + eq(child.name, "child"); final parent:ParentWithNonPhysical = child; eq(parent.name, "child"); diff --git a/tests/unit/src/unit/issues/Issue12268.hx b/tests/unit/src/unit/issues/Issue12268.hx index 65e12e3a03c..f21925db2f6 100644 --- a/tests/unit/src/unit/issues/Issue12268.hx +++ b/tests/unit/src/unit/issues/Issue12268.hx @@ -1,21 +1,20 @@ package unit.issues; interface I12268 { - function f():Void; + function f():Int; } class C12268 implements I12268 { public function new() {} - function f() {} + function f():Int + return 42; } class Issue12268 extends Test { public function test() { var c = new C12268(); var i:I12268 = c; - var ok = true; - i.f(); - t(ok); + eq(i.f(), 42); } } From 736c7ad80391d07abdf11e1f36b4a3bebc7eb636 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 07:12:56 +0000 Subject: [PATCH 4/4] Fix CPPIA duplicate member var for non-physical var redefinition Co-authored-by: Simn <634365+Simn@users.noreply.github.com> --- src/generators/cpp/gen/cppCppia.ml | 81 ++++++++++++++++++++++--- tests/unit/src/unit/TestRedefinition.hx | 3 - 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/generators/cpp/gen/cppCppia.ml b/src/generators/cpp/gen/cppCppia.ml index c13fab9f6a3..e8423a64b9b 100644 --- a/src/generators/cpp/gen/cppCppia.ml +++ b/src/generators/cpp/gen/cppCppia.ml @@ -542,6 +542,20 @@ class script_writer ctx filename asciiOut basic = val mutable just_finished_block = false val mutable classCount = 0 val mutable return_type = TMono (Monomorph.create ()) + (* Maps original field names to backing names for physical vars that redefine + a parent's non-physical var - e.g. "scheduler" -> "_hxb_scheduler". + Set per class in generate_script_class. *) + val mutable backing_field_remap : (string * string) list = [] + + method set_backing_remap remap = backing_field_remap <- remap + + (* Returns the backing field name to use in CPPIA for "this.fieldname" accesses. + Returns the mangled backing name if the field is being remapped, otherwise + returns the original name unchanged. *) + method remap_field_name name = + match List.assoc_opt name backing_field_remap with + | Some mangled -> mangled + | None -> name val buffer = Buffer.create 0 val identTable = Hashtbl.create 0 val fileTable = Hashtbl.create 0 @@ -1146,10 +1160,11 @@ class script_writer ctx filename asciiOut basic = ^ this#stringText field.cf_name ^ this#commentOf field.cf_name) | FInstance (_, _, field) when is_this obj -> + let name = this#remap_field_name field.cf_name in this#write (this#op IaFThisInst ^ typeText ^ " " - ^ this#stringText field.cf_name - ^ this#commentOf field.cf_name) + ^ this#stringText name + ^ this#commentOf name) | FInstance (_, _, field) -> this#write (this#op IaFLink ^ typeText ^ " " @@ -1803,16 +1818,59 @@ let generate_script_class common_ctx script class_def = let ordered_statics = List.filter (non_dodgy_function false) class_def.cl_ordered_statics in - let ordered_fields = + let all_ordered_fields = List.filter (non_dodgy_function true) class_def.cl_ordered_fields in + (* Check whether a class field redefines a non-physical var from the parent class. + This can happen when a subclass has a property with the same name as a parent + non-physical property. The parent has no physical backing slot for the field, so + both the parent and child would emit CPPIA metadata for the same name, causing a + "duplicate member var" error at runtime. *) + let redefines_parent_non_physical cf = + match cf.cf_kind with + | Var _ -> ( + match class_def.cl_super with + | None -> false + | Some (parent, _) -> ( + try + let parent_cf = PMap.find cf.cf_name parent.cl_fields in + (match parent_cf.cf_kind with + | Var _ -> not (is_physical_var_field parent_cf) + | _ -> false) + with Not_found -> false)) + | _ -> false + in + (* Build a remap from original name to backing name for physical vars that + redefine a parent's non-physical var. These get renamed to "_hxb_" in + the CPPIA metadata so they don't conflict with the parent's accessor-based + definition. The parent's IaAccessCall mechanism remains in place for external + access; internal "this.field" accesses within the class are remapped to use + the backing name directly via IaFThisInst. *) + let physical_backing_remap = + List.filter_map + (fun cf -> + if redefines_parent_non_physical cf && is_physical_var_field cf then + Some (cf.cf_name, "_hxb_" ^ cf.cf_name) + else None) + all_ordered_fields + in + (* Non-physical vars that redefine a parent's non-physical var can be skipped + entirely in CPPIA. There is no physical storage to register, and the overridden + accessor methods (get_/set_) handle all dispatch naturally. *) + let ordered_fields = + List.filter + (fun cf -> + not (redefines_parent_non_physical cf && not (is_physical_var_field cf))) + all_ordered_fields + in + script#set_backing_remap physical_backing_remap; script#write (string_of_int (List.length ordered_fields + List.length ordered_statics + (match class_def.cl_constructor with Some _ -> 1 | _ -> 0) + match TClass.get_cl_init class_def with Some _ -> 1 | _ -> 0) - ^ "\n"); + ^ "\n"); let generate_field isStatic field = match (field.cf_kind, follow field.cf_type) with @@ -1834,9 +1892,17 @@ let generate_script_class common_ctx script class_def = | AccInline -> IaAccessNormal | AccRequire (_, _) -> IaAccessNormal in - let isExtern = not (is_physical_field field) in - script#var (mode_code v.v_read) (mode_code v.v_write) isExtern isStatic - field.cf_name field.cf_type field.cf_expr + let backing_name = script#remap_field_name field.cf_name in + if backing_name <> field.cf_name then + (* Physical var redefining parent's non-physical: emit with direct-access + backing name so the CPPIA runtime doesn't see a duplicate. *) + script#var IaAccessNormal IaAccessNormal false isStatic + backing_name field.cf_type None + else begin + let isExtern = not (is_physical_field field) in + script#var (mode_code v.v_read) (mode_code v.v_write) isExtern isStatic + field.cf_name field.cf_type field.cf_expr + end | Method MethDynamic, TFun (args, ret) -> script#func isStatic true field.cf_name ret args (has_class_flag class_def CInterface) @@ -1864,6 +1930,7 @@ let generate_script_class common_ctx script class_def = List.iter (generate_field false) ordered_fields; List.iter (generate_field true) ordered_statics; + script#set_backing_remap []; script#write "\n" let generate_script_enum script enum_def meta = diff --git a/tests/unit/src/unit/TestRedefinition.hx b/tests/unit/src/unit/TestRedefinition.hx index 8a1a0e67b38..65a6e5c7b41 100644 --- a/tests/unit/src/unit/TestRedefinition.hx +++ b/tests/unit/src/unit/TestRedefinition.hx @@ -60,8 +60,6 @@ class ChildWidenedNonPhysical extends ParentWithNonPhysical { } class TestRedefinition extends Test { - // cppia doesn't support redefining non-physical vars from a parent class - #if !cppia public function testDispatcher() { final dispatcher = new ConcreteDispatcher(123); eq(dispatcher.scheduler, 123); @@ -89,5 +87,4 @@ class TestRedefinition extends Test { final parent:ParentWithNonPhysical = child; eq(parent.name, "child"); } - #end }