Only set cf_expr_unoptimized if we think we need it (#11462)

* [typer] set cf_expr_unoptimized only if we think we need it

closes #11460

* use server/invalidate like it's 2024

* actually just delete touchFile so I don't use it again

* invalidate more because putContent doesn't do it...

This is not a great testing experience I'm having but oh well...

src-json/warning.json

@@ -98,6 +98,11 @@
 		"doc": "A type path is being used that is supposed to be reserved on the current target",
 		"parent": "WTyper"
 	},
+	{
+		"name": "WInlineOptimizedField",
+		"doc": "A cached field which was optimized might lead to different output when inlined",
+		"parent": "WTyper"
+	},
 	{
 		"name": "WPatternMatcher",
 		"doc": "Warnings related to the pattern matcher",

src/filters/filters.ml

@@ -708,6 +708,15 @@ let save_class_state com t =
 			a.a_meta <- List.filter (fun (m,_,_) -> m <> Meta.ValueUsed) a.a_meta
 		)
 
+let might_need_cf_unoptimized c cf =
+	match cf.cf_kind,c.cl_kind with
+	| Method MethInline,_ ->
+		true
+	| _,KGeneric ->
+		true
+	| _ ->
+		has_class_field_flag cf CfGeneric
+
 let run tctx main before_destruction =
 	let com = tctx.com in
 	let detail_times = (try int_of_string (Common.defined_value_safe com ~default:"0" Define.FilterTimes) with _ -> 0) in
@@ -723,8 +732,11 @@ let run tctx main before_destruction =
 				(* Save cf_expr_unoptimized early: We want to inline with the original expression
 				   on the next compilation. *)
 				if not cached then begin
-					let field cf =
-						cf.cf_expr_unoptimized <- cf.cf_expr
+					let field cf = match cf.cf_expr,cf.cf_expr_unoptimized with
+						| Some e,None when might_need_cf_unoptimized cls cf ->
+							cf.cf_expr_unoptimized <- Some e
+						| _ ->
+							()
 					in
 					List.iter field cls.cl_ordered_fields;
 					List.iter field cls.cl_ordered_statics;

src/typing/calls.ml

@@ -67,17 +67,27 @@ let make_call ctx e params t ?(force_inline=false) p =
 		);
 		let params = List.map (Optimizer.reduce_expression ctx) params in
 		let force_inline = is_forced_inline cl f in
-		(match f.cf_expr_unoptimized,f.cf_expr with
-		| Some {eexpr = TFunction fd},_
-		| None,Some { eexpr = TFunction fd } ->
+		let inline fd =
 			Inline.type_inline ctx f fd ethis params t config p force_inline
+		in
+		begin match f.cf_expr_unoptimized with
+		| Some {eexpr = TFunction fd} ->
+			inline fd
 		| _ ->
-			(*
-				we can't inline because there is most likely a loop in the typing.
-				this can be caused by mutually recursive vars/functions, some of them
-				being inlined or not. In that case simply ignore inlining.
-			*)
-			raise Exit)
+			if has_class_field_flag f CfPostProcessed then
+				warning ctx  WInlineOptimizedField (Printf.sprintf "Inlining of cached field %s might lead to unexpected output" f.cf_name) p;
+			match f.cf_expr with
+			| Some ({ eexpr = TFunction fd } as e) ->
+				f.cf_expr_unoptimized <- Some (e);
+				inline fd
+			| _ ->
+				(*
+					we can't inline because there is most likely a loop in the typing.
+					this can be caused by mutually recursive vars/functions, some of them
+					being inlined or not. In that case simply ignore inlining.
+				*)
+				raise Exit
+		end
 	with Exit ->
 		mk (TCall (e,params)) t p
 

tests/server/src/TestCase.hx

@@ -213,6 +213,10 @@ class TestCase implements ITest {
 		}
 	}
 
+	function assertSilence() {
+		return Assert.isTrue(lastResult.stderr == "");
+	}
+
 	function assertSuccess(?p:haxe.PosInfos) {
 		return Assert.isTrue(0 == errorMessages.length, p);
 	}

tests/server/src/cases/ServerTests.hx

