Skip to content

Commit

Permalink
DCE cleanup 2024 (HaxeFoundation#11485)
Browse files Browse the repository at this point in the history
* remove m_if_feature, make DCE check the meta directly instead

also copy class @:ifFeature to field @:ifFeature

* add CfUsed, CfMaybeUsed, CUsed

Can't get rid of Meta.Used entirely yet because enums and abstracts don't have flags

* don't play silly meta merge games in the typeloader

instead play them in DCE because otherwise we won't deal with __init__ correctly

* update haxelib

it won't help
  • Loading branch information
Simn authored and 0b1kn00b committed Jan 25, 2024
1 parent 2558336 commit 2bce008
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 86 deletions.
6 changes: 0 additions & 6 deletions src-json/meta.json
Original file line number Diff line number Diff line change
Expand Up @@ -714,12 +714,6 @@
"metadata": ":macro",
"doc": "(deprecated)"
},
{
"name": "MaybeUsed",
"metadata": ":maybeUsed",
"doc": "Internally used by DCE to mark fields that might be kept.",
"internal": true
},
{
"name": "MergeBlock",
"metadata": ":mergeBlock",
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/genxml.ml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ let rec follow_param t =
t

let gen_meta meta =
let meta = List.filter (fun (m,_,_) -> match m with Meta.Used | Meta.MaybeUsed | Meta.RealPath | Meta.Pure -> false | _ -> true) meta in
let meta = List.filter (fun (m,_,_) -> match m with Meta.Used | Meta.RealPath | Meta.Pure -> false | _ -> true) meta in
match meta with
| [] -> []
| _ ->
Expand Down
10 changes: 8 additions & 2 deletions src/context/common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1033,9 +1033,15 @@ let rec has_feature com f =
(match List.find (fun t -> t_path t = path && not (Meta.has Meta.RealPath (t_infos t).mt_meta)) com.types with
| t when field = "*" ->
not (has_dce com) ||
(match t with TAbstractDecl a -> Meta.has Meta.ValueUsed a.a_meta | _ -> Meta.has Meta.Used (t_infos t).mt_meta)
begin match t with
| TClassDecl c ->
has_class_flag c CUsed;
| TAbstractDecl a ->
Meta.has Meta.ValueUsed a.a_meta
| _ -> Meta.has Meta.Used (t_infos t).mt_meta
end;
| TClassDecl c when (has_class_flag c CExtern) && (com.platform <> Js || cl <> "Array" && cl <> "Math") ->
not (has_dce com) || Meta.has Meta.Used (try PMap.find field c.cl_statics with Not_found -> PMap.find field c.cl_fields).cf_meta
not (has_dce com) || has_class_field_flag (try PMap.find field c.cl_statics with Not_found -> PMap.find field c.cl_fields) CfUsed
| TClassDecl c ->
PMap.exists field c.cl_statics || PMap.exists field c.cl_fields
| _ ->
Expand Down
11 changes: 0 additions & 11 deletions src/context/feature.ml

This file was deleted.

1 change: 0 additions & 1 deletion src/core/tFunctions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ let module_extra file sign time kind added policy =
m_deps = PMap.empty;
m_kind = kind;
m_cache_bound_objects = DynArray.create ();
m_if_feature = [];
m_features = Hashtbl.create 0;
m_check_policy = policy;
}
Expand Down
1 change: 0 additions & 1 deletion src/core/tPrinting.ml
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,6 @@ module Printer = struct
"m_processed",string_of_int me.m_processed;
"m_kind",s_module_kind me.m_kind;
"m_binded_res",""; (* TODO *)
"m_if_feature",""; (* TODO *)
"m_features",""; (* TODO *)
]

Expand Down
6 changes: 4 additions & 2 deletions src/core/tType.ml
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ and module_def_extra = {
mutable m_deps : (int,(Digest.t (* sign *) * path)) PMap.t;
mutable m_kind : module_kind;
mutable m_cache_bound_objects : cache_bound_object DynArray.t;
mutable m_if_feature : (string * class_field_ref) list;
mutable m_features : (string,bool) Hashtbl.t;
}

