From 2bce008736b3ebb864f077fba91a55b39b57cd19 Mon Sep 17 00:00:00 2001 From: Simon Krajewski Date: Fri, 12 Jan 2024 08:26:33 +0100 Subject: [PATCH] DCE cleanup 2024 (#11485) * 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 --- src-json/meta.json | 6 -- src/codegen/genxml.ml | 2 +- src/context/common.ml | 10 ++- src/context/feature.ml | 11 ---- src/core/tFunctions.ml | 1 - src/core/tPrinting.ml | 1 - src/core/tType.ml | 6 +- src/core/tUnification.ml | 10 ++- src/generators/genhxold.ml | 2 +- src/optimization/dce.ml | 86 ++++++++++++++++--------- src/typing/typeloadFields.ml | 11 ---- tests/unit/src/unit/issues/Issue8502.hx | 28 ++++---- 12 files changed, 88 insertions(+), 86 deletions(-) delete mode 100644 src/context/feature.ml diff --git a/src-json/meta.json b/src-json/meta.json index 672b94a795a..f2bfefec68e 100644 --- a/src-json/meta.json +++ b/src-json/meta.json @@ -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", diff --git a/src/codegen/genxml.ml b/src/codegen/genxml.ml index 64c19c1ef25..6b737f484f3 100644 --- a/src/codegen/genxml.ml +++ b/src/codegen/genxml.ml @@ -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 | [] -> [] | _ -> diff --git a/src/context/common.ml b/src/context/common.ml index adeb921624b..e2e99c8d9f5 100644 --- a/src/context/common.ml +++ b/src/context/common.ml @@ -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 | _ -> diff --git a/src/context/feature.ml b/src/context/feature.ml deleted file mode 100644 index 647d03da10b..00000000000 --- a/src/context/feature.ml +++ /dev/null @@ -1,11 +0,0 @@ -open Ast -open Type -open Error - -let rec check_if_feature = function - | [] -> [] - | (Meta.IfFeature,el,_) :: _ -> List.map (fun (e,p) -> match e with EConst (String(s,_)) -> s | _ -> raise_typing_error "String expected" p) el - | _ :: l -> check_if_feature l - -let set_feature m cf_ref s = - m.m_extra.m_if_feature <- (s, cf_ref) :: m.m_extra.m_if_feature diff --git a/src/core/tFunctions.ml b/src/core/tFunctions.ml index ad0cb531bcc..116c0f9f333 100644 --- a/src/core/tFunctions.ml +++ b/src/core/tFunctions.ml @@ -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; } diff --git a/src/core/tPrinting.ml b/src/core/tPrinting.ml index 5ac61c1d827..4b83743db41 100644 --- a/src/core/tPrinting.ml +++ b/src/core/tPrinting.ml @@ -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 *) ] diff --git a/src/core/tType.ml b/src/core/tType.ml index 8d89f77950a..fd679655f8f 100644 --- a/src/core/tType.ml +++ b/src/core/tType.ml @@ -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; } @@ -464,6 +463,7 @@ type flag_tclass = | CInterface | CAbstract | CFunctionalInterface + | CUsed (* Marker for DCE *) type flag_tclass_field = | CfPublic @@ -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 = diff --git a/src/core/tUnification.ml b/src/core/tUnification.ml index 055c1ecf562..25b7a5fa280 100644 --- a/src/core/tUnification.ml +++ b/src/core/tUnification.ml @@ -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 @@ -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) diff --git a/src/generators/genhxold.ml b/src/generators/genhxold.ml index df7667fb2dd..5f8660319f6 100644 --- a/src/generators/genhxold.ml +++ b/src/generators/genhxold.ml @@ -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) diff --git a/src/optimization/dce.ml b/src/optimization/dce.ml index d9a0b410116..85ffc7a546f 100644 --- a/src/optimization/dce.ml +++ b/src/optimization/dce.ml @@ -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 = @@ -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) @@ -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 -> () @@ -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); @@ -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); @@ -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 @@ -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 @@ -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 -> @@ -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 = @@ -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; @@ -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 -> () @@ -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 *) @@ -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; @@ -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 diff --git a/src/typing/typeloadFields.ml b/src/typing/typeloadFields.ml index fbc8a656ac3..50b63f1bf32 100644 --- a/src/typing/typeloadFields.ml +++ b/src/typing/typeloadFields.ml @@ -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 -> @@ -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 diff --git a/tests/unit/src/unit/issues/Issue8502.hx b/tests/unit/src/unit/issues/Issue8502.hx index 495cde258db..140e1a31eb2 100644 --- a/tests/unit/src/unit/issues/Issue8502.hx +++ b/tests/unit/src/unit/issues/Issue8502.hx @@ -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); @@ -50,5 +51,4 @@ class Issue8502_2 extends scripthost.Issue8502 { return 'cppia ' + super.doTest5u(f); } } - -#end \ No newline at end of file +#end