Real accessibility

[KevinRansom]

When the F# compile writes out an assembly, it writes all members with internal and private scopes in the source code as internal. All members of types which are internal are written to the assembly with the IL internal scope. This PR introduces a new compiler switch that preserves the source code annotated scope in the IL.

For example the source code: Module example: Class type example:

This change of IL scope impacts the initalization code. Originally intialization required the generation of a type per source file with a .cctor containing all of the initialization code. With real IL signatures this initialization has had to move to a classInitialization method on each class that requires initialization. That is because internal values can be accessed from anywhere inside of the assembly, whereas a private type that is initialized can only happen from a location where that value can be accessed. Ie the class it is specified in and any nested class below it. In C# the initialization method is the .cctor for that class, unfortunately F# initialization is more structured than C#, C# initialization happens when the type is first used, whereas in F# initialization happens in the order of definition of the values within the source file.

Consider this F# program:

It produces output in this order:

So the initializers need to be invoked with care.

It should also be noted that a type reference can activate any of these types first, and so doing the initialization solely in class constructors would cause the intiialization sequenced to be out of order.

[github-actions[bot]]

:heavy_exclamation_mark: Release notes required

---

:white_check_mark: Found changes and release notes in following paths:

Change path	Release notes path	Description
src/FSharp.Core	docs/release-notes/.FSharp.Core/8.0.300.md
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.300.md

[vzarytovskii]

Before we proceed to merging it, as part of review, we need to make sure the section 12.5.1 of the spec, "Execution of Static Initializers" still true in new codegen. Otherwise some rfc needs to be written.

[KevinRansom]

Before we proceed to merging it, as part of review, we need to make sure the section 12.5.1 of the spec, "Execution of Static Initializers" still true in new codegen. Otherwise some rfc needs to be written.

I took every effort to ensure that initialization ordering remains identical, but yes we should indeed review that aspect.

[T-Gro]

Before we proceed to merging it, as part of review, we need to make sure the section 12.5.1 of the spec, "Execution of Static Initializers" still true in new codegen. Otherwise some rfc needs to be written.

I took every effort to ensure that initialization ordering remains identical, but yes we should indeed review that aspect.

I assume we will notice this in the diff of IL baselines ( I can volunteer to read those, at least a bigger subset)

[T-Gro]

ALL GREEN, this is excellent news. Ready for review?

[T-Gro]

(test of AI-generated summary)

This pull request introduces several changes primarily focused on enhancing code visibility and refining the build process. The most significant changes include the introduction of a realInternalSignature switch to generate types and members with IL visibility that accurately represents their F# visibility, modifications to the azure-pipelines.yml to add new jobs for WindowsRealInternalSignature testing, and changes to eng/Build.ps1 to incorporate the realInternalSignature switch into the build process.

Changes related to realInternalSignature:

• Directory.Build.props: Added a new PropertyGroup to define constants and other flags based on the RealInternalSignature condition.
• FSharpBuild.Directory.Build.props: Adjusted the PropertyGroup related to the 'Proto' configuration.
• eng/Build.ps1: Incorporated the realInternalSignature switch into the build process. [1] [2] [3] [4]
• src/Compiler/Checking/CheckBasics.fs: Added realInternalSignature to TcFileState and adjusted related functions to accommodate this change. [1] [2] [3]
• src/Compiler/Checking/CheckBasics.fsi: Added realInternalSignature to TcFileState and adjusted related functions to accommodate this change. [1] [2]
• src/Compiler/Checking/CheckDeclarations.fs: Adjusted several functions to accommodate the realInternalSignature switch. [1] [2] [3] [4] [5] [6]

Changes related to azure-pipelines.yml:

• azure-pipelines.yml: Added new jobs for WindowsRealInternalSignature testing for both Coreclr and Desktop.

Changes related to documentation:

• docs/release-notes/.FSharp.Compiler.Service/8.0.300.md: Added a note about the new switch to generate types and members with IL visibility that accurately represents their F# visibility.
• docs/release-notes/.FSharp.Core/8.0.200.md: Added a note about the new switch to generate types and members with IL visibility that accurately represents their F# visibility.
• docs/release-notes/.FSharp.Core/8.0.300.md: Added a note about minor tweaks to inline specifications to support the Visibility PR.

Other changes:

• FSharp.Compiler.Service.sln.DotSettings: Replaced the word 'mustinline' with 'shouldinline'.
• src/Compiler/Checking/CheckComputationExpressions.fs: Replaced the word 'mustInline' with 'shouldInline'.
• src/Compiler/AbstractIL/il.fs: Removed an unnecessary line in ILMethodDef.

[KevinRansom]

/run fantomas

[github-actions[bot]]

Ran fantomas: https://github.com/dotnet/fsharp/actions/runs/7667048518

[TIHan]

Does this have changes on where closures are defined?

[KevinRansom]

Does this have changes on where closures are defined?

@TIHan no. It just moves where the code is generated to a type nested where it is visible rather than parallel to where it is visible.

[KevinRansom]

Real visibility used together with signature files.

What is the default visibility for symbols used only in implementation .fs and never exposed via the signature .fsi ? Should they get to be private as well?

Real visibility used together with signature files.

What is the default visibility for symbols used only in implementation .fs and never exposed via the signature .fsi ? Should they get to be private as well?

@T-Gro

I just looked at the fsi testing and wierdly I forgot the scenario where implementation wasn't mentioned in the fsi file. I will add testing. I believe the result should be 'hidden' which should result in 'internal'

[psfinaki]

Awesome work! Any change to get those BSL migrations separately? This would make it more reviewable...

[auduchinok]

The change sounds great!

This PR introduces a new compiler switch that preserves the source code annotated scope in the IL.

Is this going to be a new default one day?

[vzarytovskii]

The change sounds great!

This PR introduces a new compiler switch that preserves the source code annotated scope in the IL.

Is this going to be a new default one day?

Maybe one day, if stars align.

[auduchinok]

Maybe one day, if stars align.

With so much work done here, it would be sad to have it hidden under a feature flag 🙂 Not enabling it also means we'd have much less real life testing, compared to what the most of the users would use.

[vzarytovskii]

Maybe one day, if stars align.

With so much work done here, it would be sad to have it hidden under a feature flag 🙂 Not enabling it also means we'd have much less real life testing, compared to what the most of the users would use.

Sure, as it is with all opt-in features and warnings. It's a huge change in codegen and takes time.