Nest augmentation-member closures inside their declaring type under --realsig+

.github/instructions/CodeGen.instructions.md

@@ -4,3 +4,5 @@ applyTo:
 ---
 
 Read `docs/representations.md`.
+
+For `--realsig+` visibility / closure-placement bugs (MethodAccessException under realsig+, IL `private` vs `assembly`, where synthesized closures/state-machines/TLR-lifts nest via `eenv.cloc`), see the `realsig-codegen` skill.

.github/skills/realsig-codegen/SKILL.md

@@ -0,0 +1,90 @@
+---
+name: realsig-codegen
+description: Debug and fix --realsig+ (RealInternalSignature) codegen bugs in IlxGen — MethodAccessException / FieldAccessException / TypeAccessException at runtime, IL `private` vs `assembly` visibility, closure and TLR-lift placement (cloc / NestedTypeRefForCompLoc / effectiveCloc / moduleCloc). Use when a program compiles cleanly but crashes only under --realsig+, when IL accessibility differs between realsig modes, or when reasoning about where compiler-synthesized closures/state-machines/quotation helpers are nested.
+---
+
+# --realsig+ Codegen (IlxGen)
+
+## Mental model
+
+- `--realsig-` (legacy default): source `private` → IL `assembly`; `internal` → IL `assembly`. Visibility intent is hidden; almost everything intra-assembly is reachable.
+- `--realsig+`: source `private` → IL `private` (type-scoped); `internal` → IL `assembly`. Matches C# expectations. Flag exists since F# 8 GA; documented in `fsc --help`.
+- A compiler-synthesized helper (closure for an inner `let rec`, a `task`/`async`/`seq` state machine, a quotation-splice helper, or a TLR-lifted static) is emitted as its own IL type, nested under the type identified by `eenv.cloc`.
+- ECMA-335: a **nested** type may access its **enclosing** type's `private` members; a **sibling** nested type may NOT. So under `--realsig+`, a synthesized helper that calls a `private` member must nest **inside** the declaring type, or the CLR throws `MethodAccessException`/`FieldAccessException`/`TypeAccessException` at first invocation.
+- This is the usual root cause of "compiles clean, crashes only under `--realsig+`": the source is legal (the type checker allows `private` access from any lexical position within the type, including inner lambdas), but the helper landed beside the type instead of inside it.
+
+## Key code locations (src/Compiler/CodeGen/IlxGen.fs)
+
+- `GetIlxClosureFreeVars` builds the closure type-ref: `let ilCloTypeRef = NestedTypeRefForCompLoc eenv.cloc cloName`. Whatever `eenv.cloc` is here decides nesting.
+- `GenMethodForBinding` generates a member body; `eenvForMeth` is built from the incoming `eenv`. The body (and its closures) run lazily via `DelayCodeGenMethodForExpr`, capturing that env.
+- `AddEnclosingToEnv eenv enclosing name ns` sets `cloc.Enclosing = enclosing @ [name]` (the canonical way to push a type onto cloc). `mspec.MethodRef.DeclaringTypeRef` gives a member's exact IL declaring-type path (`Enclosing` + `Name`).
+- `effectiveCloc` / `moduleCloc` (PR #19882): TLR-lifted vals route to a stable module/init-class location; the TLR private-ref guard in `InnerLambdasToTopLevelFuncs.SelectTLRVals` refuses lifting an inner-rec that references a type-scoped `private` val under realsig+ (otherwise it would lose access when lifted to the module).
+- `ComputeMemberAccess hidden accessibility realsig` (≈line 485): the single point that maps source accessibility → IL access under realsig.
+
+## Gotcha: the optimizer hides the bug in minimal repros
+
+A trivial `private` member (e.g. `= 1`) is **inlined away** by the F# optimizer before codegen, so the call site disappears and the crash vanishes under `--optimize+`. To force a faithful repro, make the member non-inlinable: read mutable state (`backing + 1`) or mark it `[<NoCompilerInlining>]`. NOTE: `[<NoCompilerInlining>]` is the F# **optimizer** attribute; `[<MethodImpl(MethodImplOptions.NoInlining)>]` is the **JIT** attribute — for compiler-inlining experiments use `NoCompilerInlining`.
+
+## Repro methodology
+
+1. Use a **shipped** SDK fsc as a still-broken control: `& "C:\Program Files\dotnet\sdk\<ver>\FSharp\fsc.dll"` (or `dotnet <fsc.dll>`). Compile the same source with `--realsig+` and `--realsig-`; the bug is the delta.
+2. Always pass `--optimize+` (and a non-inlinable private) so the call survives to runtime.
+3. Inspect IL with ildasm and read **nesting by indentation**: a `.class … C` at 2-space indent with a child `.class … h@N` at 4-space indent = nested (good). Same indent = sibling (the bug). Confirm with the full type name in field refs, e.g. `M/C/h@8` (nested) vs `M/h@8` (sibling).
+4. Minimal repro shape (crashes only under `--realsig+`):
+
+```fsharp
+type C() =
+    static let mutable backing = 0
+    static member Set v = backing <- v
+    static member private Secret() = backing + 1   // non-inlinable -> survives to runtime
+type C with                                          // intrinsic augmentation
+    member _.Run() =
+        let rec h n = if n = 0 then C.Secret() else h (n - 1)
+        h 5
+```
+
+5. Compile and run (runtimeconfig pins the shared runtime):
+
+```bash
+dotnet <fsc.dll> --target:exe --out:X.dll --realsig+ --optimize+ -r:FSharp.Core.dll X.fs
+# X.runtimeconfig.json:
+# {"runtimeOptions":{"tfm":"net10.0","framework":{"name":"Microsoft.NETCore.App","version":"10.0.9"}}}
+dotnet X.dll
+```
+
+## Instrumentation pattern
+
+`dprintf` is not in scope in `GetIlxClosureFreeVars`; use `eprintfn` to stderr. To pin where two cases diverge, log the closure's enclosing cloc and the member identity, and capture a stack trace guarded by a predicate:
+
+```fsharp
+// in GenMethodForBinding, after `let m = v.Range`
+eprintfn "MFBDBG v=%s ext=%b cloc=[%s] apparent=%s"
+    v.LogicalName v.IsExtensionMember
+    (String.concat "/" eenv.cloc.Enclosing)
+    (match v.ApparentEnclosingEntity with Parent e -> e.LogicalName | ParentNone -> "<none>")
+if v.LogicalName = "Run" then eprintfn "STACK:\n%s" System.Environment.StackTrace
+```
+
+Rebuild FCS + fsc Release, compile a minimal intrinsic-vs-augmentation pair, diff the logs. Remove all instrumentation before committing.
+
+## Worked example: #19933 (PR #19955)
+
+Members declared in an **intrinsic augmentation** (`type C with member ...`) reached `GenMethodForBinding` with only the module in `eenv.cloc` (the augmentation is a separate definition group, so the type was not in the realsig dict-routing path that intrinsic members use), so their closures nested in the module as siblings of `C` → `MethodAccessException` under realsig+. Fix: normalize `eenv.cloc` to `mspec.MethodRef.DeclaringTypeRef` for every non-extension member under `g.realsig` at the top of `GenMethodForBinding` (idempotent for members that already have it; skip `v.IsExtensionMember` — real extension members live in their own module). Gating on `g.realsig` avoids perturbing realsig- IL baselines. One fix covers `let rec`, `task`/`async`, and quotation-splice closures because all go through the same `NestedTypeRefForCompLoc eenv.cloc` site.
+
+## Diagnostics reality (don't mis-cite)
+
+- `FS0193` is the catch-all default in `CompilerDiagnostics.fs` (`| _ -> 193`), not a specific check.
+- `FS0491` = `csMemberIsNotAccessible2` (`FSComp.txt`, raised from `ConstraintSolver.fs`) on overload resolution finding 0 accessible candidates; its "from inner lambda expressions" clause is about **protected**, not **private**.
+- There is no existing source-level guard for "private member captured into a lambda": an instance `member private this.Secret` called from `let f () = this.Secret()` inside another member compiles cleanly today.
+
+## Tests
+
+- Live under `tests/FSharp.Compiler.ComponentTests/EmittedIL/...`, namespace `EmittedIL.RealInternalSignature`.
+- Helpers: `FSharp src |> withRealInternalSignature realsig |> asExe |> withOptimize |> ignoreWarnings`, then `compileExeAndRun |> shouldSucceed` for runtime tests, or `verifyILPresent` / `verifyILNotPresent` for IL-structural assertions.
+- Use `[<Theory; InlineData(true); InlineData(false)>]` so both realsig settings run and a regression in either path is caught.
+- realsig baselines are the `*.RealInternalSignatureOn.*` / `*.RealInternalSignatureOff.*` `.il.bsl` pairs; regenerate with `TEST_UPDATE_BSL=1` and pair every diff with its `.fs` + a one-line semantic summary (e.g. "closure renested under declaring type").
+- IL shape changes can shift `tests/ILVerify` baselines — see the `ilverify-failure` skill.
+
+## Build/run on Windows (when the cwd is content-excluded)
+
+If the `powershell` tool rejects commands because it validates the first token against the repo path, invoke executables by absolute path: `& "C:\Program Files\Git\cmd\git.exe" …`, `& "C:\Program Files\dotnet\dotnet.exe" build …`. Run the built component-test dll directly: `dotnet exec <…\FSharp.Compiler.ComponentTests.dll> --filter-class "*Pattern*"` (xUnit v3 simple filters: `--filter-class` / `--filter-method` / `--filter-namespace`, `*` wildcard). Ensure the matching shared runtime exists under `<repo>\.dotnet\shared\Microsoft.NETCore.App\`.

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -1,5 +1,6 @@
 ### Fixed
 
+* Fix `MethodAccessException` under `--realsig+` when a closure (inner `let rec`, `task`/`async` state machine, or quotation splice) inside a member defined in an intrinsic type augmentation (`type C with member ...`) accesses a `private` member of `C`. The synthesized closure is now nested inside the declaring type instead of beside it in the module class. ([Issue #19933](https://github.com/dotnet/fsharp/issues/19933), [PR #19955](https://github.com/dotnet/fsharp/pull/19955))
 * Tooltip "Full name" now shows demangled companion module names (e.g. `MyType.func` instead of `MyTypeModule.func`). ([Issue #17335](https://github.com/dotnet/fsharp/issues/17335), [PR #19867](https://github.com/dotnet/fsharp/pull/19867))
 * Fix internal error (FS0193) when calling an indexed property setter with a named argument that matches an indexer parameter. ([Issue #16034](https://github.com/dotnet/fsharp/issues/16034), [PR #19851](https://github.com/dotnet/fsharp/pull/19851))
 * Fix missing FS1182 ("unused binding") warning for unused `let` function bindings inside class types. ([Issue #13849](https://github.com/dotnet/fsharp/issues/13849), [PR #19805](https://github.com/dotnet/fsharp/pull/19805))

src/Compiler/CodeGen/IlxGen.fs

@@ -9504,6 +9504,30 @@ and GenMethodForBinding
     let g = cenv.g
     let m = v.Range
 
+    // Closures synthesized inside a member body (inner `let rec`, `task`/`async` state
+    // machines, quotation-splice helpers) are nested in the IL type identified by
+    // eenv.cloc. Under --realsig+ a source-`private` member compiles to IL `private`
+    // (type-scoped), so a closure that calls it must nest inside the declaring type, not
+    // beside it in the module class, or the CLR raises MethodAccessException at runtime.
+    //
+    // Members declared in the type's own definition already reach here with the declaring
+    // type in eenv.cloc, but members declared in an intrinsic augmentation (`type C with
+    // member ...`) reach here with only the enclosing module in scope, because the
+    // augmentation is a separate definition group from the type. Normalize eenv.cloc to the
+    // declaring type for every non-extension member so closure placement is consistent
+    // (idempotent for members that already have it). Real extension members are compiled as
+    // static methods in their own module and must not be re-homed.
+    //
+    // This only matters under --realsig+: with the legacy --realsig- visibility a
+    // module-level sibling closure can still reach the (IL `assembly`) member, so the
+    // placement is left unchanged there to avoid perturbing existing IL.
+    let eenv =
+        match v.MemberInfo with
+        | Some _ when g.realsig && not v.IsExtensionMember ->
+            let declTref = mspec.MethodRef.DeclaringTypeRef
+            AddEnclosingToEnv eenv declTref.Enclosing declTref.Name None
+        | _ -> eenv
+
     // If a method has a witness-passing version of the code, then suppress
     // the generation of any witness in the non-witness passing version of the code
     let eenv =

tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/AugmentationClosureNesting.fs

@@ -0,0 +1,14 @@
+module Sample
+type C() =
+    static let mutable backing = 0
+    static member Set v = backing <- v
+    [<NoCompilerInlining>]
+    static member private Secret() = backing + 1
+type C with
+    member _.Run() =
+        let rec h n = if n = 0 then C.Secret() else h (n - 1)
+        h 5
+[<EntryPoint>]
+let main _ =
+    C.Set 41
+    if C().Run() = 42 then 0 else 1

tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/AugmentationClosureNesting.fs.RealInternalSignatureOff.il.bsl

@@ -0,0 +1,190 @@
+
+
+
+
+
+.assembly extern runtime { }
+.assembly extern FSharp.Core { }
+.assembly assembly
+{
+  .custom instance void [FSharp.Core]Microsoft.FSharp.Core.FSharpInterfaceDataVersionAttribute::.ctor(int32,
+                                                                                                      int32,
+                                                                                                      int32) = ( 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 ) 
+
+
+
+
+  .hash algorithm 0x00008004
+  .ver 0:0:0:0
+}
+.module assembly.exe
+
+.imagebase {value}
+.file alignment 0x00000200
+.stackreserve 0x00100000
+.subsystem 0x0003       
+.corflags 0x00000001    
+
+
+
+
+
+.class public abstract auto ansi sealed Sample
+       extends [runtime]System.Object
+{
+  .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) 
+  .class auto ansi serializable nested public C
+         extends [runtime]System.Object
+  {
+    .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 03 00 00 00 00 00 ) 
+    .field static assembly int32 backing
+    .field static assembly int32 init@2
+    .method public specialname rtspecialname instance void  .ctor() cil managed
+    {
+
+      .maxstack  8
+      IL_0000:  ldarg.0
+      IL_0001:  callvirt   instance void [runtime]System.Object::.ctor()
+      IL_0006:  ldarg.0
+      IL_0007:  pop
+      IL_0008:  ret
+    } 
+
+    .method public static void  Set(int32 v) cil managed
+    {
+
+      .maxstack  8
+      IL_0000:  nop
+      IL_0001:  volatile.
+      IL_0003:  ldsfld     int32 Sample/C::init@2
+      IL_0008:  ldc.i4.1
+      IL_0009:  bge.s      IL_0014
+
+      IL_000b:  call       void [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives/IntrinsicFunctions::FailStaticInit()
+      IL_0010:  nop
+      IL_0011:  nop
+      IL_0012:  br.s       IL_0015
+
+      IL_0014:  nop
+      IL_0015:  ldarg.0
+      IL_0016:  stsfld     int32 Sample/C::backing
+      IL_001b:  ret
+    } 
+
+    .method assembly static int32  Secret() cil managed
+    {
+      .custom instance void [FSharp.Core]Microsoft.FSharp.Core.NoCompilerInliningAttribute::.ctor() = ( 01 00 00 00 ) 
+
+      .maxstack  8
+      IL_0000:  nop
+      IL_0001:  volatile.
+      IL_0003:  ldsfld     int32 Sample/C::init@2
+      IL_0008:  ldc.i4.1
+      IL_0009:  bge.s      IL_0014
+
+      IL_000b:  call       void [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives/IntrinsicFunctions::FailStaticInit()
+      IL_0010:  nop
+      IL_0011:  nop
+      IL_0012:  br.s       IL_0015
+
+      IL_0014:  nop
+      IL_0015:  ldsfld     int32 Sample/C::backing
+      IL_001a:  ldc.i4.1
+      IL_001b:  add
+      IL_001c:  ret
+    } 
+
+    .method public hidebysig instance int32 Run() cil managed
+    {
+
+      .maxstack  8
+      IL_0000:  ldc.i4.5
+      IL_0001:  call       int32 Sample::h@9(int32)
+      IL_0006:  ret
+    } 
+
+    .method private specialname rtspecialname static void  .cctor() cil managed
+    {
+
+      .maxstack  8
+      IL_0000:  ldc.i4.0
+      IL_0001:  stsfld     int32 '<StartupCode$assembly>'.$Sample::init@
+      IL_0006:  ldsfld     int32 '<StartupCode$assembly>'.$Sample::init@
+      IL_000b:  pop
+      IL_000c:  ret
+    } 
+
+  } 
+
+  .method public static int32  main(string[] _arg1) cil managed
+  {
+    .entrypoint
+    .custom instance void [FSharp.Core]Microsoft.FSharp.Core.EntryPointAttribute::.ctor() = ( 01 00 00 00 ) 
+
+    .maxstack  8
+    IL_0000:  ldc.i4.0
+    IL_0001:  stsfld     int32 '<StartupCode$assembly>'.$Sample::init@
+    IL_0006:  ldsfld     int32 '<StartupCode$assembly>'.$Sample::init@
+    IL_000b:  pop
+    IL_000c:  ldc.i4.s   41
+    IL_000e:  call       void Sample/C::Set(int32)
+    IL_0013:  nop
+    IL_0014:  nop
+    IL_0015:  newobj     instance void Sample/C::.ctor()
+    IL_001a:  callvirt   instance int32 Sample/C::Run()
+    IL_001f:  ldc.i4.s   42
+    IL_0021:  bne.un.s   IL_0025
+
+    IL_0023:  ldc.i4.0
+    IL_0024:  ret
+
+    IL_0025:  ldc.i4.1
+    IL_0026:  ret
+  } 
+
+  .method assembly static int32  h@9(int32 n) cil managed
+  {
+
+    .maxstack  8
+    IL_0000:  nop
+    IL_0001:  ldarg.0
+    IL_0002:  brtrue.s   IL_000a
+
+    IL_0004:  call       int32 Sample/C::Secret()
+    IL_0009:  ret
+
+    IL_000a:  ldarg.0
+    IL_000b:  ldc.i4.1
+    IL_000c:  sub
+    IL_000d:  starg.s    n
+    IL_000f:  br.s       IL_0000
+  } 
+
+} 
+
+.class private abstract auto ansi sealed '<StartupCode$assembly>'.$Sample
+       extends [runtime]System.Object
+{
+  .field static assembly int32 init@
+  .custom instance void [runtime]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [runtime]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) 
+  .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) 
+  .custom instance void [runtime]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) 
+  .method private specialname rtspecialname static void  .cctor() cil managed
+  {
+
+    .maxstack  8
+    IL_0000:  ldc.i4.0
+    IL_0001:  stsfld     int32 Sample/C::backing
+    IL_0006:  ldc.i4.1
+    IL_0007:  volatile.
+    IL_0009:  stsfld     int32 Sample/C::init@2
+    IL_000e:  ret
+  } 
+
+} 
+
+
+
+
+
+