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

extra/haxelib_src

@@ -1 +1 @@
-Subproject commit 70ff6b69a5b35049d767056555c0bf7a54e8ad4e
+Subproject commit 98637027327d8cf385d302acaaf104bd6107d2bf

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",

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
 	| [] -> []
 	| _ ->

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
 				| _ ->

src/context/feature.ml

@@ -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

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;
 	}

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 *)
 		]
 

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 =

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)

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)

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

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

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
+#end