From d11f0b952c99b119c37e884ea835c42cbc3d65ab Mon Sep 17 00:00:00 2001 From: Simon Krajewski Date: Tue, 21 Nov 2023 15:56:25 +0100 Subject: [PATCH] Run some filters in diagnostics (#11220) * let's see how much breaks * [tests] enable diagnostics tests for 11177 and 11184 * [tests] Update test for 5306 * Don't cache/run filters for find reference/implementation requests (#11226) * Only run filters and save cache on diagnostics, not usage requests * [tests] Update test for 11184 * disable test * add VUsedByTyper to avoid bad unused local errors * revert @:compilerGenerated change --------- Co-authored-by: Rudy Ges --- src/compiler/compiler.ml | 12 ++-- src/context/display/diagnostics.ml | 9 +-- src/core/displayTypes.ml | 1 - src/core/tType.ml | 7 ++- src/filters/filters.ml | 6 +- src/optimization/analyzer.ml | 6 +- src/optimization/inline.ml | 1 - src/typing/callUnification.ml | 1 - src/typing/typer.ml | 4 +- src/typing/typerBase.ml | 10 ++-- tests/display/src/cases/Issue5306.hx | 14 ++++- tests/server/src/cases/issues/Issue11177.hx | 55 +++++++++---------- tests/server/src/cases/issues/Issue11184.hx | 49 ++++++++--------- tests/server/src/cases/issues/Issue11203.hx | 23 ++++++++ .../issues/Issue11203/MainAbstract.hx | 16 ++++++ .../templates/issues/Issue11203/MainClass.hx | 15 +++++ 16 files changed, 147 insertions(+), 82 deletions(-) create mode 100644 tests/server/src/cases/issues/Issue11203.hx create mode 100644 tests/server/test/templates/issues/Issue11203/MainAbstract.hx create mode 100644 tests/server/test/templates/issues/Issue11203/MainClass.hx diff --git a/src/compiler/compiler.ml b/src/compiler/compiler.ml index 36b3509474e..840c6450468 100644 --- a/src/compiler/compiler.ml +++ b/src/compiler/compiler.ml @@ -302,10 +302,10 @@ let finalize_typing ctx tctx = com.modules <- modules; t() -let filter ctx tctx = +let filter ctx tctx before_destruction = let t = Timer.timer ["filters"] in DeprecationCheck.run ctx.com; - Filters.run ctx.com tctx ctx.com.main; + run_or_diagnose ctx (fun () -> Filters.run tctx ctx.com.main before_destruction); t() let compile ctx actx = @@ -334,8 +334,12 @@ let compile ctx actx = end; DisplayProcessing.handle_display_after_typing ctx tctx display_file_dot_path; finalize_typing ctx tctx; - DisplayProcessing.handle_display_after_finalization ctx tctx display_file_dot_path; - filter ctx tctx; + if is_diagnostics com then + filter ctx tctx (fun () -> DisplayProcessing.handle_display_after_finalization ctx tctx display_file_dot_path) + else begin + DisplayProcessing.handle_display_after_finalization ctx tctx display_file_dot_path; + filter ctx tctx (fun () -> ()); + end; if ctx.has_error then raise Abort; Generate.check_auxiliary_output com actx; com.stage <- CGenerationStart; diff --git a/src/context/display/diagnostics.ml b/src/context/display/diagnostics.ml index 29b7153375e..651fb4d8396 100644 --- a/src/context/display/diagnostics.ml +++ b/src/context/display/diagnostics.ml @@ -20,7 +20,7 @@ let find_unused_variables com e = let vars = Hashtbl.create 0 in let pmin_map = Hashtbl.create 0 in let rec loop e = match e.eexpr with - | TVar({v_kind = VUser _} as v,eo) when v.v_name <> "_" -> + | TVar({v_kind = VUser _} as v,eo) when v.v_name <> "_" && not (has_var_flag v VUsedByTyper) -> Hashtbl.add pmin_map e.epos.pmin v; let p = match eo with | None -> e.epos @@ -179,15 +179,10 @@ let prepare com = dctx.unresolved_identifiers <- com.display_information.unresolved_identifiers; dctx -let secure_generated_code ctx e = - (* This causes problems and sucks in general... need a different solution. But I forgot which problem this solved anyway. *) - (* mk (TMeta((Meta.Extern,[],e.epos),e)) e.etype e.epos *) - e - let print com = let dctx = prepare com in Json.string_of_json (DiagnosticsPrinter.json_of_diagnostics com dctx) let run com = let dctx = prepare com in - dctx \ No newline at end of file + dctx diff --git a/src/core/displayTypes.ml b/src/core/displayTypes.ml index 9e04cbc0195..93fe76263fd 100644 --- a/src/core/displayTypes.ml +++ b/src/core/displayTypes.ml @@ -235,7 +235,6 @@ module DisplayMode = struct | DMDefault | DMDefinition | DMTypeDefinition | DMPackage | DMHover | DMSignature -> settings | DMUsage _ | DMImplementation -> { settings with dms_full_typing = true; - dms_populate_cache = !ServerConfig.populate_cache_from_display; dms_force_macro_typing = true; dms_display_file_policy = DFPAlso; dms_exit_during_typing = false diff --git a/src/core/tType.ml b/src/core/tType.ml index 5fdb0509be7..1c140ef314a 100644 --- a/src/core/tType.ml +++ b/src/core/tType.ml @@ -442,7 +442,12 @@ let flag_tclass_field_names = [ type flag_tvar = | VCaptured | VFinal - | VUsed (* used by the analyzer *) + | VAnalyzed | VAssigned | VCaught | VStatic + | VUsedByTyper (* Set if the typer looked up this variable *) + +let flag_tvar_names = [ + "VCaptured";"VFinal";"VAnalyzed";"VAssigned";"VCaught";"VStatic";"VUsedByTyper" +] diff --git a/src/filters/filters.ml b/src/filters/filters.ml index d0af8ec9475..0cd27b07b6d 100644 --- a/src/filters/filters.ml +++ b/src/filters/filters.ml @@ -916,7 +916,8 @@ let save_class_state ctx t = a.a_meta <- List.filter (fun (m,_,_) -> m <> Meta.ValueUsed) a.a_meta ) -let run com tctx main = +let run tctx main before_destruction = + let com = tctx.com in let detail_times = Common.defined com DefineList.FilterTimes in let new_types = List.filter (fun t -> let cached = is_cached com t in @@ -1025,4 +1026,5 @@ let run com tctx main = let t = filter_timer detail_times ["callbacks"] in com.callbacks#run com.callbacks#get_after_save; (* macros onGenerate etc. *) t(); - destruction tctx detail_times main locals \ No newline at end of file + before_destruction(); + destruction tctx detail_times main locals diff --git a/src/optimization/analyzer.ml b/src/optimization/analyzer.ml index 82ab684ed66..f6c9825bb66 100644 --- a/src/optimization/analyzer.ml +++ b/src/optimization/analyzer.ml @@ -661,14 +661,14 @@ module LocalDce = struct let rec apply ctx = let is_used v = - has_var_flag v VUsed + has_var_flag v VAnalyzed in let keep v = is_used v || ((match v.v_kind with VUser _ | VInlined -> true | _ -> false) && not ctx.config.local_dce) || ExtType.has_reference_semantics v.v_type || has_var_flag v VCaptured || Meta.has Meta.This v.v_meta in let rec use v = if not (is_used v) then begin - add_var_flag v VUsed; + add_var_flag v VAnalyzed; (try expr (get_var_value ctx.graph v) with Not_found -> ()); begin match Ssa.get_reaching_def ctx.graph v with | None -> @@ -676,7 +676,7 @@ module LocalDce = struct reaching definition (issue #10972). Simply marking it as being used should be sufficient. *) let v' = get_var_origin ctx.graph v in if not (is_used v') then - add_var_flag v' VUsed + add_var_flag v' VAnalyzed | Some v -> use v; end diff --git a/src/optimization/inline.ml b/src/optimization/inline.ml index 098a1ff7af0..727c0648a66 100644 --- a/src/optimization/inline.ml +++ b/src/optimization/inline.ml @@ -589,7 +589,6 @@ class inline_state ctx ethis params cf f p = object(self) mk (TBlock (DynArray.to_list el)) tret e.epos in let e = inline_metadata e cf.cf_meta in - let e = Diagnostics.secure_generated_code ctx e in if has_params then begin let mt = map_type cf.cf_type in let unify_func () = unify_raise mt (TFun (tl,tret)) p in diff --git a/src/typing/callUnification.ml b/src/typing/callUnification.ml index 5accc16d384..13cb72f7c2b 100644 --- a/src/typing/callUnification.ml +++ b/src/typing/callUnification.ml @@ -488,7 +488,6 @@ object(self) !ethis_f(); raise exc in - let e = Diagnostics.secure_generated_code ctx e in ctx.com.located_error <- old; !ethis_f(); e diff --git a/src/typing/typer.ml b/src/typing/typer.ml index 5047c26bc31..e62aca173da 100644 --- a/src/typing/typer.ml +++ b/src/typing/typer.ml @@ -350,6 +350,7 @@ let rec type_ident_raise ctx i p mode with_type = | _ -> try let v = PMap.find i ctx.locals in + add_var_flag v VUsedByTyper; (match v.v_extra with | Some ve -> let (params,e) = (ve.v_params,ve.v_expr) in @@ -1130,7 +1131,7 @@ and type_new ctx path el with_type force_inline p = typing_error (s_type (print_context()) t ^ " cannot be constructed") p end with Error(No_constructor _ as err,p,depth) when ctx.com.display.dms_kind <> DMNone -> located_display_error ~depth ctx.com (error_msg p err); - Diagnostics.secure_generated_code ctx (mk (TConst TNull) t p) + mk (TConst TNull) t p and type_try ctx e1 catches with_type p = let e1 = type_expr ctx (Expr.ensure_block e1) with_type in @@ -1796,7 +1797,6 @@ and type_call_builtin ctx e el mode with_type p = | _ -> let e = type_expr ctx e WithType.value in warning ctx WInfo (s_type (print_context()) e.etype) e.epos; - let e = Diagnostics.secure_generated_code ctx e in e end | (EField(e,"match",efk_todo),p), [epat] -> diff --git a/src/typing/typerBase.ml b/src/typing/typerBase.ml index ca6192d7ac8..3370bd3447a 100644 --- a/src/typing/typerBase.ml +++ b/src/typing/typerBase.ml @@ -155,9 +155,11 @@ let get_this ctx p = | FunMemberClassLocal | FunMemberAbstractLocal -> let v = match ctx.vthis with | None -> - let v = if ctx.curfun = FunMemberAbstractLocal then - PMap.find "this" ctx.locals - else + let v = if ctx.curfun = FunMemberAbstractLocal then begin + let v = PMap.find "this" ctx.locals in + add_var_flag v VUsedByTyper; + v + end else add_local ctx VGenerated "`this" ctx.tthis p in ctx.vthis <- Some v; @@ -347,4 +349,4 @@ let get_abstract_froms ctx a pl = acc) | _ -> acc - ) l a.a_from_field \ No newline at end of file + ) l a.a_from_field diff --git a/tests/display/src/cases/Issue5306.hx b/tests/display/src/cases/Issue5306.hx index 941379acd46..043cc96ef36 100644 --- a/tests/display/src/cases/Issue5306.hx +++ b/tests/display/src/cases/Issue5306.hx @@ -7,8 +7,8 @@ class Issue5306 extends DisplayTestCase { class Main { static function main() { var ib:Array; - ib[0] = 0; ib[1] = 1; ib[2] - {-5-}trace{-6-}("test"); + {-5-}ib{-6-}[0] = 0; ib[1] = 1; ib[2] + {-7-}trace{-8-}("test"); } } **/ @@ -22,7 +22,7 @@ class Issue5306 extends DisplayTestCase { // }, { kind: DKParserError, - range: diagnosticsRange(pos(5), pos(6)), + range: diagnosticsRange(pos(7), pos(8)), severity: Error, relatedInformation: [], args: "Missing ;" @@ -33,6 +33,14 @@ class Issue5306 extends DisplayTestCase { severity: Error, relatedInformation: [], args: "Type not found : InvalidType" + }, + { + kind: DKCompilerError, + range: diagnosticsRange(pos(5), pos(6)), + severity: Error, + code: null, + relatedInformation: [], + args: "Local variable ib used without being initialized" } ]; arrayEq(expected, diagnostics()); diff --git a/tests/server/src/cases/issues/Issue11177.hx b/tests/server/src/cases/issues/Issue11177.hx index 16837b21e65..f967005da3d 100644 --- a/tests/server/src/cases/issues/Issue11177.hx +++ b/tests/server/src/cases/issues/Issue11177.hx @@ -1,33 +1,32 @@ package cases.issues; class Issue11177 extends TestCase { - // Disabled for now until #11177 is actually fixed, likely by #11220 - // function test(_) { - // vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx")); - // vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx")); - // vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx")); - // var args = ["-main", "Main", "--interp"]; - // runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> { - // Assert.equals(0, res.length); - // }); - // vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx")); - // runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")}); - // runHaxe(args); - // runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> { - // Assert.equals(0, res.length); - // }); - // } + function test(_) { + vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx")); + vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx")); + vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx")); + var args = ["-main", "Main", "--interp"]; + runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> { + Assert.equals(0, res.length); + }); + vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx")); + runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")}); + runHaxe(args); + runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> { + Assert.equals(0, res.length); + }); + } - // function testLegacyDiagnostics(_) { - // vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx")); - // vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx")); - // vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx")); - // var args = ["-main", "Main", "--interp"]; - // runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"])); - // vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx")); - // runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")}); - // runHaxe(args); - // runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"])); - // Assert.isTrue(lastResult.stderr.length == 2); - // } + function testLegacyDiagnostics(_) { + vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx")); + vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx")); + vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx")); + var args = ["-main", "Main", "--interp"]; + runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"])); + vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx")); + runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")}); + runHaxe(args); + runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"])); + Assert.isTrue(lastResult.stderr.length == 2); + } } diff --git a/tests/server/src/cases/issues/Issue11184.hx b/tests/server/src/cases/issues/Issue11184.hx index 2444c991ecd..966e595e818 100644 --- a/tests/server/src/cases/issues/Issue11184.hx +++ b/tests/server/src/cases/issues/Issue11184.hx @@ -1,32 +1,31 @@ package cases.issues; class Issue11184 extends TestCase { - // Disabled for now until #11184 is actually fixed, likely by #11220 - // function testDiagnostics(_) { - // vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx")); - // var args = ["-main", "Main", "-js", "bin/test.js"]; + function testDiagnostics(_) { + vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx")); + var args = ["-main", "Main", "-js", "bin/test.js"]; - // runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Main.hx")}, res -> { - // Assert.equals(1, res.length); - // Assert.equals(1, res[0].diagnostics.length); - // Assert.equals(res[0].diagnostics[0].args, "Cannot use Void as value"); - // }); + runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Main.hx")}, res -> { + Assert.equals(1, res.length); + Assert.equals(1, res[0].diagnostics.length); + Assert.equals(res[0].diagnostics[0].args, "Cannot use Void as value"); + }); - // runHaxe(args); - // Assert.isTrue(hasErrorMessage("Cannot use Void as value")); - // runHaxe(args); - // Assert.isTrue(hasErrorMessage("Cannot use Void as value")); - // } + runHaxe(args); + Assert.isTrue(hasErrorMessage("Cannot use Void as value")); + runHaxe(args); + Assert.isTrue(hasErrorMessage("Cannot use Void as value")); + } - // function testLegacyDiagnostics(_) { - // vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx")); - // var args = ["-main", "Main", "-js", "bin/test.js"]; - // runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"])); - // final diagnostics = haxe.Json.parse(lastResult.stderr)[0].diagnostics; - // Assert.equals(diagnostics[0].args, "Cannot use Void as value"); - // runHaxe(args); - // Assert.isTrue(hasErrorMessage("Cannot use Void as value")); - // runHaxe(args); - // Assert.isTrue(hasErrorMessage("Cannot use Void as value")); - // } + function testLegacyDiagnostics(_) { + vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx")); + var args = ["-main", "Main", "-js", "bin/test.js"]; + runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"])); + final diagnostics = haxe.Json.parse(lastResult.stderr)[0].diagnostics; + Assert.equals(diagnostics[0].args, "Cannot use Void as value"); + runHaxe(args); + Assert.isTrue(hasErrorMessage("Cannot use Void as value")); + runHaxe(args); + Assert.isTrue(hasErrorMessage("Cannot use Void as value")); + } } diff --git a/tests/server/src/cases/issues/Issue11203.hx b/tests/server/src/cases/issues/Issue11203.hx new file mode 100644 index 00000000000..f28726dcd13 --- /dev/null +++ b/tests/server/src/cases/issues/Issue11203.hx @@ -0,0 +1,23 @@ +package cases.issues; + +class Issue11203 extends TestCase { + function testClass(_) { + vfs.putContent("Main.hx", getTemplate("issues/Issue11203/MainClass.hx")); + var args = ["Main", "--interp"]; + runHaxe(args); + runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"])); + + var diag = parseDiagnostics(); + Assert.isTrue(diag.length == 0); + } + + function testAbstract(_) { + vfs.putContent("Main.hx", getTemplate("issues/Issue11203/MainAbstract.hx")); + var args = ["Main", "--interp"]; + runHaxe(args); + runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"])); + + var diag = parseDiagnostics(); + Assert.isTrue(diag.length == 0); + } +} diff --git a/tests/server/test/templates/issues/Issue11203/MainAbstract.hx b/tests/server/test/templates/issues/Issue11203/MainAbstract.hx new file mode 100644 index 00000000000..8e532f5eca9 --- /dev/null +++ b/tests/server/test/templates/issues/Issue11203/MainAbstract.hx @@ -0,0 +1,16 @@ +class Main { + static function main() { + var future = new Future(); + future.eager(); + } +} + +abstract Future({}) from {} { + public function new() + this = {}; + + public inline function eager():Future { + trace("much side effect!"); + return this; + } +} diff --git a/tests/server/test/templates/issues/Issue11203/MainClass.hx b/tests/server/test/templates/issues/Issue11203/MainClass.hx new file mode 100644 index 00000000000..b965a67c71e --- /dev/null +++ b/tests/server/test/templates/issues/Issue11203/MainClass.hx @@ -0,0 +1,15 @@ +class Main { + static function main() { + var future = new Future(); + future.eager(); + } +} + +class Future { + public function new() {} + + public inline function eager():Future { + trace("much side effect!"); + return this; + } +}