From 4a5eceed69fad42519572ce0c8813f9713cd7d2a Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 4 Oct 2022 18:19:12 +0200 Subject: [PATCH 1/3] Add eager formatting of exceptions to fsi Eval* methods of FSIEvaluationSession that are using CompilationDiagnosticsLogger are at risk of mutating data that's referred to in the exception before error message is printed. To prevent that, this commit makes CompilationDiagnosticsLogger take additional argument that can control early pre-processing of logged exceptions. --- src/Compiler/Interactive/fsi.fs | 8 +++++--- src/Compiler/Symbols/FSharpDiagnostic.fs | 9 +++++++-- src/Compiler/Symbols/FSharpDiagnostic.fsi | 4 +++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index 6552c6a62da..f4ee6685a11 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -3521,6 +3521,8 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let dummyScriptFileName = "input.fsx" + let eagerFormat (diag : PhasedDiagnostic) = diag.EagerlyFormatCore true + interface IDisposable with member _.Dispose() = (tcImports :> IDisposable).Dispose() @@ -3639,7 +3641,7 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let ctok = AssumeCompilationThreadWithoutEvidence() let errorOptions = TcConfig.Create(tcConfigB,validate = false).diagnosticsOptions - let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions) + let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions, eagerFormat) fsiInteractionProcessor.EvalExpression(ctok, code, dummyScriptFileName, diagnosticsLogger) |> commitResultNonThrowing errorOptions dummyScriptFileName diagnosticsLogger @@ -3661,7 +3663,7 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let cancellationToken = defaultArg cancellationToken CancellationToken.None let errorOptions = TcConfig.Create(tcConfigB,validate = false).diagnosticsOptions - let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions) + let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions, eagerFormat) fsiInteractionProcessor.EvalInteraction(ctok, code, dummyScriptFileName, diagnosticsLogger, cancellationToken) |> commitResultNonThrowing errorOptions "input.fsx" diagnosticsLogger @@ -3682,7 +3684,7 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i let ctok = AssumeCompilationThreadWithoutEvidence() let errorOptions = TcConfig.Create(tcConfigB, validate = false).diagnosticsOptions - let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions) + let diagnosticsLogger = CompilationDiagnosticLogger("EvalInteraction", errorOptions, eagerFormat) fsiInteractionProcessor.EvalScript(ctok, filePath, diagnosticsLogger) |> commitResultNonThrowing errorOptions filePath diagnosticsLogger |> function Choice1Of2 _, errs -> Choice1Of2 (), errs | Choice2Of2 exn, errs -> Choice2Of2 exn, errs diff --git a/src/Compiler/Symbols/FSharpDiagnostic.fs b/src/Compiler/Symbols/FSharpDiagnostic.fs index a0c9a47fa88..b4633ba4a7f 100644 --- a/src/Compiler/Symbols/FSharpDiagnostic.fs +++ b/src/Compiler/Symbols/FSharpDiagnostic.fs @@ -158,13 +158,18 @@ type DiagnosticsScope() = | None -> err "" /// A diagnostics logger that capture diagnostics, filtering them according to warning levels etc. -type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDiagnosticOptions) = +type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDiagnosticOptions, ?preprocess: (PhasedDiagnostic -> PhasedDiagnostic)) = inherit DiagnosticsLogger("CompilationDiagnosticLogger("+debugName+")") let mutable errorCount = 0 let diagnostics = ResizeArray<_>() override _.DiagnosticSink(diagnostic, severity) = + let diagnostic = + match preprocess with + | Some f -> f diagnostic + | _ -> diagnostic + if diagnostic.ReportAsError (options, severity) then diagnostics.Add(diagnostic, FSharpDiagnosticSeverity.Error) errorCount <- errorCount + 1 @@ -172,7 +177,7 @@ type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDia diagnostics.Add(diagnostic, FSharpDiagnosticSeverity.Warning) elif diagnostic.ReportAsInfo (options, severity) then diagnostics.Add(diagnostic, severity) - + override _.ErrorCount = errorCount member _.GetDiagnostics() = diagnostics.ToArray() diff --git a/src/Compiler/Symbols/FSharpDiagnostic.fsi b/src/Compiler/Symbols/FSharpDiagnostic.fsi index 2ebfcb71980..313c14240e4 100644 --- a/src/Compiler/Symbols/FSharpDiagnostic.fsi +++ b/src/Compiler/Symbols/FSharpDiagnostic.fsi @@ -106,7 +106,9 @@ type internal CompilationDiagnosticLogger = inherit DiagnosticsLogger /// Create the diagnostics logger - new: debugName: string * options: FSharpDiagnosticOptions -> CompilationDiagnosticLogger + new: + debugName: string * options: FSharpDiagnosticOptions * ?preprocess: (PhasedDiagnostic -> PhasedDiagnostic) -> + CompilationDiagnosticLogger /// Get the captured diagnostics member GetDiagnostics: unit -> (PhasedDiagnostic * FSharpDiagnosticSeverity)[] From aa262828a8d0a144d24611d82b30c6d46127d671 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 5 Oct 2022 16:16:14 +0200 Subject: [PATCH 2/3] Add a test for ValueRestriction error --- .../FSharpScriptTests.fs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs b/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs index c218cc2f408..b74e81a3641 100644 --- a/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs +++ b/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs @@ -16,6 +16,15 @@ open Xunit type InteractiveTests() = + [] + member _.``ValueRestriction error message should not have type variables fully solved``() = + use script = new FSharpScript() + let code = "id id" + let _, errors = script.Eval(code) + Assert.Equal(1, errors.Length) + let msg = errors[0].Message + Assert.DoesNotMatch("obj -> obj", msg) + [] member _.``Eval object value``() = use script = new FSharpScript() From caf35fdcf34c41c4f49f78669af5e30b2d703bb3 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 7 Oct 2022 15:58:38 +0200 Subject: [PATCH 3/3] Minor fixes from code review --- src/Compiler/Symbols/FSharpDiagnostic.fs | 2 +- .../FSharpScriptTests.fs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Symbols/FSharpDiagnostic.fs b/src/Compiler/Symbols/FSharpDiagnostic.fs index b4633ba4a7f..c4b57e84c9f 100644 --- a/src/Compiler/Symbols/FSharpDiagnostic.fs +++ b/src/Compiler/Symbols/FSharpDiagnostic.fs @@ -168,7 +168,7 @@ type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDia let diagnostic = match preprocess with | Some f -> f diagnostic - | _ -> diagnostic + | None -> diagnostic if diagnostic.ReportAsError (options, severity) then diagnostics.Add(diagnostic, FSharpDiagnosticSeverity.Error) diff --git a/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs b/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs index b74e81a3641..2c4a8dd198f 100644 --- a/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs +++ b/tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs @@ -23,7 +23,7 @@ type InteractiveTests() = let _, errors = script.Eval(code) Assert.Equal(1, errors.Length) let msg = errors[0].Message - Assert.DoesNotMatch("obj -> obj", msg) + Assert.Matches("'_\\w+ -> '_\\w+", msg) [] member _.``Eval object value``() =