Allow redefinition of non-physical vars in subclasses and private fields satisfying public interfaces

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_<name>" 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 =

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

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 =

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() {}

tests/misc/eval/projects/Issue12521/compile-fail.hxml

@@ -0,0 +1,2 @@
+-main Main
+--interp

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

tests/unit/src/unit/TestMain.hx

@@ -106,6 +106,7 @@ function main() {
 		#if (!flash && !hl && !cppia)
 		new TestCoroutines(),
 		#end
+		new TestRedefinition(),
 		// new TestUnspecified(),
 	];
 

tests/unit/src/unit/TestRedefinition.hx

@@ -0,0 +1,90 @@
+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 {
+	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";
+		// Non-physical setter has no storage, so the value is unchanged
+		eq(child.name, "child");
+
+		final parent:ParentWithNonPhysical = child;
+		eq(parent.name, "child");
+	}
+}

tests/unit/src/unit/issues/Issue12268.hx

@@ -0,0 +1,20 @@
+package unit.issues;
+
+interface I12268 {
+	function f():Int;
+}
+
+class C12268 implements I12268 {
+	public function new() {}
+
+	function f():Int
+		return 42;
+}
+
+class Issue12268 extends Test {
+	public function test() {
+		var c = new C12268();
+		var i:I12268 = c;
+		eq(i.f(), 42);
+	}
+}