Skip to content

Properly fix realsig - #20880

Open
KevinRansom wants to merge 4 commits into
dotnet:mainfrom
KevinRansom:realsig
Open

KevinRansom wants to merge 4 commits into
dotnet:mainfrom
KevinRansom:realsig

Conversation

@KevinRansom

Copy link
Copy Markdown
Contributor

First, an apology.

Realsig was intended to be a feature to fix the IL representation of F# members. To be specific, to ensure that values in F# that were declared as private in the source code were represented as private in IL. Historically, F# represents private values as internal, which causes confusion among developers when they are using tools like ILSpy.

The reason for the original design was quite elegant. It makes it fairly straightforward, if a little convoluted in the code generator, to construct for each source file an initialization sequence that conforms to the F# specified initialization order of source files, that is, initialization occurs in the exact order it appears in the source file. C# does not have this ordering guarantee, which is among the many reasons why C# is not as satisfactory for scripting as F# is, with top-level C# being a slightly weird C# syntax.

Anyway, this visibility and sequencing behavior was fully, and I believe correctly, implemented in F# 9.0. At least I have seen no bugs reported against it, so I'm reasonably sure it's right. However, and here's the apology: I overlooked the fact that lots of F# code is implemented as closures, and closures, whilst being excellent things, are erased during optimization and replaced by top-level functions.

By "overlooked", I mean I literally missed it until we got a bug report after release indicating that realsig+ code was much bigger and slower than with realsig-. Our response was to change the default back to realsig- and leave realsig+ in place until I could fix the optimizer to emit correctly formatted top-level functions and erase the closures for realsig+ code generation.

Whilst I was still at Microsoft, I spent months on it and bounced hard. Although this PR is actually astonishingly small, I had to learn about parts of the compiler I had barely thought about before. When I was laid off in the May 2025 round of layoffs, an event that hastened my retirement, I suppose, even though nobody who knows me imagined I would voluntarily quit Microsoft. It may not be an easy place to work, but it is, without a shadow of a doubt, the most fantastic place to work.

Anyway, on and off since then I have continued to work on fixing the realsig+ optimization issue. This rather compact PR is the result.

In between looking after my grandson, who was eight months old and is now two years old, I figured out how to fix it, and even more importantly, how to get AI to do a decent job of helping fix it, although lots of times it said, "Nope, I can't fix it, just write a bug. The maintainers will know how to fix it." Sigh!!!!

Anyway, this was done using local AI, Qwen 3.8 and OpenCode, on a GMKTec EVO-X2 with a Ryzen AI Max+ 395 Pro and 128GB of shared memory.

I thoroughly recommend it. Qwen 3.8 is an astonishing model. I have no idea what Claude is like. It may be magical; I wouldn't know.

Here is what this PR contains.

Test cases:

The biggest part of this PR is test cases. I added a bunch more test cases. I did it up front, even though I wasn't sure exactly what the IL would be. I did have a decent idea of the closure types that needed to be handled. Also, many of the existing realsig test cases needed modification because the code generation was going to be modified for realsig+/optimize+, and that code needed improving.

I also added ilverify to more realsig+ test cases, as well as IL content verification.

The approach:

With realsig+ it was important to ensure that the original behaviour remained available in case unforeseen compatibility errors were introduced, and because SQLCLR requires initialization to happen only in class constructors (i.e. realsig- style).

In realsig- the TLRs are rehosted on a top-level function for the source code being generated.

In realsig+ the TLR is rehomed on the class in which the closure was originally discovered.

This rehoming happens primarily in:

and AllocValReprWithinExpr cenv cgbuf endMark cloc v eenv =

at:

let effectiveCloc =

There are a few other places where we also refer to the cloc in order to get the surrounding type information for the ilTypeInfo used when creating the MethodSpec.

For example, in:

ComputeStorageForFSharpFunctionOrFSharpExtensionMember

Okay, the main design element of the fix is:

RealsigClosureParentTryGet(v)

This is a table of registered closures. It contains the ParentType and the typars for that parent type. The typars are the ambient generic parameters for the type.

This table is necessary because:

member x.DeclaringEntity =

and

member x.MemberApparentEntity: TyconRef =

are always ParentNone for closures. Indeed, the type checker explicitly discards this information for closures and sets the parent container to None.

