Add CfPostProcessed flag (#10643)

* [server] add CfPostProcessed, unset m_processed when adding a dependency

see #10635

* hello?

* make sure add_field_inits still runs the analyzer

* run insert_save_stacks after DCE again

* don't copy super constructor fields to avoid inheriting CfProcessed flag

* maybe it needs an explicit Invalidate

* [generic] rename to build_generic_class

* [generic] remove CfPostProcessed from generic copies too

src/compiler/server.ml

@@ -211,8 +211,7 @@ module Communication = struct
 				if has_error ctx then begin
 					measure_times := false;
 					write "\x02\n"
-				end else
-					maybe_cache_context sctx ctx.com;
+				end
 			)
 		);
 		exit = (fun i ->

src/context/display/displayJson.ml

@@ -204,6 +204,10 @@ let handler =
 				| [] ->
 					hctx.send_error [jstring "No such type"]
 				| mt :: mtl ->
+					begin match mt with
+					| TClassDecl c -> c.cl_restore()
+					| _ -> ()
+					end;
 					let infos = t_infos mt in
 					if snd infos.mt_path = typeName then begin
 						let ctx = Genjson.create_context GMMinimum in

src/core/tFunctions.ml

@@ -231,7 +231,11 @@ let null_abstract = {
 }
 
 let add_dependency m mdep =
-	if m != null_module && m != mdep then m.m_extra.m_deps <- PMap.add mdep.m_id mdep m.m_extra.m_deps
+	if m != null_module && m != mdep then begin
+		m.m_extra.m_deps <- PMap.add mdep.m_id mdep m.m_extra.m_deps;
+		(* In case the module is cached, we'll have to run post-processing on it again (issue #10635) *)
+		m.m_extra.m_processed <- 0
+	end
 
 let arg_name (a,_) = a.v_name
 

src/core/tType.ml

@@ -420,6 +420,7 @@ type flag_tclass_field =
 	| CfEnum
 	| CfGeneric
 	| CfDefault (* Interface field with default implementation (only valid on Java) *)
+	| CfPostProcessed (* Marker to indicate the field has been post-processed *)
 
 (* Order has to match declaration for printing*)
 let flag_tclass_field_names = [

src/filters/defaultArguments.ml

@@ -61,6 +61,8 @@ let rec change_func com cl cf =
 	List.iter (change_func com cl) cf.cf_overloads;
 
 	match cf.cf_kind, follow cf.cf_type with
+	| _ when has_class_field_flag cf CfPostProcessed ->
+		()
 	| Var _, _ | Method MethDynamic, _ ->
 		()
 	| _, TFun(args, ret) ->

src/filters/filters.ml

@@ -345,67 +345,6 @@ let check_abstract_as_value e =
 
 (* PASS 1 end *)
 
-(* Saves a class state so it can be restored later, e.g. after DCE or native path rewrite *)
-let save_class_state ctx t = match t with
-	| TClassDecl c ->
-		let vars = ref [] in
-		let rec save_vars e =
-			let add v = vars := (v, v.v_type) :: !vars in
-			match e.eexpr with
-				| TFunction fn ->
-					List.iter (fun (v, _) -> add v) fn.tf_args;
-					save_vars fn.tf_expr
-				| TVar (v, e) ->
-					add v;
-					Option.may save_vars e
-				| _ ->
-					iter save_vars e
-		in
-		let mk_field_restore f =
-			Option.may save_vars f.cf_expr;
-			let rec mk_overload_restore f =
-				f.cf_name,f.cf_kind,f.cf_expr,f.cf_type,f.cf_meta,f.cf_params
-			in
-			( f,mk_overload_restore f, List.map (fun f -> f,mk_overload_restore f) f.cf_overloads )
-		in
-		let restore_field (f,res,overloads) =
-			let restore_field (f,(name,kind,expr,t,meta,params)) =
-				f.cf_name <- name; f.cf_kind <- kind; f.cf_expr <- expr; f.cf_type <- t; f.cf_meta <- meta; f.cf_params <- params;
-				f
-			in
-			let f = restore_field (f,res) in
-			f.cf_overloads <- List.map restore_field overloads;
-			f
-		in
-		let mk_pmap lst =
-			List.fold_left (fun pmap f -> PMap.add f.cf_name f pmap) PMap.empty lst
-		in
-
-		let meta = c.cl_meta and path = c.cl_path and ext = (has_class_flag c CExtern) in
-		let sup = c.cl_super and impl = c.cl_implements in
-		let csr = Option.map (mk_field_restore) c.cl_constructor in
-		let ofr = List.map (mk_field_restore) c.cl_ordered_fields in
-		let osr = List.map (mk_field_restore) c.cl_ordered_statics in
-		let init = c.cl_init in
-		Option.may save_vars init;
-		c.cl_restore <- (fun() ->
-			c.cl_super <- sup;
-			c.cl_implements <- impl;
-			c.cl_meta <- meta;
-			if ext then add_class_flag c CExtern else remove_class_flag c CExtern;
-			c.cl_path <- path;
-			c.cl_init <- init;
-			c.cl_ordered_fields <- List.map restore_field ofr;
-			c.cl_ordered_statics <- List.map restore_field osr;
-			c.cl_fields <- mk_pmap c.cl_ordered_fields;
-			c.cl_statics <- mk_pmap c.cl_ordered_statics;
-			c.cl_constructor <- Option.map restore_field csr;
-			c.cl_descendants <- [];
-			List.iter (fun (v, t) -> v.v_type <- t) !vars;
-		)
-	| _ ->
-		()
-
 (* PASS 2 begin *)
 
 let remove_generic_base t = match t with
@@ -571,7 +510,9 @@ let add_field_inits cl_path locals com t =
 					die "" __LOC__
 			in
 			let config = AnalyzerConfig.get_field_config com c cf in
+			remove_class_field_flag cf CfPostProcessed;
 			Analyzer.Run.run_on_field com config c cf;
+			add_class_field_flag cf CfPostProcessed;
 			(match cf.cf_expr with
 			| Some e ->
 				(* This seems a bit expensive, but hopefully constructor expressions aren't that massive. *)
@@ -625,7 +566,7 @@ let check_cs_events com t = match t with
 	| TClassDecl cl when not (has_class_flag cl CExtern) ->
 		let check fields f =
 			match f.cf_kind with
-			| Var { v_read = AccNormal; v_write = AccNormal } when Meta.has Meta.Event f.cf_meta ->
+			| Var { v_read = AccNormal; v_write = AccNormal } when Meta.has Meta.Event f.cf_meta && not (has_class_field_flag f CfPostProcessed) ->
 				if (has_class_field_flag f CfPublic) then typing_error "@:event fields must be private" f.cf_pos;
 
 				(* prevent generating reflect helpers for the event in gencommon *)
@@ -765,6 +706,133 @@ module ForRemap = struct
 		loop e
 end
 
+let destruction tctx detail_times main locals =
+	let com = tctx.com in
+	let t = filter_timer detail_times ["type 2"] in
+	(* PASS 2: type filters pre-DCE *)
+	List.iter (fun t ->
+		remove_generic_base t;
+		remove_extern_fields com t;
+		Codegen.update_cache_dependencies t;
+		(* check @:remove metadata before DCE so it is ignored there (issue #2923) *)
+		check_remove_metadata t;
+	) com.types;
+	t();
+	com.stage <- CDceStart;
+	let t = filter_timer detail_times ["dce"] in
+	(* DCE *)
+	let dce_mode = try Common.defined_value com Define.Dce with _ -> "no" in
+	let dce_mode = match dce_mode with
+		| "full" -> if Common.defined com Define.Interp then Dce.DceNo else DceFull
+		| "std" -> DceStd
+		| "no" -> DceNo
+		| _ -> failwith ("Unknown DCE mode " ^ dce_mode)
+	in
+	Dce.run com main dce_mode;
+	t();
+	com.stage <- CDceDone;
+	(* PASS 3: type filters post-DCE *)
+	List.iter
+		(run_expression_filters
+			~ignore_processed_status:true
+			(timer_label detail_times [])
+			tctx
+			(* This has to run after DCE, or otherwise its condition always holds. *)
+			["insert_save_stacks",Exceptions.insert_save_stacks tctx]
+		)
+		com.types;
+	let type_filters = [
+		Exceptions.patch_constructors tctx; (* TODO: I don't believe this should load_instance anything at this point... *)
+		check_private_path tctx;
+		apply_native_paths;
+		add_rtti com;
+		(match com.platform with | Java | Cs -> (fun _ -> ()) | _ -> (fun mt -> add_field_inits tctx.curclass.cl_path locals com mt));
+		(match com.platform with Hl -> (fun _ -> ()) | _ -> add_meta_field com);
+		check_void_field;
+		(match com.platform with | Cpp -> promote_first_interface_to_super | _ -> (fun _ -> ()));
+		commit_features com;
+		(if com.config.pf_reserved_type_paths <> [] then check_reserved_type_paths tctx else (fun _ -> ()));
+	] in
+	let type_filters = match com.platform with
+		| Cs -> type_filters @ [ fun t -> InterfaceProps.run t ]
+		| _ -> type_filters
+	in
+	let t = filter_timer detail_times ["type 3"] in
+	List.iter (fun t ->
+		begin match t with
+		| TClassDecl c ->
+			tctx.curclass <- c
+		| _ ->
+			()
+		end;
+		List.iter (fun f -> f t) type_filters
+	) com.types;
+	t();
+	List.iter (fun f -> f()) (List.rev com.callbacks#get_after_filters);
+	com.stage <- CFilteringDone
+
+(* Saves a class state so it can be restored later, e.g. after DCE or native path rewrite *)
+let save_class_state ctx t = match t with
+| TClassDecl c ->
+	let vars = ref [] in
+	let rec save_vars e =
+		let add v = vars := (v, v.v_type) :: !vars in
+		match e.eexpr with
+			| TFunction fn ->
+				List.iter (fun (v, _) -> add v) fn.tf_args;
+				save_vars fn.tf_expr
+			| TVar (v, e) ->
+				add v;
+				Option.may save_vars e
+			| _ ->
+				iter save_vars e
+	in
+	let mk_field_restore f =
+		Option.may save_vars f.cf_expr;
+		let rec mk_overload_restore f =
+			add_class_field_flag f CfPostProcessed;
+			f.cf_name,f.cf_kind,f.cf_expr,f.cf_type,f.cf_meta,f.cf_params
+		in
+		( f,mk_overload_restore f, List.map (fun f -> f,mk_overload_restore f) f.cf_overloads )
+	in
+	let restore_field (f,res,overloads) =
+		let restore_field (f,(name,kind,expr,t,meta,params)) =
+			f.cf_name <- name; f.cf_kind <- kind; f.cf_expr <- expr; f.cf_type <- t; f.cf_meta <- meta; f.cf_params <- params;
+			f
+		in
+		let f = restore_field (f,res) in
+		f.cf_overloads <- List.map restore_field overloads;
+		f
+	in
+	let mk_pmap lst =
+		List.fold_left (fun pmap f -> PMap.add f.cf_name f pmap) PMap.empty lst
+	in
+
+	let meta = c.cl_meta and path = c.cl_path and ext = (has_class_flag c CExtern) in
+	let sup = c.cl_super and impl = c.cl_implements in
+	let csr = Option.map (mk_field_restore) c.cl_constructor in
+	let ofr = List.map (mk_field_restore) c.cl_ordered_fields in
+	let osr = List.map (mk_field_restore) c.cl_ordered_statics in
+	let init = c.cl_init in
+	Option.may save_vars init;
+	c.cl_restore <- (fun() ->
+		c.cl_super <- sup;
+		c.cl_implements <- impl;
+		c.cl_meta <- meta;
+		if ext then add_class_flag c CExtern else remove_class_flag c CExtern;
+		c.cl_path <- path;
+		c.cl_init <- init;
+		c.cl_ordered_fields <- List.map restore_field ofr;
+		c.cl_ordered_statics <- List.map restore_field osr;
+		c.cl_fields <- mk_pmap c.cl_ordered_fields;
+		c.cl_statics <- mk_pmap c.cl_ordered_statics;
+		c.cl_constructor <- Option.map restore_field csr;
+		c.cl_descendants <- [];
+		List.iter (fun (v, t) -> v.v_type <- t) !vars;
+	)
+| _ ->
+	()
+
 let run com tctx main =
 	let detail_times = Common.defined com DefineList.FilterTimes in
 	let new_types = List.filter (fun t ->
@@ -790,6 +858,15 @@ let run com tctx main =
 		end;
 		not cached
 	) com.types in
+	(* IMPORTANT:
+	    There may be types in new_types which have already been post-processed, but then had their m_processed flag unset
+		because they received an additional dependency. This could happen in cases such as @:generic methods in #10635.
+		It is important that all filters from here up to save_class_state only process fields which do not have the
+		CfPostProcessed flag set.
+
+		This is mostly covered by run_expression_filters already, but any new additions which don't utilize that have to
+		be aware of this.
+	*)
 	NullSafety.run com new_types;
 	(* PASS 1: general expression filters *)
 	let filters = [
@@ -862,63 +939,4 @@ let run com tctx main =
 	let t = filter_timer detail_times ["callbacks"] in
 	List.iter (fun f -> f()) (List.rev com.callbacks#get_after_save); (* macros onGenerate etc. *)
 	t();
-	let t = filter_timer detail_times ["type 2"] in
-	(* PASS 2: type filters pre-DCE *)
-	List.iter (fun t ->
-		remove_generic_base t;
-		remove_extern_fields com t;
-		Codegen.update_cache_dependencies t;
-		(* check @:remove metadata before DCE so it is ignored there (issue #2923) *)
-		check_remove_metadata t;
-	) com.types;
-	t();
-	com.stage <- CDceStart;
-	let t = filter_timer detail_times ["dce"] in
-	(* DCE *)
-	let dce_mode = try Common.defined_value com Define.Dce with _ -> "no" in
-	let dce_mode = match dce_mode with
-		| "full" -> if Common.defined com Define.Interp then Dce.DceNo else DceFull
-		| "std" -> DceStd
-		| "no" -> DceNo
-		| _ -> failwith ("Unknown DCE mode " ^ dce_mode)
-	in
-	Dce.run com main dce_mode;
-	t();
-	com.stage <- CDceDone;
-	(* PASS 3: type filters post-DCE *)
-	List.iter
-		(run_expression_filters
-			(timer_label detail_times [])
-			tctx
-			["insert_save_stacks",Exceptions.insert_save_stacks tctx]
-		)
-		new_types;
-	let type_filters = [
-		Exceptions.patch_constructors tctx; (* TODO: I don't believe this should load_instance anything at this point... *)
-		check_private_path tctx;
-		apply_native_paths;
-		add_rtti com;
-		(match com.platform with | Java | Cs -> (fun _ -> ()) | _ -> (fun mt -> add_field_inits tctx.curclass.cl_path locals com mt));
-		(match com.platform with Hl -> (fun _ -> ()) | _ -> add_meta_field com);
-		check_void_field;
-		(match com.platform with | Cpp -> promote_first_interface_to_super | _ -> (fun _ -> ()));
-		commit_features com;
-		(if com.config.pf_reserved_type_paths <> [] then check_reserved_type_paths tctx else (fun _ -> ()));
-	] in
-	let type_filters = match com.platform with
-		| Cs -> type_filters @ [ fun t -> InterfaceProps.run t ]
-		| _ -> type_filters
-	in
-	let t = filter_timer detail_times ["type 3"] in
-	List.iter (fun t ->
-		begin match t with
-		| TClassDecl c ->
-			tctx.curclass <- c
-		| _ ->
-			()
-		end;
-		List.iter (fun f -> f t) type_filters
-	) com.types;
-	t();
-	List.iter (fun f -> f()) (List.rev com.callbacks#get_after_filters);
-	com.stage <- CFilteringDone
+	destruction tctx detail_times main locals

src/filters/filtersCommon.ml

@@ -48,7 +48,7 @@ let is_overridden cls field =
 	in
 	List.exists (fun d -> loop_inheritance d) cls.cl_descendants
 
-let run_expression_filters time_details ctx filters t =
+let run_expression_filters ?(ignore_processed_status=false) time_details ctx filters t =
 	let run e =
 		List.fold_left
 			(fun e (filter_name,f) ->
@@ -67,11 +67,13 @@ let run_expression_filters time_details ctx filters t =
 	| TClassDecl c ->
 		ctx.curclass <- c;
 		let rec process_field f =
-			ctx.curfield <- f;
-			(match f.cf_expr with
-			| Some e when not (is_removable_field ctx.com f) ->
-				f.cf_expr <- Some (rec_stack_loop AbstractCast.cast_stack f run e);
-			| _ -> ());
+			if ignore_processed_status || not (has_class_field_flag f CfPostProcessed) then begin
+				ctx.curfield <- f;
+				(match f.cf_expr with
+				| Some e when not (is_removable_field ctx.com f) ->
+					f.cf_expr <- Some (rec_stack_loop AbstractCast.cast_stack f run e);
+				| _ -> ());
+			end;
 			List.iter process_field f.cf_overloads
 		in
 		List.iter process_field c.cl_ordered_fields;

src/optimization/analyzer.ml

@@ -1058,7 +1058,7 @@ module Run = struct
 		Optimizer.reduce_control_flow com e
 
 	let run_on_field com config c cf = match cf.cf_expr with
-		| Some e when not (is_ignored cf.cf_meta) && not (Typecore.is_removable_field com cf) ->
+		| Some e when not (is_ignored cf.cf_meta) && not (Typecore.is_removable_field com cf) && not (has_class_field_flag cf CfPostProcessed) ->
 			let config = update_config_from_meta com config cf.cf_meta in
 			let actx = create_analyzer_context com config e in
 			let debug() =

src/typing/generic.ml

@@ -204,7 +204,7 @@ let set_type_parameter_dependencies mg tl =
 	in
 	List.iter loop tl
 
-let rec build_generic ctx c p tl =
+let rec build_generic_class ctx c p tl =
 	let pack = fst c.cl_path in
 	let recurse = ref false in
 	let rec check_recursive t =
@@ -274,6 +274,7 @@ let rec build_generic ctx c p tl =
 			) ([],[]) cf_old.cf_params in
 			let gctx = {gctx with subst = param_subst @ gctx.subst} in
 			let cf_new = {cf_old with cf_pos = cf_old.cf_pos} in (* copy *)
+			remove_class_field_flag cf_new CfPostProcessed;
 			(* Type parameter constraints are substituted here. *)
 			cf_new.cf_params <- List.rev_map (fun tp -> match follow tp.ttp_type with
 				| TInst({cl_kind = KTypeParameter tl1} as c,_) ->
@@ -308,7 +309,7 @@ let rec build_generic ctx c p tl =
 				unify_raise t0 t p;
 				link_dynamic t0 t;
 				t
-			) "build_generic" in
+			) "build_generic_class" in
 			cf_new.cf_type <- TLazy r;
 			cf_new
 		in
@@ -324,7 +325,7 @@ let rec build_generic ctx c p tl =
 				let cs,pl = TypeloadCheck.Inheritance.check_extends ctx c ts p in
 				match cs.cl_kind with
 				| KGeneric ->
-					(match build_generic ctx cs p pl with
+					(match build_generic_class ctx cs p pl with
 					| TInst (cs,pl) -> Some (cs,pl)
 					| _ -> die "" __LOC__)
 				| _ -> Some(cs,pl)

src/typing/instanceBuilder.ml

@@ -91,7 +91,7 @@ let build_instance ctx mtype p =
 		let ft = (fun pl ->
 			match c.cl_kind with
 			| KGeneric ->
-				build (fun () -> Generic.build_generic ctx c p pl) "build_generic"
+				build (fun () -> Generic.build_generic_class ctx c p pl) "build_generic"
 			| KMacroType ->
 				build (fun () -> build_macro_type ctx pl p) "macro_type"
 			| KGenericBuild cfl ->

src/typing/typeloadFunction.ml

@@ -185,13 +185,10 @@ let add_constructor ctx c force_constructor p =
 	let constructor = try Some (Type.get_constructor_class c (extract_param_types c.cl_params)) with Not_found -> None in
 	match constructor with
 	| Some(cfsup,csup,cparams) when not (has_class_flag c CExtern) ->
-		let cf = {
-			cfsup with
-			cf_pos = p;
-			cf_meta = List.filter (fun (m,_,_) -> m = Meta.CompilerGenerated) cfsup.cf_meta;
-			cf_doc = None;
-			cf_expr = None;
-		} in
+		let cf = mk_field "new" cfsup.cf_type p null_pos in
+		cf.cf_kind <- cfsup.cf_kind;
+		cf.cf_params <- cfsup.cf_params;
+		cf.cf_meta <- List.filter (fun (m,_,_) -> m = Meta.CompilerGenerated) cfsup.cf_meta;
 		let r = exc_protect ctx (fun r ->
 			let t = mk_mono() in
 			r := lazy_processing (fun() -> t);

src/typing/typer.ml

@@ -963,7 +963,7 @@ and type_new ctx path el with_type force_inline p =
 		ctx.call_argument_stack <- List.tl ctx.call_argument_stack;
 		(* Try to properly build @:generic classes here (issue #2016) *)
 		begin match t_follow with
-			| TInst({cl_kind = KGeneric } as c,tl) -> follow (Generic.build_generic ctx c p tl)
+			| TInst({cl_kind = KGeneric } as c,tl) -> follow (Generic.build_generic_class ctx c p tl)
 			| _ -> t
 		end
 	with
@@ -976,7 +976,7 @@ and type_new ctx path el with_type force_inline p =
 			no_abstract_constructor c p;
 			ignore (unify_constructor_call c fa);
 			begin try
-				Generic.build_generic ctx c p monos
+				Generic.build_generic_class ctx c p monos
 			with Generic.Generic_Exception _ as exc ->
 				(* If we have an expected type, just use that (issue #3804) *)
 				begin match with_type with

tests/server/.vscode/tasks.json

@@ -3,7 +3,7 @@
 	"tasks": [
 		{
 			"type": "hxml",
-			"file": "build.hxml",
+			"file": "run.hxml",
 			"group": {
 				"kind": "build",
 				"isDefault": true

tests/server/src/cases/display/issues/Issue10635.hx

@@ -1,5 +1,8 @@
 package cases.display.issues;
 
+import haxe.display.JsonModuleTypes;
+import haxe.Json;
+
 class Issue10635 extends DisplayTestCase {
 	/**
 		class C {
@@ -36,4 +39,24 @@ class Issue10635 extends DisplayTestCase {
 		runHaxe(args);
 		Assert.isTrue(lastResult.stderr.length == 2); // dumb, but we don't have a proper diagnostics structure in these tests
 	}
+
+	function testGenericAddition(_) {
+		var args = ["-main", "Main"];
+		vfs.putContent("GenericMethod.hx", getTemplate("GenericMethod.hx"));
+		vfs.putContent("Main.hx", getTemplate("issues/Issue10635/MainBefore.hx"));
+		runHaxe(args);
+		vfs.putContent("Main.hx", getTemplate("issues/Issue10635/MainAfter.hx"));
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		// Note: We only have to run this once to reproduce because ServerMethods.Type will call cl_restore anyway
+		runHaxe(args);
+		runHaxeJson(args, ServerMethods.Contexts, null);
+		var contexts:Array<HaxeServerContext> = Json.parse(lastResult.stderr).result.result;
+		utest.Assert.equals(1, contexts.length);
+		var sig = contexts[0].signature;
+		runHaxeJson(args, ServerMethods.Type, { signature: sig, modulePath: "GenericMethod", typeName: "GenericMethod"});
+		var type:JsonModuleType<JsonClass> = Json.parse(lastResult.stderr).result.result;
+		var statics = type.args.statics;
+		Assert.isTrue(statics.exists(cf -> cf.name == "f"));
+		Assert.isTrue(statics.exists(cf -> cf.name == "f_Class<Main>"));
+	}
 }

tests/server/test/templates/GenericMethod.hx

@@ -0,0 +1,3 @@
+class GenericMethod {
+	@:generic static public function f<T>(t:T) {}
+}

tests/server/test/templates/issues/Issue10635/MainAfter.hx

@@ -0,0 +1,6 @@
+class Main {
+	static function main() {
+		GenericMethod;
+		GenericMethod.f(Main);
+	}
+}

tests/server/test/templates/issues/Issue10635/MainBefore.hx

@@ -0,0 +1,5 @@
+class Main {
+	static function main() {
+		GenericMethod;
+	}
+}