Skip to content

Commit d4bdcee

Browse files
SimnRblSb
Simn
and
RblSb
authored
Fix null coal assign (with less code duplication) (#11980)
* Fix null coal assign * Can i extract this TBinop? * Nope, so there is more tests * start implementing the ??= stuff * turn into real errors so we know what's happening * adjust test for now * implement AKField * implement AKAccessor * implement AKUsingAccessor * implement AKResolve * try something * fix and adjust --------- Co-authored-by: RblSb <[email protected]>
1 parent cd7ba8c commit d4bdcee

File tree

6 files changed

+367
-67
lines changed

6 files changed

+367
-67
lines changed

‎src/typing/operators.ml

+104-39
Original file line numberDiff line numberDiff line change
@@ -650,42 +650,38 @@ let process_lhs_expr ctx name e_lhs =
650650
let e = vr#get_expr name e_lhs in
651651
e,vr
652652

653-
let type_assign_op ctx op e1 e2 with_type p =
654-
let field_rhs_by_name op name ev with_type =
653+
type 'a assign_op_api = {
654+
akno_fallback : unit -> texpr;
655+
type_rhs : texpr -> expr -> 'a;
656+
to_texpr : value_reference -> 'a -> (texpr -> texpr) -> texpr;
657+
generate : value_reference -> texpr -> texpr -> texpr;
658+
assign : value_reference -> texpr -> 'a -> texpr;
659+
}
660+
661+
let handle_assign_op ctx api e1 e2 with_type p =
662+
let field_rhs_by_name name ev with_type =
655663
let access_get = type_field_default_cfg ctx ev name p MGet with_type in
656664
let e_get = acc_get ctx access_get in
657-
e_get.etype,type_binop2 ctx op e_get e2 true WithType.value p
665+
e_get,api.type_rhs e_get e2
658666
in
659-
let field_rhs op cf ev =
660-
field_rhs_by_name op cf.cf_name ev (WithType.with_type cf.cf_type)
667+
let field_rhs cf ev =
668+
field_rhs_by_name cf.cf_name ev (WithType.with_type cf.cf_type)
661669
in
662-
let assign vr e r_rhs =
663-
if BinopResult.needs_assign r_rhs then check_assign ctx e;
670+
let set vr fa e_lhs r_rhs el =
664671
let assign e_rhs =
665-
let e_rhs = AbstractCast.cast_or_unify ctx e.etype e_rhs p in
666-
match e_rhs.eexpr with
667-
| TBinop(op',e1',e2') when op = op' && Texpr.equal e e1' ->
668-
mk (TBinop(OpAssignOp op',e1',e2')) e.etype p
669-
| _ ->
670-
mk (TBinop(OpAssign,e,e_rhs)) e.etype p
671-
in
672-
let e = BinopResult.to_texpr vr r_rhs assign in
673-
vr#to_texpr e
674-
in
675-
let set vr fa t_lhs r_rhs el =
676-
let assign e_rhs =
677-
let e_rhs = AbstractCast.cast_or_unify ctx t_lhs e_rhs p in
672+
let e_rhs = AbstractCast.cast_or_unify ctx e_lhs.etype e_rhs p in
678673
let dispatcher = new call_dispatcher ctx (MSet (Some e2)) with_type p in
679674
dispatcher#accessor_call fa (el @ [e_rhs]) [];
680675
in
681-
let e = BinopResult.to_texpr vr r_rhs assign in
682-
vr#to_texpr e
676+
let e = api.to_texpr vr r_rhs assign in
677+
api.generate vr e_lhs e
683678
in
684679
let rec loop acc = match acc with
685680
| AKNo(_,p) ->
686681
(* try abstract operator overloading *)
687682
begin try
688-
type_non_assign_op ctx op e1 e2 true true with_type p
683+
api.akno_fallback();
684+
(* type_non_assign_op ctx op e1 e2 true true with_type p *)
689685
with Not_found ->
690686
raise_typing_error "This expression cannot be accessed for writing" p
691687
end
@@ -695,23 +691,23 @@ let type_assign_op ctx op e1 e2 with_type p =
695691
raise_typing_error "Invalid operation" p
696692
| AKExpr e ->
697693
let e,vr = process_lhs_expr ctx "lhs" e in
698-
let e_rhs = type_binop2 ctx op e e2 true WithType.value p in
699-
assign vr e e_rhs
694+
let e_rhs = api.type_rhs e e2 in
695+
api.assign vr e e_rhs
700696
| AKField fa ->
701697
let vr = new value_reference ctx in
702698
let ef = vr#get_expr_part "fh" fa.fa_on in
703-
let _,e_rhs = field_rhs op fa.fa_field ef in
699+
let _,e_rhs = field_rhs fa.fa_field ef in
704700
let e_lhs = FieldAccess.get_field_expr {fa with fa_on = ef} FWrite in
705-
assign vr e_lhs e_rhs
701+
api.assign vr e_lhs e_rhs
706702
| AKAccessor fa ->
707703
let vr = new value_reference ctx in
708704
let ef = vr#get_expr_part "fh" fa.fa_on in
709-
let t_lhs,e_rhs = field_rhs op fa.fa_field ef in
710-
set vr {fa with fa_on = ef} t_lhs e_rhs []
705+
let e_lhs,e_rhs = field_rhs fa.fa_field ef in
706+
set vr {fa with fa_on = ef} e_lhs e_rhs []
711707
| AKUsingAccessor sea ->
712708
let fa = sea.se_access in
713709
let ef,vr = process_lhs_expr ctx "fh" sea.se_this in
714-
let t_lhs,e_rhs = field_rhs op fa.fa_field ef in
710+
let t_lhs,e_rhs = field_rhs fa.fa_field ef in
715711
set vr sea.se_access t_lhs e_rhs [ef]
716712
| AKAccess(a,tl,c,ebase,ekey) ->
717713
let cf_get,tf_get,r_get,ekey = AbstractCast.find_array_read_access ctx a tl ekey p in
@@ -724,16 +720,16 @@ let type_assign_op ctx op e1 e2 with_type p =
724720
in
725721
let ebase = maybe_bind_to_temp "base" ebase in
726722
let ekey = maybe_bind_to_temp "key" ekey in
727-
let eget = mk_array_get_call ctx (cf_get,tf_get,r_get,ekey) c ebase p in
728-
let eget = type_binop2 ctx op eget e2 true WithType.value p in
729-
let eget = BinopResult.to_texpr vr eget (fun e -> e) in
723+
let eread = mk_array_get_call ctx (cf_get,tf_get,r_get,ekey) c ebase p in
724+
let eget = api.type_rhs eread e2 in
725+
let eget = api.to_texpr vr eget (fun e -> e) in
730726
unify ctx eget.etype r_get p;
731727
let cf_set,tf_set,r_set,ekey,eget = AbstractCast.find_array_write_access ctx a tl ekey eget p in
732728
let et = type_module_type ctx (TClassDecl c) p in
733729
let e = match cf_set.cf_expr,cf_get.cf_expr with
734-
| None,None ->
730+
(* | None,None ->
735731
let ea = mk (TArray(ebase,ekey)) r_get p in
736-
mk (TBinop(OpAssignOp op,ea,type_expr ctx e2 (WithType.with_type r_get))) r_set p
732+
mk (TBinop(OpAssignOp op,ea,type_expr ctx e2 (WithType.with_type r_get))) r_set p *)
737733
| Some _,Some _ ->
738734
let ef_set = mk (TField(et,(FStatic(c,cf_set)))) tf_set p in
739735
let el = [make_call ctx ef_set [ebase;ekey;eget] r_set p] in
@@ -745,20 +741,89 @@ let type_assign_op ctx op e1 e2 with_type p =
745741
raise_typing_error "Invalid array access getter/setter combination" p
746742
in
747743
save();
748-
vr#to_texpr e
744+
api.generate vr eread e
749745
| AKResolve(sea,name) ->
750746
let e,vr = process_lhs_expr ctx "fh" sea.se_this in
751-
let t_lhs,r_rhs = field_rhs_by_name op name e WithType.value in
747+
let e_lhs,r_rhs = field_rhs_by_name name e WithType.value in
752748
let assign e_rhs =
753749
let e_name = Texpr.Builder.make_string ctx.t name null_pos in
754750
(new call_dispatcher ctx (MCall [e2]) with_type p)#field_call sea.se_access [sea.se_this;e_name;e_rhs] []
755751
in
756-
let e = BinopResult.to_texpr vr r_rhs assign in
757-
vr#to_texpr e
752+
let e = api.to_texpr vr r_rhs assign in
753+
api.generate vr e_lhs e
758754
in
759755
let with_type = with_type_or_value with_type in
760756
loop (!type_access_ref ctx (fst e1) (snd e1) (MSet (Some e2)) with_type)
761757

758+
let type_assign_op ctx op e1 e2 with_type p =
759+
let api = {
760+
akno_fallback = (fun () ->
761+
type_non_assign_op ctx op e1 e2 true true with_type p
762+
);
763+
type_rhs = (fun e_lhs e2 ->
764+
type_binop2 ctx op e_lhs e2 true WithType.value p
765+
);
766+
to_texpr = (fun vr br assign ->
767+
BinopResult.to_texpr vr br assign
768+
);
769+
generate = (fun vr e_lhs e ->
770+
vr#to_texpr e
771+
);
772+
assign = (fun vr e_lhs r_rhs ->
773+
let assign e_rhs =
774+
if BinopResult.needs_assign r_rhs then check_assign ctx e_lhs;
775+
let e_rhs = AbstractCast.cast_or_unify ctx e_lhs.etype e_rhs p in
776+
match e_rhs.eexpr with
777+
| TBinop(op',e1',e2') when op = op' && Texpr.equal e_lhs e1' ->
778+
mk (TBinop(OpAssignOp op',e1',e2')) e_lhs.etype p
779+
| _ ->
780+
mk (TBinop(OpAssign,e_lhs,e_rhs)) e_lhs.etype p
781+
in
782+
let e = BinopResult.to_texpr vr r_rhs assign in
783+
vr#to_texpr e
784+
)
785+
} in
786+
handle_assign_op ctx api e1 e2 with_type p
787+
788+
let type_op_null_coal_assign ctx e1 e2 with_type p =
789+
let hack = ref (fun e -> e) in
790+
let gen vr e1 t2 e_assign =
791+
let e1,eelse,tif = match with_type with
792+
| WithType.NoValue ->
793+
e1,None,ctx.t.tvoid
794+
| _ ->
795+
let e1 = vr#as_var "tmp" e1 in
796+
(* The t2 is here so that `anything ??= 2` doesn't become Null<T> *)
797+
e1,Some e1,t2
798+
in
799+
let e_null = Texpr.Builder.make_null e1.etype e1.epos in
800+
let e_null = Texpr.Builder.binop OpEq e1 e_null ctx.t.tbool e1.epos in
801+
let e = mk (TIf(e_null,e_assign,eelse)) tif e1.epos in
802+
vr#to_texpr e
803+
in
804+
let api = {
805+
akno_fallback = (fun () ->
806+
raise Not_found
807+
);
808+
type_rhs = (fun e_lhs e2 ->
809+
type_expr ctx e2 (WithType.WithType(e_lhs.etype,None))
810+
);
811+
to_texpr = (fun vr e assign ->
812+
hack := assign;
813+
e
814+
);
815+
generate = (fun vr e_lhs e ->
816+
gen vr e_lhs e.etype (!hack e)
817+
);
818+
assign = (fun vr e_lhs e_rhs ->
819+
let assign e_rhs =
820+
let e_rhs = AbstractCast.cast_or_unify ctx e_lhs.etype e_rhs p in
821+
mk (TBinop(OpAssign,e_lhs,e_rhs)) e_lhs.etype p
822+
in
823+
gen vr e_lhs e_rhs.etype (assign e_rhs)
824+
)
825+
} in
826+
handle_assign_op ctx api e1 e2 with_type p
762827

763828
let type_binop ctx op e1 e2 is_assign_op with_type p =
764829
match op with

‎src/typing/typer.ml

+1-3
Original file line numberDiff line numberDiff line change
@@ -1878,9 +1878,7 @@ and type_expr ?(mode=MGet) ctx (e,p) (with_type:WithType.t) =
18781878
let e_if = mk (TIf(e_cond,cast e1,Some e2)) iftype p in
18791879
vr#to_texpr e_if
18801880
| EBinop (OpAssignOp OpNullCoal,e1,e2) ->
1881-
let e_cond = EBinop(OpNotEq,e1,(EConst(Ident "null"), p)) in
1882-
let e_if = EIf ((e_cond, p),e1,Some e2) in
1883-
type_assign ctx e1 (e_if, p) with_type p
1881+
type_op_null_coal_assign ctx e1 e2 with_type p
18841882
| EBinop (op,e1,e2) ->
18851883
type_binop ctx op e1 e2 false with_type p
18861884
| EBlock [] when (match with_type with

‎src/typing/typerBase.ml

+1-1
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ let rec s_access_kind acc =
269269
| AKUsingField sea -> Printf.sprintf "AKUsingField(%s)" (s_static_extension_access sea)
270270
| AKUsingAccessor sea -> Printf.sprintf "AKUsingAccessor(%s)" (s_static_extension_access sea)
271271
| AKAccess(a,tl,c,e1,e2) -> Printf.sprintf "AKAccess(%s, [%s], %s, %s, %s)" (s_type_path a.a_path) (String.concat ", " (List.map st tl)) (s_type_path c.cl_path) (se e1) (se e2)
272-
| AKResolve(_) -> ""
272+
| AKResolve(sea,name) -> Printf.sprintf "AKResolve(%s, %s)" (s_static_extension_access sea) name
273273

274274
and s_safe_nav_access sn =
275275
let st = s_type (print_context()) in

‎tests/misc/projects/Issue10845/compile-fail.hxml.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Main.hx:21: characters 3-10 : Cannot modify abstract value of final field
22
Main.hx:22: characters 3-10 : Cannot modify abstract value of final local
3-
Main.hx:24: characters 3-8 : Cannot modify abstract value of final field
3+
Main.hx:24: characters 3-13 : Cannot modify abstract value of final field
44
Main.hx:25: characters 3-13 : Cannot modify abstract value of final local
55
Main.hx:29: characters 3-8 : This expression cannot be accessed for writing
66
Main.hx:30: characters 3-8 : Cannot assign to final

‎tests/optimization/src/issues/Issue11931.hx

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package issues;
2+
3+
class Issue11931 {
4+
@:js('
5+
var arr = [];
6+
var tmp = arr[0];
7+
issues_Issue11931.use(tmp == null ? arr[0] = [] : tmp);
8+
')
9+
static function test() {
10+
var arr = [];
11+
var e = arr[0] ??= [];
12+
use(e);
13+
}
14+
15+
@:pure(false)
16+
static function use(v:Array<Int>) {}
17+
}

0 commit comments

Comments
 (0)