As an amusing aside, whenever the AI got stuck trying the right path, it would fall back and try super hard to use TryDeclaringEntity(). It would then waste a couple of cycles and usually fail to make progress for that run, and sometimes plead with me to just write a bug report for the feature.

The solution to losing this information was either to add the information to Val or to use this table. The table has the very nice benefit that gaining the new behaviour for a closure requires knowing where the closure is discovered and registering it. All other closures will behave exactly as they did before.

It does mean that TcGlobals now carries around this data, but there is some existing precedent for this.

Registration is keyed by Val, so the value that is going to be used to handle the closure data and methods.

It is mainly done during check expressions, although there is a case where registration happens in the TLR pipeline itself for the newly created fHat.

The other important algorithmic detail is this:

In realsig-, the top-level functions are static generic methods on the top-level module. The generic arguments of those methods are the free typars discovered by examining the method body and determining the generic types it uses.

In realsig+, some of those typars, often all of them, are in fact generic types from the class where the closure was discovered.

The TLR pipeline has been amended to reshape those functions into static members without those ambient typars on the method generic arguments, because they come from the surrounding class. This also requires the method body to have its generic typars mapped to the class typars when necessary.

This mapping is required because typars are cloned massively inside the compiler, and the code generator will not emit the correct IL if it does not have the correct clone in scope.

I have removed from the TLR pipeline the BodyReferencesTypeScopedPrivate refuse.

With the types rehosted on the closure's originating type, they should have access to privates. To be honest, this bug fix is probably what spurred me to knuckle down and fix realsig properly, because it pretty much says "go slow when you have private data", and the workaround is to make the data internal, which is exactly what realsig- was for.

I have also added:

if (not g.realsig) && ...

to LanguageFeature.AccessProtectedBaseFieldFromClosure for the same reason. Protected fields should be available from closures rehomed on the original class.

The rest of the code is, I believe, fairly straightforward.

@github-actions

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev

@KevinRansom,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

`* . (PR #XXXXX)`

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

