Skip to content

Fix race in graph checking of type extensions - #19062

Merged
T-Gro merged 11 commits into
dotnet:mainfrom
majocha:fix-19033
Nov 11, 2025
Merged

Fix race in graph checking of type extensions#19062
T-Gro merged 11 commits into
dotnet:mainfrom
majocha:fix-19033

Conversation

@majocha

@majochamajocha commented Nov 9, 2025

Copy link
Copy Markdown
Contributor

Description

Fixes#19033

By saving the declared type argument name instead of mutating (possibly concurrently) the source typar.

Checklist

  • Test cases added
  • Release notes entry updated:

@github-actions

github-actionsBot commented Nov 9, 2025

Copy link
Copy Markdown
Contributor

❗ Release notes required


✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
src/Compilerdocs/release-notes/.FSharp.Compiler.Service/11.0.0.md

@majochamajocha changed the title Save declared typar ident in opt dataFix race in graph checking of type extensionsNov 9, 2025
@majocha
majocha marked this pull request as ready for review November 9, 2025 20:23
@majocha
majocha requested a review from a team as a code ownerNovember 9, 2025 20:23
@majocha

Copy link
Copy Markdown
ContributorAuthor

To unblock #19028

@nojaf

Copy link
Copy Markdown
Contributor

It has been too long since I understood all this, so could you high-level explain to me why this works?

Does signature conformance happen after the parallel phase?

@majocha

majocha commented Nov 10, 2025

Copy link
Copy Markdown
ContributorAuthor

It has been too long since I understood all this, so could you high-level explain to me why this works?

Does signature conformance happen after the parallel phase?

No, it seems it just does SetIdent in the opposite direction sig -> impl. Or extended type -> extensions. :) So what is being mutated is naturally isolated by graph checking.

This seems the only place this happens so we can save the declared ident here.

@nojaf

Copy link
Copy Markdown
Contributor

Thanks, this is a clever solution. Well done!

Comment threadsrc/Compiler/TypedTree/TypedTree.fs Outdated
Comment threadsrc/Compiler/TypedTree/TypedTree.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadsrc/Compiler/TypedTree/TypedTree.fs
Comment threadtests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs
majochaand others added 6 commits November 10, 2025 19:58
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
@github-project-automationgithub-project-automationBot moved this from New to In Progress in F# Compiler and ToolingNov 11, 2025
@T-Gro
T-Gro enabled auto-merge (squash) November 11, 2025 11:27
@T-Gro
T-Gro merged commit 3c3a388 into dotnet:mainNov 11, 2025
38 checks passed
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in F# Compiler and ToolingNov 11, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Benign race condition in graph based type checking of type extensions but breaks determinism

3 participants

@majocha@nojaf@T-Gro