Browse Source

[cs] support field name hash collisions in dynamic objects by storing collided fields in a special "conflicts" list (closes #5572)

Dan Korostelev 9 years ago
parent
commit
4e1aae795c

+ 80 - 10
src/generators/gencommon.ml

@@ -6555,6 +6555,14 @@ struct
 
 
 	let name = "reflection_cfs"
 	let name = "reflection_cfs"
 
 
+	type rcf_hash_conflict_ctx = {
+		t : t;
+		add_names : texpr->texpr->texpr;
+		get_conflict : texpr->texpr->texpr->texpr;
+		set : texpr->texpr->texpr->texpr->texpr;
+		delete : texpr->texpr->texpr->texpr;
+	}
+
 	type rcf_ctx =
 	type rcf_ctx =
 	{
 	{
 		rcf_gen : generator_ctx;
 		rcf_gen : generator_ctx;
@@ -6586,6 +6594,8 @@ struct
 
 
 		rcf_hash_paths : (path * int, string) Hashtbl.t;
 		rcf_hash_paths : (path * int, string) Hashtbl.t;
 
 
+		rcf_hash_conflict_ctx : rcf_hash_conflict_ctx option;
+
 		(*
 		(*
 			main expr -> field expr -> field string -> possible hash int (if optimize) -> possible set expr -> should_throw_exceptions -> changed expression
 			main expr -> field expr -> field string -> possible hash int (if optimize) -> possible set expr -> should_throw_exceptions -> changed expression
 
 
@@ -6596,7 +6606,7 @@ struct
 		rcf_on_call_field : texpr->texpr->string->int32 option->texpr list->texpr;
 		rcf_on_call_field : texpr->texpr->string->int32 option->texpr list->texpr;
 	}
 	}
 
 
-	let new_ctx gen ft object_iface optimize dynamic_getset_field dynamic_call_field hash_function lookup_function insert_function remove_function =
+	let new_ctx gen ft object_iface optimize dynamic_getset_field dynamic_call_field hash_function lookup_function insert_function remove_function hash_conflict_ctx =
 		{
 		{
 			rcf_gen = gen;
 			rcf_gen = gen;
 			rcf_ft = ft;
 			rcf_ft = ft;
@@ -6618,6 +6628,7 @@ struct
 
 
 			rcf_on_getset_field = dynamic_getset_field;
 			rcf_on_getset_field = dynamic_getset_field;
 			rcf_on_call_field = dynamic_call_field;
 			rcf_on_call_field = dynamic_call_field;
+			rcf_hash_conflict_ctx = hash_conflict_ctx;
 		}
 		}
 
 
 	(*
 	(*
@@ -6714,7 +6725,7 @@ struct
 
 
 	let mk_throw ctx str pos = { eexpr = TThrow (ExprBuilder.make_string ctx.rcf_gen.gcon str pos); etype = ctx.rcf_gen.gcon.basic.tvoid; epos = pos }
 	let mk_throw ctx str pos = { eexpr = TThrow (ExprBuilder.make_string ctx.rcf_gen.gcon str pos); etype = ctx.rcf_gen.gcon.basic.tvoid; epos = pos }
 
 
-	let enumerate_dynamic_fields ctx cl when_found =
+	let enumerate_dynamic_fields ctx cl when_found base_arr =
 		let gen = ctx.rcf_gen in
 		let gen = ctx.rcf_gen in
 		let basic = gen.gcon.basic in
 		let basic = gen.gcon.basic in
 		let pos = cl.cl_pos in
 		let pos = cl.cl_pos in
@@ -6749,6 +6760,12 @@ struct
 			mk_for (mk_this (gen.gmk_internal_name "hx" "hashes") (gen.gclasses.nativearray basic.tint)) (mk_this (gen.gmk_internal_name "hx" "length") basic.tint)
 			mk_for (mk_this (gen.gmk_internal_name "hx" "hashes") (gen.gclasses.nativearray basic.tint)) (mk_this (gen.gmk_internal_name "hx" "length") basic.tint)
 			@
 			@
 			mk_for (mk_this (gen.gmk_internal_name "hx" "hashes_f") (gen.gclasses.nativearray basic.tint)) (mk_this (gen.gmk_internal_name "hx" "length_f") basic.tint)
 			mk_for (mk_this (gen.gmk_internal_name "hx" "hashes_f") (gen.gclasses.nativearray basic.tint)) (mk_this (gen.gmk_internal_name "hx" "length_f") basic.tint)
+			@
+			(
+				let conflict_ctx = Option.get ctx.rcf_hash_conflict_ctx in
+				let ehead = mk_this (gen.gmk_internal_name "hx" "conflicts") conflict_ctx.t in
+				[conflict_ctx.add_names ehead base_arr]
+			)
 		else
 		else
 			mk_for (mk_this (gen.gmk_internal_name "hx" "hashes") (gen.gclasses.nativearray basic.tstring)) (mk_this (gen.gmk_internal_name "hx" "length") basic.tint)
 			mk_for (mk_this (gen.gmk_internal_name "hx" "hashes") (gen.gclasses.nativearray basic.tstring)) (mk_this (gen.gmk_internal_name "hx" "length") basic.tint)
 			@
 			@
@@ -6766,7 +6783,7 @@ struct
 		 A binary search or linear search algorithm may be implemented. The only need is that if not found, the NegBits of
 		 A binary search or linear search algorithm may be implemented. The only need is that if not found, the NegBits of
 		 the place where it should be inserted must be returned.
 		 the place where it should be inserted must be returned.
 	*)
 	*)
-	let abstract_dyn_lookup_implementation ctx this hash_local may_value is_float pos =
+	let abstract_dyn_lookup_implementation ctx this field_local hash_local may_value is_float pos =
 		let gen = ctx.rcf_gen in
 		let gen = ctx.rcf_gen in
 		let basic = gen.gcon.basic in
 		let basic = gen.gcon.basic in
 		let mk_this field t = { (mk_field_access gen this field pos) with etype = t } in
 		let mk_this field t = { (mk_field_access gen this field pos) with etype = t } in
@@ -6827,7 +6844,26 @@ struct
 						epos = pos;
 						epos = pos;
 					})); etype = ret_t; epos = pos }
 					})); etype = ret_t; epos = pos }
 				] in
 				] in