Change path Release notes path Description
`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.200.md No release notes found or release notes format is not correct

@KevinRansom

Copy link
Copy Markdown
Contributor Author

@T-Gro -- Hey Tomas, somehow I managed to figure out realsig.

@auduchinok

Copy link
Copy Markdown
Member

Hey @KevinRansom, nice to see you again!

@KevinRansom

Copy link
Copy Markdown
Contributor Author

Hey @KevinRansom, nice to see you again!

Hey dood, it's been a while.

@KevinRansom

Copy link
Copy Markdown
Contributor Author

I get the same failures on Endtoend tests as the ci when building and running main:
Am i doing something wrong:

Build succeeded with 15 warning(s) in 11.1s
dotnet test --project BasicProvider.Tests\BasicProvider.Tests.fsproj -c Release -v minimal -p:TestTargetFramework=net472 -p:FSharpTestCompilerVersion=net40
Restore complete (0.1s)
info NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
BasicProvider.Tests net472 win-x86 failed with 10 error(s) (2.3s)
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(23,20): error FS3033: The type provider 'ProviderImplementation.BasicGenerativeProvider' reported an error: Error in pass3 for type Generative2, error: An attempt was made to load a program with an incorrect format. (Exception from HRESULT: 0x8007000B)
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(24,20): error FS3033: The type provider 'ProviderImplementation.BasicGenerativeProvider' reported an error: Error in pass3 for type Generative4, error: An attempt was made to load a program with an incorrect format. (Exception from HRESULT: 0x8007000B)
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(28,15): error FS0800: Invalid use of a type name
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(29,18): error FS0072: The type of this expression could not be inferred before accessing its members. Add a type annotation, e.g. (expr: SomeType), to constrain the type before this point.
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(30,18): error FS0072: The type of this expression could not be inferred before accessing its members. Add a type annotation, e.g. (expr: SomeType), to constrain the type before this point.
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(34,15): error FS0800: Invalid use of a type name
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(35,18): error FS0072: The type of this expression could not be inferred before accessing its members. Add a type annotation, e.g. (expr: SomeType), to constrain the type before this point.
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(36,18): error FS0072: The type of this expression could not be inferred before accessing its members. Add a type annotation, e.g. (expr: SomeType), to constrain the type before this point.
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(37,18): error FS0072: The type of this expression could not be inferred before accessing its members. Add a type annotation, e.g. (expr: SomeType), to constrain the type before this point.
c:\kevinransom\fsharp\tests\EndToEndBuildTests\BasicProvider\BasicProvider.Tests\BasicProvider.Tests.fs(38,18): error FS0072: The type of this expression could not be inferred before accessing its members. Add a type annotation, e.g. (expr: SomeType), to constrain the type before this point.

B

@majocha

majocha commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

I get the same failures on Endtoend tests as the ci when building and running main: Am i doing something wrong:

Hi Kevin!
possibly nuget cache still holds the BasicProvider built with pr compiler?

@KevinRansom

Copy link
Copy Markdown
Contributor Author

Dayum I broke generative type providers. That is not so great..

@KevinRansom

Copy link
Copy Markdown
Contributor Author

I get the same failures on Endtoend tests as the ci when building and running main: Am i doing something wrong:

Hi Kevin! possibly nuget cache still holds the BasicProvider built with pr compiler?

No, It was a failure in how I deal with typars for module hosted closures. It is weird that every unit test succeeded but this weird endtoend test showed the error, weird but lucky.

The one that defeats me is the determinism test. I have no idea how that happens, there is clearly a race in codegen when parallel. But I don't see how it's what I added because the behavior difference is gated on realsig. Somehow I guess I increased the chance of seeing a parallelism failure. But, when I look back the determinism leg seems to be quite reliable. And for sure I made I unreliable, i'm hoping it's not that I increased the time a leg takes so that it mostly loses now, because they are difficult to find.
Right now retirement is looking peaceful and desirable :-)


// [realsig+] Written from the typechecker (TcNormalizedBinding) and read from TLR (FlatEnvPacks).
// Keyed by closure Val.Stamp (int64) because `Val` is NoEquality. Only present when realsig is on.
let mutable realsigClosureParents : Map<int64, RealsigClosureParentInfo> = Map.empty

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the mutation safe under parallel typechecking?

@majocha

majocha commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

I get the same failures on Endtoend tests as the ci when building and running main: Am i doing something wrong:

Hi Kevin! possibly nuget cache still holds the BasicProvider built with pr compiler?

No, It was a failure in how I deal with typars for module hosted closures. It is weird that every unit test succeeded but this weird endtoend test showed the error, weird but lucky.

The one that defeats me is the determinism test. I have no idea how that happens, there is clearly a race in codegen when parallel. But I don't see how it's what I added because the behavior difference is gated on realsig. Somehow I guess I increased the chance of seeing a parallelism failure. But, when I look back the determinism leg seems to be quite reliable. And for sure I made I unreliable, i'm hoping it's not that I increased the time a leg takes so that it mostly loses now, because they are difficult to find. Right now retirement is looking peaceful and desirable :-)

:)

Yeah, parallel compilation (and determinism probably even more) is still buggy, see for example #20812, so everything is possible.

@T-Gro

T-Gro commented Nov 2, 2026

Copy link
Copy Markdown
Member

Hi @KevinRansom , good to see you back 👍 .
This is great stuff, kudos for figuring it out.

Could you pls point your OpenCode to the .github folder and apply the basic skills and instructions that apply, e.g. w.r.t. to code comments?

@KevinRansom

Copy link
Copy Markdown
Contributor Author

Hi @KevinRansom , good to see you back 👍 . This is great stuff, kudos for figuring it out.

Could you pls point your OpenCode to the .github folder and apply the basic skills and instructions that apply, e.g. w.r.t. to code comments?

Sure, I don't think it's figured out yet though. Originally I intended the optimized method shape in realsig- and realsig+ to be identical. Effectively rehost the optimized method on the original class the closure was discovered on.
But the Jit failed with out even though peverify and ilverify loved the IL. Which was where I was last year too. I finally succumbed and built a reshaped method that uses the parent classes typars instead of duplicate method typars.
I have to consider how an old compiler can correctly inline a public closure compiled realsig+ I don't think a refuse is acceptable, at least it is a performance take back and a reason to stick with realsig- as well as a requirement that we change metadata version number. Which I am struggling to avoid.

I will probably close the PR shortly when I know more about the inline IL cross assembly thing. I still don't really understand why it doesn't work. AI's fix which is almost certainly wrong, is wrong, because it just fails with some other closures now, I took at face value, but it always chooses the laziest first attempt.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

4 participants