@@ -150,10 +150,9 @@ class ServerTests extends TestCase {
 		vfs.putContent("Other.hx", getTemplate("issues/Issue9134/Other.hx"));
 		var args = ["-main", "Main", "Other"];
 
-		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {fileContents: [
-			{file: new FsPath("Other.hx")},
-			{file: new FsPath("Main.hx")},
-		]}, res -> {
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {
+			fileContents: [{file: new FsPath("Other.hx")}, {file: new FsPath("Main.hx")},]
+		}, res -> {
 			Assert.equals(1, res.length);
 			Assert.equals(1, res[0].diagnostics.length);
 			var arg = res[0].diagnostics[0].args;
@@ -164,10 +163,12 @@ class ServerTests extends TestCase {
 		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
 		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Other.hx")});
 
-		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {fileContents: [
-			{file: new FsPath("Main.hx"), contents: getTemplate("issues/Issue9134/Main2.hx")},
-			{file: new FsPath("Other.hx"), contents: getTemplate("issues/Issue9134/Other2.hx")}
-		]}, res -> {
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {
+			fileContents: [
+				{file: new FsPath("Main.hx"), contents: getTemplate("issues/Issue9134/Main2.hx")},
+				{file: new FsPath("Other.hx"), contents: getTemplate("issues/Issue9134/Other2.hx")}
+			]
+		}, res -> {
 			Assert.equals(1, res.length);
 			Assert.equals(1, res[0].diagnostics.length);
 			var arg = res[0].diagnostics[0].args;
@@ -178,10 +179,12 @@ class ServerTests extends TestCase {
 		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
 		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Other.hx")});
 
-		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {fileContents: [
-			{file: new FsPath("Main.hx"), contents: getTemplate("issues/Issue9134/Main.hx")},
-			{file: new FsPath("Other.hx"), contents: getTemplate("issues/Issue9134/Other2.hx")}
-		]}, res -> {
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {
+			fileContents: [
+				{file: new FsPath("Main.hx"), contents: getTemplate("issues/Issue9134/Main.hx")},
+				{file: new FsPath("Other.hx"), contents: getTemplate("issues/Issue9134/Other2.hx")}
+			]
+		}, res -> {
 			Assert.equals(2, res.length);
 
 			for (i in 0...2) {
@@ -206,9 +209,12 @@ class ServerTests extends TestCase {
 
 			for (result in res) {
 				var file = result.file.toString();
-				if (StringTools.endsWith(file, "Main.hx")) hasMain = true;
-				else if (StringTools.endsWith(file, "Other.hx")) hasOther = true;
-				else continue;
+				if (StringTools.endsWith(file, "Main.hx"))
+					hasMain = true;
+				else if (StringTools.endsWith(file, "Other.hx"))
+					hasOther = true;
+				else
+					continue;
 
 				var arg = result.diagnostics[0].args;
 				Assert.equals("Unused variable", (cast arg).description);
@@ -427,7 +433,7 @@ class ServerTests extends TestCase {
 		vfs.putContent("haxe/ds/Vector.hx", getTemplate("issues/Issue10986/Vector.hx"));
 		var args = ["-main", "Main", "--jvm", "Main.jar"];
 		runHaxe(args);
-		vfs.touchFile("haxe/ds/Vector.hx");
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("haxe/ds/Vector.hx")});
 		runHaxe(args);
 		assertSuccess();
 	}
@@ -459,16 +465,18 @@ class ServerTests extends TestCase {
 		var transform = Marker.extractMarkers(getTemplate("issues/Issue8368/MyMacro2.macro.hx"));
 		var args = ["-main", "Main", "--macro", "define('whatever')"];
 
-		vfs.putContent(
-			"MyMacro.macro.hx",
-			transform.source.substr(0, transform.markers[1])
-			+ transform.source.substr(transform.markers[2], transform.source.length)
-		);
+		vfs.putContent("MyMacro.macro.hx",
+			transform.source.substr(0, transform.markers[1]) + transform.source.substr(transform.markers[2], transform.source.length));
 
 		runHaxe(args);
 		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("MyMacro.macro.hx")});
 
-		var completionRequest = {file: new FsPath("MyMacro.macro.hx"), contents: transform.source, offset: transform.markers[2], wasAutoTriggered: false};
+		var completionRequest = {
+			file: new FsPath("MyMacro.macro.hx"),
+			contents: transform.source,
+			offset: transform.markers[2],
+			wasAutoTriggered: false
+		};
 		runHaxeJson(args, DisplayMethods.Completion, completionRequest);
 		Assert.isTrue(parseCompletion().result.items.length == 23);
 		runHaxeJson(args, DisplayMethods.Completion, completionRequest);
@@ -488,8 +496,10 @@ class ServerTests extends TestCase {
 		function runLoop() {
 			runHaxeJson(args, DisplayMethods.Diagnostics, {file: new FsPath("Empty.hx")}, () -> {
 				runHaxe(args.concat(["-D", "compile-only-define"]), () -> {
-					if (assertSuccess() && ++runs < 20) runLoop();
-					else async.done();
+					if (assertSuccess() && ++runs < 20)
+						runLoop();
+					else
+						async.done();
 				});
 			});
 		}

tests/server/src/cases/issues/Issue11460.hx

@@ -0,0 +1,53 @@
+package cases.issues;
+
+using StringTools;
+
+class Issue11460 extends TestCase {
+	function testClass(_) {
+		var mainContentWithInline = getTemplate("issues/Issue11460/Main.hx");
+		var mainContentWithoutInline = mainContentWithInline.replace("inline", "");
+		vfs.putContent("Main.hx", mainContentWithInline);
+		vfs.putContent("C.hx", getTemplate("issues/Issue11460/C.hx"));
+		var args = ["Main", "--interp"];
+
+		// initial cache
+		runHaxe(args);
+		assertSilence();
+
+		// touching Main doesn't do anything
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxe(args);
+		assertSilence();
+
+		// touching both doesn't do anything
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("C.hx")});
+		runHaxe(args);
+		assertSilence();
+
+		// removing the inline is fine
+		vfs.putContent("Main.hx", mainContentWithoutInline);
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxe(args);
+		assertSilence();
+
+		// adding it back is fine too because C is still cached
+		vfs.putContent("Main.hx", mainContentWithInline);
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxe(args);
+		assertSilence();
+
+		// removing the inline and changing C is still fine
+		vfs.putContent("Main.hx", mainContentWithoutInline);
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("C.hx")});
+		runHaxe(args);
+		assertSilence();
+
+		// but adding it now gives us the warning
+		vfs.putContent("Main.hx", mainContentWithInline);
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxe(args);
+		utest.Assert.match(~/WInlineOptimizedField/, lastResult.stderr);
+	}
+}

tests/server/src/cases/issues/Issue9358.hx

@@ -6,7 +6,7 @@ class Issue9358 extends TestCase {
 		vfs.putContent("StateHandler.hx", getTemplate("issues/Issue9358/StateHandler.hx"));
 		var args = ["-cp", "src", "-m", "Main", "-hl", "hl.hl"];
 		runHaxe(args);
-		vfs.touchFile("Main.hx");
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Dependency.hx")});
 		runHaxe(args);
 		assertSuccess();
 	}

tests/server/src/utils/Vfs.hx

@@ -17,16 +17,6 @@ class Vfs {
 		FileSystem.createDirectory(physicalPath);
 	}
 
-	public function touchFile(path:String) {
-		var path = getPhysicalPath(path);
-		FileSystem.createDirectory(path.dir);
-		var file = Fs.openSync(path.dir + "/" + path.file + "." + path.ext, 'a');
-		var last = Fs.fstatSync(file).mtime;
-		var notNow = last.delta(1000);
-		Fs.futimesSync(file, notNow, notNow);
-		Fs.closeSync(file);
-	}
-
 	public function overwriteContent(path:String, content:String) {
 		var path = getPhysicalPath(path).toString();
 		if (!FileSystem.exists(path)) {

tests/server/test/templates/issues/Issue11460/C.hx

@@ -0,0 +1,5 @@
+class C {
+	static public function doSomething() {
+		trace("Ok I did something");
+	}
+}

tests/server/test/templates/issues/Issue11460/Main.hx

@@ -0,0 +1,3 @@
+function main() {
+	inline C.doSomething();
+}