Expand Down Expand Up @@ -464,6 +463,7 @@ type flag_tclass =
| CInterface
| CAbstract
| CFunctionalInterface
| CUsed (* Marker for DCE *)

type flag_tclass_field =
| CfPublic
Expand All @@ -479,10 +479,12 @@ type flag_tclass_field =
| CfGeneric
| CfDefault (* Interface field with default implementation (only valid on Java) *)
| CfPostProcessed (* Marker to indicate the field has been post-processed *)
| CfUsed (* Marker for DCE *)
| CfMaybeUsed (* Marker for DCE *)

(* Order has to match declaration for printing*)
let flag_tclass_field_names = [
"CfPublic";"CfStatic";"CfExtern";"CfFinal";"CfModifiesThis";"CfOverride";"CfAbstract";"CfOverload";"CfImpl";"CfEnum";"CfGeneric";"CfDefault";"CfPostProcessed"
"CfPublic";"CfStatic";"CfExtern";"CfFinal";"CfModifiesThis";"CfOverride";"CfAbstract";"CfOverload";"CfImpl";"CfEnum";"CfGeneric";"CfDefault";"CfPostProcessed";"CfUsed";"CfMaybeUsed"
]

type flag_tvar =
Expand Down
10 changes: 4 additions & 6 deletions src/core/tUnification.ml
Original file line number Diff line number Diff line change
Expand Up @@ -788,15 +788,14 @@ let rec unify (uctx : unification_context) a b =
then error [Missing_overload (f1, f2o.cf_type)]
) f2.cf_overloads;
(* we mark the field as :?used because it might be used through the structure *)
if not (Meta.has Meta.MaybeUsed f1.cf_meta) then begin
f1.cf_meta <- (Meta.MaybeUsed,[],f1.cf_pos) :: f1.cf_meta;
if not (has_class_field_flag f1 CfMaybeUsed) then begin
add_class_field_flag f1 CfMaybeUsed;
match f2.cf_kind with
| Var vk ->
let check name =
try
let _,_,cf = raw_class_field make_type c tl name in
if not (Meta.has Meta.MaybeUsed cf.cf_meta) then
cf.cf_meta <- (Meta.MaybeUsed,[],f1.cf_pos) :: cf.cf_meta
add_class_field_flag cf CfMaybeUsed
with Not_found ->
()
in
Expand Down Expand Up @@ -951,8 +950,7 @@ and unify_anons uctx a b a1 a2 =
)
| ClassStatics c1,_ ->
unify_fields c1.cl_statics (fun f1 ->
if not (Meta.has Meta.MaybeUsed f1.cf_meta) then
f1.cf_meta <- (Meta.MaybeUsed,[],f1.cf_pos) :: f1.cf_meta
add_class_field_flag f1 CfMaybeUsed
) (fun _ -> false)
| _ ->
unify_fields a1.a_fields (fun _ -> ()) (fun _ -> false)
Expand Down
2 changes: 1 addition & 1 deletion src/generators/genhxold.ml
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ let generate_type com t =
let print_meta ml =
List.iter (fun (m,pl,_) ->
match m with
| Meta.DefParam | Meta.CoreApi | Meta.Used | Meta.MaybeUsed | Meta.FlatEnum | Meta.Value | Meta.DirectlyUsed | Meta.Enum -> ()
| Meta.DefParam | Meta.CoreApi | Meta.Used | Meta.FlatEnum | Meta.Value | Meta.DirectlyUsed | Meta.Enum -> ()
| _ ->
match pl with
| [] -> p "@%s " (Meta.to_string m)
Expand Down
86 changes: 56 additions & 30 deletions src/optimization/dce.ml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type dce = {
mutable marked_maybe_fields : tclass_field list;
mutable t_stack : t list;
mutable ts_stack : t list;
mutable features : (string, class_field_ref list) Hashtbl.t;
mutable features : (string, class_field_ref list ref) Hashtbl.t;
}

let push_class dce c =
Expand Down Expand Up @@ -121,7 +121,8 @@ let mk_keep_meta pos =
*)
let rec keep_field dce cf c kind =
let is_static = kind = CfrStatic in
Meta.has_one_of (Meta.Used :: keep_metas) cf.cf_meta
Meta.has_one_of keep_metas cf.cf_meta
|| has_class_field_flag cf CfUsed
|| cf.cf_name = "__init__"
|| has_class_field_flag cf CfExtern
|| (not is_static && overrides_extern_field cf c)
Expand Down Expand Up @@ -153,7 +154,7 @@ let rec check_feature dce s =
List.iter (fun cfr ->
let (c, cf) = resolve_class_field_ref dce.com cfr in
mark_field dce c cf cfr.cfr_kind
) l;
) !l;
Hashtbl.remove dce.features s;
with Not_found ->
()
Expand All @@ -166,8 +167,8 @@ and check_and_add_feature dce s =
(* mark a field as kept *)
and mark_field dce c cf kind =
let add c' cf =
if not (Meta.has Meta.Used cf.cf_meta) then begin
cf.cf_meta <- (mk_used_meta cf.cf_pos) :: cf.cf_meta;
if not (has_class_field_flag cf CfUsed) then begin
add_class_field_flag cf CfUsed;
dce.added_fields <- (c',cf,kind) :: dce.added_fields;
dce.marked_fields <- cf :: dce.marked_fields;
check_feature dce (Printf.sprintf "%s.%s" (s_type_path c.cl_path) cf.cf_name);
Expand Down Expand Up @@ -202,10 +203,10 @@ let rec update_marked_class_fields dce c =
let pop = push_class dce c in
(* mark all :?used fields as surely :used now *)
List.iter (fun cf ->
if Meta.has Meta.MaybeUsed cf.cf_meta then mark_field dce c cf CfrStatic
if has_class_field_flag cf CfMaybeUsed then mark_field dce c cf CfrStatic
) c.cl_ordered_statics;
List.iter (fun cf ->
if Meta.has Meta.MaybeUsed cf.cf_meta then mark_field dce c cf CfrMember
if has_class_field_flag cf CfMaybeUsed then mark_field dce c cf CfrMember
) c.cl_ordered_fields;
(* we always have to keep super classes and implemented interfaces *)
(match c.cl_init with None -> () | Some init -> dce.follow_expr dce init);
Expand All @@ -214,8 +215,8 @@ let rec update_marked_class_fields dce c =
pop()

(* mark a class as kept. If the class has fields marked as @:?keep, make sure to keep them *)
and mark_class dce c = if not (Meta.has Meta.Used c.cl_meta) then begin
c.cl_meta <- (mk_used_meta c.cl_pos) :: c.cl_meta;
and mark_class dce c = if not (has_class_flag c CUsed) then begin
add_class_flag c CUsed;
check_feature dce (Printf.sprintf "%s.*" (s_type_path c.cl_path));
update_marked_class_fields dce c;
end
Expand All @@ -238,8 +239,8 @@ and mark_t dce p t =
dce.t_stack <- t :: dce.t_stack;
begin match follow t with
| TInst({cl_kind = KTypeParameter ttp} as c,pl) ->
if not (Meta.has Meta.Used c.cl_meta) then begin
c.cl_meta <- (mk_used_meta c.cl_pos) :: c.cl_meta;
if not (has_class_flag c CUsed) then begin
add_class_flag c CUsed;
List.iter (mark_t dce p) (get_constraints ttp);
end;
List.iter (mark_t dce p) pl
Expand Down Expand Up @@ -288,10 +289,10 @@ let mark_dependent_fields dce csup n kind =
let cf = PMap.find n (if stat then c.cl_statics else c.cl_fields) in
(* if it's clear that the class is kept, the field has to be kept as well. This is also true for
extern interfaces because we cannot remove fields from them *)
if Meta.has Meta.Used c.cl_meta || ((has_class_flag csup CInterface) && (has_class_flag csup CExtern)) then mark_field dce c cf kind
if has_class_flag c CUsed || ((has_class_flag csup CInterface) && (has_class_flag csup CExtern)) then mark_field dce c cf kind
(* otherwise it might be kept if the class is kept later, so mark it as :?used *)
else if not (Meta.has Meta.MaybeUsed cf.cf_meta) then begin
cf.cf_meta <- (Meta.MaybeUsed,[],cf.cf_pos) :: cf.cf_meta;
else if not (has_class_field_flag cf CfMaybeUsed) then begin
add_class_field_flag cf CfMaybeUsed;
dce.marked_maybe_fields <- cf :: dce.marked_maybe_fields;
end
with Not_found ->
Expand Down Expand Up @@ -685,7 +686,7 @@ and expr dce e =
let fix_accessors com =
List.iter (fun mt -> match mt with
(* filter empty abstract implementation classes (issue #1885). *)
| TClassDecl({cl_kind = KAbstractImpl _} as c) when c.cl_ordered_statics = [] && c.cl_ordered_fields = [] && not (Meta.has Meta.Used c.cl_meta) ->
| TClassDecl({cl_kind = KAbstractImpl _} as c) when c.cl_ordered_statics = [] && c.cl_ordered_fields = [] && not (has_class_flag c CUsed) ->
add_class_flag c CExtern;
| TClassDecl({cl_kind = KAbstractImpl a} as c) when a.a_enum ->
let is_runtime_field cf =
Expand Down Expand Up @@ -716,16 +717,44 @@ let fix_accessors com =
| _ -> ()
) com.types

let extract_if_feature meta =
let rec loop = function
| [] ->
[]
| (Meta.IfFeature,el,_) :: _ ->
List.map (fun (e,p) -> match e with
| EConst (String(s,_)) -> s
| _ -> Error.raise_typing_error "String expected" p
) el
| _ :: l ->
loop l
in
loop meta

let collect_entry_points dce com =
let delayed = ref [] in
let check_feature cf_ref meta =
List.iter (fun s ->
try
let l = Hashtbl.find dce.features s in
l := cf_ref :: !l
with Not_found ->
Hashtbl.add dce.features s (ref [cf_ref])
) meta;
in
List.iter (fun t ->
let mt = t_infos t in
mt.mt_meta <- Meta.remove Meta.Used mt.mt_meta;
match t with
| TClassDecl c ->
remove_class_flag c CUsed;
let cl_if_feature = extract_if_feature c.cl_meta in
let keep_class = keep_whole_class dce c && (not (has_class_flag c CExtern) || (has_class_flag c CInterface)) in
let is_struct = dce.com.platform = Hl && Meta.has Meta.Struct c.cl_meta in
let loop kind cf =
if keep_class || is_struct || keep_field dce cf c kind then mark_field dce c cf kind
let cf_ref = mk_class_field_ref c cf kind com.is_macro_context in
let cf_if_feature = extract_if_feature cf.cf_meta in
check_feature cf_ref (cl_if_feature @ cf_if_feature);
(* Have to delay mark_field so that we see all @:ifFeature *)
if keep_class || is_struct || keep_field dce cf c kind then delayed := (fun () -> mark_field dce c cf kind) :: !delayed
in
List.iter (loop CfrStatic) c.cl_ordered_statics;
List.iter (loop CfrMember) c.cl_ordered_fields;
Expand All @@ -743,12 +772,16 @@ let collect_entry_points dce com =
()
end;
| TEnumDecl en when keep_whole_enum dce en ->
let pop = push_class dce {null_class with cl_module = en.e_module} in
mark_enum dce en;
pop()
en.e_meta <- Meta.remove Meta.Used en.e_meta;
delayed := (fun () ->
let pop = push_class dce {null_class with cl_module = en.e_module} in
mark_enum dce en;
pop()
) :: !delayed;
| _ ->
()
) com.types;
List.iter (fun f -> f()) !delayed;
if dce.debug then begin
List.iter (fun (c,cf,_) -> match cf.cf_expr with
| None -> ()
Expand Down Expand Up @@ -840,7 +873,7 @@ let sweep dce com =
let inef cf = is_physical_field cf in
let has_non_extern_fields = List.exists inef c.cl_ordered_fields || List.exists inef c.cl_ordered_statics in
(* we keep a class if it was used or has a used field *)
if Meta.has Meta.Used c.cl_meta || has_non_extern_fields then loop (mt :: acc) l else begin
if has_class_flag c CUsed || has_non_extern_fields then loop (mt :: acc) l else begin
(match c.cl_init with
| Some f when Meta.has Meta.KeepInit c.cl_meta ->
(* it means that we only need the __init__ block *)
Expand Down Expand Up @@ -879,12 +912,6 @@ let run com main mode =
features = Hashtbl.create 0;
curclass = null_class;
} in
List.iter (fun m ->
List.iter (fun (s,v) ->
if Hashtbl.mem dce.features s then Hashtbl.replace dce.features s (v :: Hashtbl.find dce.features s)
else Hashtbl.add dce.features s [v]
) m.m_extra.m_if_feature;
) com.modules;

(* first step: get all entry points, which is the main method and all class methods which are marked with @:keep *)
collect_entry_points dce com;
Expand Down Expand Up @@ -928,5 +955,4 @@ let run com main mode =
) com.types;

(* cleanup added fields metadata - compatibility with compilation server *)
List.iter (fun cf -> cf.cf_meta <- Meta.remove Meta.Used cf.cf_meta) dce.marked_fields;
List.iter (fun cf -> cf.cf_meta <- Meta.remove Meta.MaybeUsed cf.cf_meta) dce.marked_maybe_fields
List.iter (fun cf -> remove_class_field_flag cf CfUsed) dce.marked_fields
11 changes: 0 additions & 11 deletions src/typing/typeloadFields.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1781,7 +1781,6 @@ let init_class ctx c p herits fields =
| _ :: l ->
check_require l
in
let cl_if_feature = Feature.check_if_feature c.cl_meta in
let cl_req = check_require c.cl_meta in
let has_init = ref false in
List.iter (fun f ->
Expand All @@ -1801,16 +1800,6 @@ let init_class ctx c p herits fields =
if fctx.is_field_debug then print_endline ("Created field: " ^ Printer.s_tclass_field "" cf);
if fctx.is_static && (has_class_flag c CInterface) && fctx.field_kind <> FKInit && not cctx.is_lib && not ((has_class_flag c CExtern)) then
raise_typing_error "You can only declare static fields in extern interfaces" p;
let set_feature s =
let ref_kind = match fctx.field_kind with
| FKConstructor -> CfrConstructor
| _ -> if fctx.is_static then CfrStatic else CfrMember
in
let cf_ref = mk_class_field_ref c cf ref_kind fctx.is_macro in
Feature.set_feature ctx.m.curmod cf_ref s;
in
List.iter set_feature cl_if_feature;
List.iter set_feature (Feature.check_if_feature cf.cf_meta);
let req = check_require f.cff_meta in
let req = (match req with None -> if fctx.is_static || fctx.field_kind = FKConstructor then cl_req else None | _ -> req) in
(match req with
Expand Down
28 changes: 14 additions & 14 deletions tests/unit/src/unit/issues/Issue8502.hx
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
package unit.issues;

class Issue8502 extends Test {
#if cpp
public function test() {
#if cpp
public function test() {
var t:scripthost.Issue8502 = Type.createInstance(Type.resolveClass('unit.issues.Issue8502_2'), []);
eq(t.doTest1(25), 'cppia 25');
eq(t.doTest2(25), 'cppia 25');
eq(t.doTest3(25), 'cppia 25');
eq(t.doTest4(25), 'cppia 25');
eq(t.doTest5(25), 'cppia 25');
eq(t.doTest3u(25), 'cppia 25');
eq(t.doTest4u(25), 'cppia 25');
eq(t.doTest5u(25), 'cppia 25');
}
#end
eq(t.doTest1(25), 'cppia 25');
eq(t.doTest2(25), 'cppia 25');
eq(t.doTest3(25), 'cppia 25');
eq(t.doTest4(25), 'cppia 25');
eq(t.doTest5(25), 'cppia 25');
eq(t.doTest3u(25), 'cppia 25');
eq(t.doTest4u(25), 'cppia 25');
eq(t.doTest5u(25), 'cppia 25');
}
#end
}

#if cpp
@:keep
class Issue8502_2 extends scripthost.Issue8502 {
override public function doTest1(f:cpp.Float32):String {
return 'cppia ' + super.doTest1(f);
Expand Down Expand Up @@ -50,5 +51,4 @@ class Issue8502_2 extends scripthost.Issue8502 {
return 'cppia ' + super.doTest5u(f);
}
}

#end
#end

0 comments on commit 2bce008

Please sign in to comment.