Skip to content

Eagerly format exceptions in FSI's interactive scripting API - #14026

Merged
abonie merged 3 commits into
dotnet:mainfrom
abonie:exception_eager_formatting
Oct 7, 2022
Merged

Eagerly format exceptions in FSI's interactive scripting API#14026
abonie merged 3 commits into
dotnet:mainfrom
abonie:exception_eager_formatting

Conversation

@abonie

@abonieabonie commented Oct 5, 2022

Copy link
Copy Markdown
Member

Adds eager formatting of exceptions to CompilationDiagnosticLogger (which accumulates exceptions in an array) in FsiEvaluationSession.

This should fix#13921 which was due to the fact, that type variables contained in ValueRestriction exception were mutated before diagnostics were committed.

Not sure whether this same approach should be used wider or not.

@abonie
abonieforce-pushed the exception_eager_formatting branch 2 times, most recently from 1258f09 to 93c68bdCompareOctober 6, 2022 12:04
@abonie
abonie marked this pull request as ready for review October 6, 2022 12:05
KevinRansom
KevinRansom previously approved these changes Oct 6, 2022
@KevinRansom
KevinRansom enabled auto-merge (squash) October 6, 2022 19:16
psfinaki
psfinaki previously approved these changes Oct 7, 2022
Comment threadsrc/Compiler/Symbols/FSharpDiagnostic.fs Outdated
Comment threadtests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs Outdated
@abonie
abonie dismissed stale reviews from psfinaki and KevinRansom via c83b221October 7, 2022 13:59
@abonie
abonieforce-pushed the exception_eager_formatting branch from faa18d7 to a3e432bCompareOctober 7, 2022 14:14
@abonie
abonie marked this pull request as draft October 7, 2022 14:15
auto-merge was automatically disabled October 7, 2022 14:15

Pull request was converted to draft

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.
@abonie
abonieforce-pushed the exception_eager_formatting branch from a3e432b to caf35fdCompareOctober 7, 2022 14:23
@abonie
abonie marked this pull request as ready for review October 7, 2022 14:24
@abonie

Copy link
Copy Markdown
MemberAuthor

@psfinaki@0101 Could you review again? I applied your suggestions

@abonie
abonie enabled auto-merge (squash) October 7, 2022 14:46
@abonie
abonie requested review from 0101 and psfinakiOctober 7, 2022 14:47
0101
0101 approved these changes Oct 7, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent error message between .NET interactive notebooks and VS / FSI session

4 participants

@abonie@0101@KevinRansom@psfinaki