Skip to content

Commit

Permalink
[cs] Fixed issue with OpAssignOp and OpAssign on Null<ValueType> types.
Browse files Browse the repository at this point in the history 
Closes HaxeFoundation#2754
  • Loading branch information
@waneck
waneck committed Mar 15, 2014
1 parent 63ea1ff commit 2433b43
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 9 deletions.
13 changes: 7 additions & 6 deletions gencommon.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2482,7 +2482,8 @@ struct

let priority_as_synf = solve_deps name [DBefore DynamicOperators.priority_as_synf; DBefore DynamicFieldAccess.priority_as_synf]

let default_implementation gen (should_change:texpr->bool) (get_fun:string) (set_fun:string) =
(* should change signature: tarray expr -> binop operation -> should change? *)
let default_implementation gen (should_change:texpr->Ast.binop option->bool) (get_fun:string) (set_fun:string) =
let basic = gen.gcon.basic in
let mk_get e e1 e2 =
let efield = mk_field_access gen e1 get_fun e.epos in
Expand All @@ -2496,10 +2497,10 @@ struct
match e.eexpr with
| TArray(e1, e2) ->
(* e1 should always be a var; no need to map there *)
if should_change e then mk_get e (run e1) (run e2) else Type.map_expr run e
| TBinop (Ast.OpAssign, ({ eexpr = TArray(e1a,e2a) } as earray), evalue) when should_change earray ->
if should_change e None then mk_get e (run e1) (run e2) else Type.map_expr run e
| TBinop (Ast.OpAssign, ({ eexpr = TArray(e1a,e2a) } as earray), evalue) when should_change earray (Some Ast.OpAssign) ->
mk_set e (run e1a) (run e2a) (run evalue)
| TBinop (Ast.OpAssignOp op,({ eexpr = TArray(e1a,e2a) } as earray) , evalue) when should_change earray ->
| TBinop (Ast.OpAssignOp op,({ eexpr = TArray(e1a,e2a) } as earray) , evalue) when should_change earray (Some (Ast.OpAssignOp op)) ->
(* cache all arguments in vars so they don't get executed twice *)
(* let ensure_local gen block name e = *)
let block = ref [] in
Expand All @@ -2510,7 +2511,7 @@ struct

{ e with eexpr = TBlock (List.rev !block) }
| TUnop(op, flag, ({ eexpr = TArray(e1a, e2a) } as earray)) ->
if should_change earray && match op with | Not | Neg -> false | _ -> true then begin
if should_change earray None && match op with | Not | Neg -> false | _ -> true then begin

let block = ref [] in

Expand Down Expand Up @@ -9386,7 +9387,7 @@ struct
| Some t1, Some t2 ->
(match op with
| Ast.OpAssign ->
{ e with eexpr = TBinop( op, run e1, handle_wrap ( handle_unwrap t2 (run e2) ) t1 ) }
Type.map_expr run e
| Ast.OpAssignOp op ->
(match e1.eexpr with
| TLocal _ ->
Expand Down
7 changes: 5 additions & 2 deletions gencs.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2403,13 +2403,16 @@ let configure gen =

InitFunction.configure gen true;
TArrayTransform.configure gen (TArrayTransform.default_implementation gen (
fun e ->
fun e binop ->
match e.eexpr with
| TArray(e1, e2) ->
( match follow e1.etype with
| TDynamic _ | TAnon _ | TMono _ -> true
| TInst({ cl_kind = KTypeParameter _ }, _) -> true
| _ -> false )
| _ -> match binop, change_param_type (t_to_md e1.etype) [e.etype] with
| Some(Ast.OpAssignOp _), ([TDynamic _] | [TAnon _]) ->
true
| _ -> false)
| _ -> assert false
) "__get" "__set" );

Expand Down
2 changes: 1 addition & 1 deletion genjava.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,7 @@ let configure gen =

InitFunction.configure gen true;
TArrayTransform.configure gen (TArrayTransform.default_implementation gen (
fun e ->
fun e _ ->
match e.eexpr with
| TArray ({ eexpr = TLocal { v_extra = Some( _ :: _, _) } }, _) -> (* captured transformation *)
false
Expand Down
170 changes: 170 additions & 0 deletions tests/unit/issues/Issue2754.hx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
package unit.issues;

class Issue2754 extends unit.Test
{
public function testClass()
{
var tc = new TestClass();

// initialized to null
nulleq(tc.dyn,null);
nulleq(tc.nullInt,null);
nulleq(tc.nullBool,null);
nulleq(tc.nullFloat,null);
nulleq(tc.nullArrayInt,null);
nulleq(tc.nullArrayNullInt,null);

// explicitly set to null
tc.dyn = null;
tc.nullInt = null;
tc.nullBool = null;
tc.nullFloat = null;
tc.nullArrayInt = null;
tc.nullArrayNullInt = null;
nulleq(tc.dyn,null);
nulleq(tc.nullInt,null);
nulleq(tc.nullBool,null);
nulleq(tc.nullFloat,null);
nulleq(tc.nullArrayInt,null);
nulleq(tc.nullArrayNullInt,null);

// set to null from dynamic
var dyn:Dynamic = null;
tc.dyn = dyn;
tc.nullInt = dyn;
tc.nullBool = dyn;
tc.nullFloat = dyn;
tc.nullArrayInt = dyn;
tc.nullArrayNullInt = dyn;
nulleq(tc.dyn,null);
nulleq(tc.nullInt,null);
nulleq(tc.nullBool,null);
nulleq(tc.nullFloat,null);
nulleq(tc.nullArrayInt,null);
nulleq(tc.nullArrayNullInt,null);

// set to null from complex expression
tc.dyn = complexGetNull();
tc.nullInt = complexGetNull();
tc.nullBool = complexGetNull();
tc.nullFloat = complexGetNull();
tc.nullArrayInt = complexGetNull();
tc.nullArrayNullInt = complexGetNull();
nulleq(tc.dyn,null);
nulleq(tc.nullInt,null);
nulleq(tc.nullBool,null);
nulleq(tc.nullFloat,null);
nulleq(tc.nullArrayInt,null);
nulleq(tc.nullArrayNullInt,null);

// set to null from generic function
tc.dyn = getNull();
tc.nullInt = getNull();
tc.nullBool = getNull();
tc.nullFloat = getNull();
tc.nullArrayInt = getNull();
tc.nullArrayNullInt = getNull();
nulleq(tc.dyn,null);
nulleq(tc.nullInt,null);
nulleq(tc.nullBool,null);
nulleq(tc.nullFloat,null);
nulleq(tc.nullArrayInt,null);
nulleq(tc.nullArrayNullInt,null);

// Null<Bool>
dyn = true;
tc.dyn = dyn;
tc.nullBool = dyn;
eq(tc.dyn,true);
eq(tc.nullBool,true);
tc.dyn = !dyn;
tc.nullBool = !dyn;
eq(tc.dyn,false);
eq(tc.nullBool,false);
tc.dyn = !tc.nullBool;
tc.nullBool = !tc.nullBool;
eq(tc.dyn,true);
eq(tc.nullBool,true);

// Null<Int> / Null<Float>

//from Dynamic
dyn = 42;
tc.dyn = dyn;
tc.nullInt = dyn;
tc.nullFloat = dyn;
tc.nullArrayInt = [dyn];
tc.nullArrayNullInt = [dyn];
eq(tc.dyn,42);
eq(tc.nullInt,42);
eq(tc.nullFloat,42);
eq(tc.nullArrayInt[0],42);
eq(tc.nullArrayNullInt[0],42);

//from Null<Int>
var ni:Null<Int> = 21;
tc.dyn = ni;
tc.nullInt = ni;
tc.nullFloat = ni;
tc.nullArrayInt = [ni];
tc.nullArrayNullInt = [ni];
eq(tc.dyn,21);
eq(tc.nullInt,21);
eq(tc.nullFloat,21);
eq(tc.nullArrayInt[0],21);
eq(tc.nullArrayNullInt[0],21);

//binops
ni = null;
tc.dyn = tc.nullInt*2 + ni;
tc.nullInt = tc.nullInt*tc.nullArrayNullInt[0] + tc.nullArrayNullInt[1] + ni;
tc.nullFloat = tc.nullFloat*tc.nullArrayNullInt[0] + tc.nullArrayNullInt[1] + ni;
tc.nullArrayInt[0] += tc.nullArrayNullInt[0];
tc.nullArrayNullInt[0] *= tc.nullArrayNullInt[0] - tc.nullArrayNullInt[1];
eq(tc.dyn, 42);
eq(tc.nullInt, 441);
eq(tc.nullFloat, 441);
eq(tc.nullArrayInt[0], 42);
eq(tc.nullArrayNullInt[0], 441);
}

private static inline function complexGetNull<T>():Null<T>
{
var x = 10;
var y = 1000;
x += y; //just do something here
return null;
}

private static inline function getNull<T>():Null<T>
{
return null;
}

// In normal 'eq', the types are inferred as <int> rather than <Null<Int>>
// which kind of defeats the purpose of the checks
private function nulleq<T>(v1:Null<T>, v2:Null<T>,?pos:haxe.PosInfos)
{
this.infos("v1 == null");
this.t(v1 == null,pos);
this.infos("v2 == null");
this.t(v2 == null,pos);
this.eq(v1,v2);
this.infos(null);
}
}

private class TestClass
{
public var dyn:Dynamic;
public var nullInt:Null<Int>;
public var nullBool:Null<Bool>;
public var nullFloat:Null<Float>;
public var nullArrayInt:Null<Array<Int>>;
public var nullArrayNullInt:Null<Array<Null<Int>>>;

public function new()
{

}
}

0 comments on commit 2433b43

Please sign in to comment.