-				block
+
+				if ctx.rcf_optimize then
+					let conflict_ctx = Option.get ctx.rcf_hash_conflict_ctx in
+					let ehead = mk_this (gen.gmk_internal_name "hx" "conflicts") conflict_ctx.t in
+					let vconflict = alloc_var "conflict" conflict_ctx.t in
+					let local_conflict = mk_local vconflict pos in 
+					[mk (TIf (
+						mk (TBinop (OpLt, hash_local, ExprBuilder.make_int gen.gcon 0 pos)) basic.tbool pos,
+						mk (TBlock [
+							mk (TVar (vconflict, Some (conflict_ctx.get_conflict ehead hash_local field_local))) basic.tvoid pos;
+							mk (TIf (
+								mk (TBinop (OpNotEq, local_conflict, mk (TConst TNull) local_conflict.etype pos)) basic.tbool pos,
+								mk_return (Codegen.field local_conflict "value" t_dynamic pos),
+								None
+							)) basic.tvoid pos;
+						]) basic.tvoid pos,
+						Some (mk (TBlock block) basic.tvoid pos) 
+					)) basic.tvoid pos]
+				else
+					block
 			| Some value_local ->
 			| Some value_local ->
 				(*
 				(*
 					//if is not float:
 					//if is not float:
@@ -6883,9 +6919,21 @@ struct
 					ctx.rcf_insert_function fst_hash fst_length neg_res hash_local;
 					ctx.rcf_insert_function fst_hash fst_length neg_res hash_local;
 					ctx.rcf_insert_function fst_dynamics fst_length neg_res value_local;
 					ctx.rcf_insert_function fst_dynamics fst_length neg_res value_local;
 					mk (TUnop(Increment,Postfix,fst_length)) basic.tint pos;
 					mk (TUnop(Increment,Postfix,fst_length)) basic.tint pos;
-					mk_return value_local
 				] in
 				] in
