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

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

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);
+	}
+}