-				block
+
+				let block =
+					if ctx.rcf_optimize then
+						let conflict_ctx = Option.get ctx.rcf_hash_conflict_ctx in
+						let ehead = mk_this (gen.gmk_internal_name "hx" "conflicts") conflict_ctx.t in
+						[mk (TIf (
+							mk (TBinop (OpLt, hash_local, ExprBuilder.make_int gen.gcon 0 pos)) basic.tbool pos,
+							conflict_ctx.set ehead hash_local field_local value_local,
+							Some (mk (TBlock block) basic.tvoid pos) 
+						)) basic.tvoid pos]
+					else
+						block
+				in
+				block @ [mk_return value_local]
 
 
 	let get_delete_field ctx cl is_dynamic =
 	let get_delete_field ctx cl is_dynamic =
 		let pos = cl.cl_pos in
 		let pos = cl.cl_pos in
@@ -6934,7 +6982,7 @@ struct
 
 
 				return false;
 				return false;
 			*)
 			*)
-			[
+			let common = [
 				{ eexpr = TVar(res,Some(ctx.rcf_hash_function local_switch_var hx_hashes hx_length)); etype = basic.tvoid; epos = pos };
 				{ eexpr = TVar(res,Some(ctx.rcf_hash_function local_switch_var hx_hashes hx_length)); etype = basic.tvoid; epos = pos };
 				{
 				{
 					eexpr = TIf(gte, { eexpr = TBlock([
 					eexpr = TIf(gte, { eexpr = TBlock([
@@ -6955,7 +7003,20 @@ struct
 					epos = pos;
 					epos = pos;
 				};
 				};
 				mk_return { eexpr = TConst(TBool false); etype = basic.tbool; epos = pos }
 				mk_return { eexpr = TConst(TBool false); etype = basic.tbool; epos = pos }
-			]
+			] in
+
+			if ctx.rcf_optimize then
+				let v_name = match tf_args with (v,_) :: _ -> v | _ -> assert false in
+				let local_name = mk_local v_name pos in
+				let conflict_ctx = Option.get ctx.rcf_hash_conflict_ctx in
+				let ehead = mk_this (gen.gmk_internal_name "hx" "conflicts") conflict_ctx.t in 
+				(mk (TIf (
+					mk (TBinop (OpLt, local_switch_var, ExprBuilder.make_int gen.gcon 0 pos)) basic.tbool pos,
+					mk (TReturn (Some (conflict_ctx.delete ehead local_switch_var local_name))) basic.tvoid pos,
+					None
+				)) basic.tvoid pos) :: common
+			else
+				common
 		end else
 		end else
 		[
 		[
 			mk_return { eexpr = TConst(TBool false); etype = basic.tbool; epos = pos }
 			mk_return { eexpr = TConst(TBool false); etype = basic.tbool; epos = pos }
@@ -7185,6 +7246,14 @@ struct
 					List.iter (fun cf -> cf.cf_expr <- Some { eexpr = TCall(mk_local v_nativearray pos, []); etype = cf.cf_type; epos = cf.cf_pos }) new_fields
 					List.iter (fun cf -> cf.cf_expr <- Some { eexpr = TCall(mk_local v_nativearray pos, []); etype = cf.cf_type; epos = cf.cf_pos }) new_fields
 				);
 				);
 
 
+				let new_fields =
+					if ctx.rcf_optimize then
+						let f = mk_class_field (gen.gmk_internal_name "hx" "conflicts") (Option.get ctx.rcf_hash_conflict_ctx).t false pos (Var { v_read = AccNormal; v_write = AccNormal }) [] in
+						f :: new_fields
+					else
+						new_fields
+				in
+
 				let delete = get_delete_field ctx cl true in
 				let delete = get_delete_field ctx cl true in
 
 
 				let new_fields = new_fields @ [
 				let new_fields = new_fields @ [
@@ -7335,7 +7404,8 @@ struct
 			(* callback : is_float fields_args switch_var throw_errors_option is_check_option value_option : texpr list *)
 			(* callback : is_float fields_args switch_var throw_errors_option is_check_option value_option : texpr list *)
 			if is_first_dynamic cl then
 			if is_first_dynamic cl then
 				create_cfs true (fun is_float fields_args switch_var _ _ value_opt ->
 				create_cfs true (fun is_float fields_args switch_var _ _ value_opt ->
-					abstract_dyn_lookup_implementation ctx this (mk_local switch_var pos) (Option.map (fun v -> mk_local v pos) value_opt) is_float pos
+					let v_name = match fields_args with (v,_) :: _ -> v | _ -> assert false in
+					abstract_dyn_lookup_implementation ctx this (mk_local v_name pos) (mk_local switch_var pos) (Option.map (fun v -> mk_local v pos) value_opt) is_float pos
 				)
 				)
 		end else if not is_override then begin
 		end else if not is_override then begin
 			create_cfs false (fun is_float fields_args switch_var _ _ value_opt ->
 			create_cfs false (fun is_float fields_args switch_var _ _ value_opt ->
@@ -7632,7 +7702,7 @@ struct
 		let exprs =
 		let exprs =
 			if is_some cl.cl_dynamic && is_first_dynamic cl then begin
 			if is_some cl.cl_dynamic && is_first_dynamic cl then begin
 				has_value := true;
 				has_value := true;
-				enumerate_dynamic_fields ctx cl mk_push
+				enumerate_dynamic_fields ctx cl mk_push base_arr
 			end else
 			end else
 				[]
 				[]
 		in
 		in

+ 16 - 1
src/generators/gencs.ml

@@ -2841,6 +2841,8 @@ let configure gen =
 	else
 	else
 		TypeParams.RealTypeParams.RealTypeParamsModf.configure gen (TypeParams.RealTypeParams.RealTypeParamsModf.set_only_hxgeneric gen);
 		TypeParams.RealTypeParams.RealTypeParamsModf.configure gen (TypeParams.RealTypeParams.RealTypeParamsModf.set_only_hxgeneric gen);
 
 
+	let flookup_cl = get_cl (get_type gen (["haxe";"lang"], "FieldLookup")) in
+
 	let rcf_ctx =
 	let rcf_ctx =
 		ReflectionCFs.new_ctx
 		ReflectionCFs.new_ctx
 			gen
 			gen
@@ -2859,6 +2861,20 @@ let configure gen =
 				let t = gen.gclasses.nativearray_type hash_array.etype in
 				let t = gen.gclasses.nativearray_type hash_array.etype in
 				{ hash_array with eexpr = TCall(rcf_static_remove t, [hash_array; length; pos]); etype = gen.gcon.basic.tvoid }
 				{ hash_array with eexpr = TCall(rcf_static_remove t, [hash_array; length; pos]); etype = gen.gcon.basic.tvoid }
 			)
 			)
+			(
+				let delete = mk_static_field_access_infer flookup_cl "deleteHashConflict" null_pos [] in
+				let get = mk_static_field_access_infer flookup_cl "getHashConflict" null_pos [] in
+				let set = mk_static_field_access_infer flookup_cl "setHashConflict" null_pos [] in
+				let add = mk_static_field_access_infer flookup_cl "addHashConflictNames" null_pos [] in
+				let conflict_t = TInst (get_cl (get_type gen (["haxe"; "lang"], "FieldHashConflict")), []) in
+				Some {
+					t = conflict_t;
+					get_conflict = (fun ehead ehash ename -> mk (TCall (get, [ehead; ehash; ename])) conflict_t ehead.epos);
+					set = (fun ehead ehash ename evalue -> mk (TCall (set, [ehead; ehash; ename; evalue])) basic.tvoid ehead.epos);
+					delete = (fun ehead ehash ename -> mk (TCall (delete, [ehead; ehash; ename])) basic.tbool ehead.epos);
+					add_names = (fun ehead earr -> mk (TCall (add, [ehead; earr])) basic.tvoid ehead.epos);
+				}
+			)
 	in
 	in
 
 
 	ReflectionCFs.UniversalBaseClass.configure gen (get_cl (get_type gen (["haxe";"lang"],"HxObject")) ) object_iface dynamic_object;
 	ReflectionCFs.UniversalBaseClass.configure gen (get_cl (get_type gen (["haxe";"lang"],"HxObject")) ) object_iface dynamic_object;
@@ -3175,7 +3191,6 @@ let configure gen =
 	let hashes = Hashtbl.fold (fun i s acc -> incr nhash; (normalize_i i,s) :: acc) rcf_ctx.rcf_hash_fields [] in
 	let hashes = Hashtbl.fold (fun i s acc -> incr nhash; (normalize_i i,s) :: acc) rcf_ctx.rcf_hash_fields [] in
 	let hashes = List.sort (fun (i,s) (i2,s2) -> compare i i2) hashes in
 	let hashes = List.sort (fun (i,s) (i2,s2) -> compare i i2) hashes in
 
 
-	let flookup_cl = get_cl (get_type gen (["haxe";"lang"], "FieldLookup")) in
 	let haxe_libs = List.filter (function (_,_,_,lookup) -> is_some (lookup (["haxe";"lang"], "DceNo"))) gen.gcon.net_libs in
 	let haxe_libs = List.filter (function (_,_,_,lookup) -> is_some (lookup (["haxe";"lang"], "DceNo"))) gen.gcon.net_libs in
 	(try
 	(try
 		(* first let's see if we're adding a -net-lib that has already a haxe.lang.FieldLookup *)
 		(* first let's see if we're adding a -net-lib that has already a haxe.lang.FieldLookup *)

+ 1 - 0
src/generators/genjava.ml

@@ -2182,6 +2182,7 @@ let configure gen =
 				let t = gen.gclasses.nativearray_type hash_array.etype in
 				let t = gen.gclasses.nativearray_type hash_array.etype in
 				{ hash_array with eexpr = TCall(rcf_static_remove t, [hash_array; length; pos]); etype = gen.gcon.basic.tvoid }
 				{ hash_array with eexpr = TCall(rcf_static_remove t, [hash_array; length; pos]); etype = gen.gcon.basic.tvoid }
 			)
 			)
+			None
 		in
 		in
 
 
 	ReflectionCFs.UniversalBaseClass.configure gen (get_cl (get_type gen (["haxe";"lang"],"HxObject")) ) object_iface dynamic_object;
 	ReflectionCFs.UniversalBaseClass.configure gen (get_cl (get_type gen (["haxe";"lang"],"HxObject")) ) object_iface dynamic_object;

+ 70 - 0
std/cs/internal/FieldLookup.hx

@@ -21,6 +21,21 @@
  */
  */
 package cs.internal;
 package cs.internal;
 
 
+@:native('haxe.lang.FieldHashConflict')
+@:final @:nativeGen
+@:keep class FieldHashConflict {
+	@:readOnly public var hash(default,never):Int;
+	@:readOnly public var name(default,never):String;
+	public var value:Dynamic;
+	public var next:FieldHashConflict;
+	public function new(hash, name, value, next) {
+		untyped this.hash = hash;
+		untyped this.name = name;
+		this.value = value;
+		this.next = next;
+	}
+}
+
 @:native('haxe.lang.FieldLookup')
 @:native('haxe.lang.FieldLookup')
 @:final @:nativeGen
 @:final @:nativeGen
 @:classCode("#pragma warning disable 628\n")
 @:classCode("#pragma warning disable 628\n")
@@ -325,4 +340,59 @@ package cs.internal;
 	static function insertDynamic(a:cs.NativeArray<Dynamic>, length:Int, pos:Int, x:Dynamic):cs.NativeArray<Dynamic> return __insert(a, length, pos, x);
 	static function insertDynamic(a:cs.NativeArray<Dynamic>, length:Int, pos:Int, x:Dynamic):cs.NativeArray<Dynamic> return __insert(a, length, pos, x);
 	static function insertString(a:cs.NativeArray<String>, length:Int, pos:Int, x:Dynamic):cs.NativeArray<String> return __insert(a, length, pos, x);
 	static function insertString(a:cs.NativeArray<String>, length:Int, pos:Int, x:Dynamic):cs.NativeArray<String> return __insert(a, length, pos, x);
 	#end
 	#end
+
+	static function getHashConflict(head:FieldHashConflict, hash:Int, name:String):FieldHashConflict {
+		while (head != null) {
+			if (head.hash == hash && head.name == name) {
+				return head;
+			}
+			head = head.next;
+		}
+		return null;
+	}
+
+	static function setHashConflict(head:cs.Ref<FieldHashConflict>, hash:Int, name:String, value:Dynamic):Void {
+		var node = head;
+		while (node != null) {
+			if (node.hash == hash && node.name == name) {
+				node.value = value;
+				return;
+			}
+			node = node.next;
+		}
+		head = new FieldHashConflict(hash, name, value, head);
+	}
+
+	static function deleteHashConflict(head:cs.Ref<FieldHashConflict>, hash:Int, name:String):Bool {
+		// no conflicting fields at all
+		if (head == null) {
+			return false;
+		}
+
+		// list head is conflicting - just point it to the next one
+		if (head.hash == hash && head.name == name) {
+			head = head.next;
+			return true;
+		}
+
+		// loop through the list, removing node if there's one
+		var prev = head, node = head.next;
+		while (node != null) {
+			if (node.hash == hash && node.name == name) {
+				prev.next = node.next;
+				return true;
+			}
+			node = node.next;
+		}
+
+		// not found
+		return false;
+	}
+
+	static function addHashConflictNames(head:FieldHashConflict, arr:Array<String>):Void {
+		while (head != null) {
+			arr.push(head.name);
+			head = head.next;
+		}
+	}
 }
 }

+ 28 - 0
tests/unit/src/unit/issues/Issue5572.hx

@@ -0,0 +1,28 @@
+package unit.issues;
+
+class Issue5572 extends unit.Test {
+	// these two fields have the same hash value
+	static var field1 = "baseReward";
+	static var field2 = "user_3235_65290";
+
+	function test() {
+		var o = {};
+		Reflect.setField(o, field1, 1); // first, goes into hashes array 
+		Reflect.setField(o, field2, 2); // second, added to object's "conflicts"
+		Reflect.setField(o, field2, 3); // should find one from "conflicts" and change the value
+		eq(Reflect.field(o, field1), 1); // retrieved from the hashes array
+		eq(Reflect.field(o, field2), 3); // retreived from "conflicts"
+		var expectedFields = [field1, field2];
+		for (field in Reflect.fields(o)) {
+			if (!expectedFields.remove(field))
+				t(false);
+		}
+		eq(expectedFields.length, 0);
+		Reflect.deleteField(o, field1); // removed from hashes array
+		Reflect.deleteField(o, field2); // removed from "conflicts"
+		eq(Reflect.field(o, field1), null);
+		eq(Reflect.field(o, field2), null);
+		eq(Reflect.fields(o).length, 0);
+
+	}
+}