Fix #17797 -- Realsig+ generates nested closures with incorrect Generic arguments (#17877)

* tests

* code

* baseline

* fantomas

* release notes

* ilverify baselines

* Update tests/FSharp.Compiler.ComponentTests/Conformance/LexicalAnalysis/SymbolicOperators.fs

Yeah the comments are pointless.

Co-authored-by: Tomas Grosup <[email protected]>

* Feedback pt 1

* temp

* tests

* release notes

* debug

* Update DEVGUIDE.md

Co-authored-by: Petr <[email protected]>

---------

Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Petr <[email protected]>

Author: 3 people

.gitignore

@@ -129,4 +129,9 @@ tests/FSharp.Compiler.Service.Tests/FSharp.CompilerService.SurfaceArea.netstanda
 /tests/AheadOfTime/Trimming/output.txt
 *.svclog
 micro.exe
-positive.exe
+positive.exe
+/tests/FSharp.Compiler.ComponentTests/FSharpChecker/StandardError.txt
+/tests/FSharp.Compiler.ComponentTests/FSharpChecker/StandardOutput.txt
+
+# ilverify baseline result files
+*.bsl.actual

DEVGUIDE.md

@@ -204,6 +204,33 @@ Linux/macOS:
 export TEST_UPDATE_BSL=1
 ```
 
+## Retain Test run built artifacts
+
+When investigating tests issues it is sometimes useful to examine the artifacts built when running tests.  Those built using the newer test framework are usually,
+built in the %TEMP%\FSharp.Test.Utilities subdirectory.
+
+To tell the test framework to not cleanup these files use the: FSHARP_RETAIN_TESTBUILDS environment variable
+
+Windows:
+
+CMD:
+
+```shell
+set FSHARP_RETAIN_TESTBUILDS=1
+```
+
+PowerShell:
+
+```shell
+$env:FSHARP_RETAIN_TESTBUILDS=1
+```
+
+Linux/macOS:
+
+```shell
+export FSHARP_RETAIN_TESTBUILDS=1
+```
+
 Next, run a build script build (debug or release, desktop or coreclr, depending which baselines you need to update), and test as described [above](#Testing-from-the-command-line). For example:
 
 `./Build.cmd -c Release -testCoreClr` to update Release CoreCLR baselines.

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

@@ -1,5 +1,6 @@
 ### Fixed
 
+* Fix Realsig+ generates nested closures with incorrect Generic  ([Issue #17797](https://github.com/dotnet/fsharp/issues/17797), [PR #17877](https://github.com/dotnet/fsharp/pull/17877))
 * Fix missing TailCall warning in Sequential in use scope ([PR #17927](https://github.com/dotnet/fsharp/pull/17927))
 * Fix false negatives for passing null to "obj" arguments. Only "obj | null" can now subsume any type ([PR #17757](https://github.com/dotnet/fsharp/pull/17757))
 * Fix internal error when calling 'AddSingleton' and other overloads only differing in generic arity ([PR #17804](https://github.com/dotnet/fsharp/pull/17804))

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

@@ -0,0 +1,12 @@
+### Fixed
+
+* Fix Realsig+ generates nested closures with incorrect Generic  ([Issue #17797](https://github.com/dotnet/fsharp/issues/17797), [PR #17877](https://github.com/dotnet/fsharp/pull/17877))
+
+### Added
+
+
+### Changed
+
+
+### Breaking Changes
+

src/Compiler/CodeGen/IlxGen.fs

@@ -521,7 +521,8 @@ let ComputeTypeAccess (tref: ILTypeRef) hidden (accessibility: Accessibility) re
 
 /// Indicates how type parameters are mapped to IL type variables
 [<NoEquality; NoComparison>]
-type TypeReprEnv(reprs: Map<Stamp, uint16>, count: int, templateReplacement: (TyconRef * ILTypeRef * Typars * TyparInstantiation) option) =
+type TypeReprEnv
+    (reprs: Map<Stamp, (uint16 * Typar)>, count: int, templateReplacement: (TyconRef * ILTypeRef * Typars * TyparInstantiation) option) =
 
     static let empty = TypeReprEnv(count = 0, reprs = Map.empty, templateReplacement = None)
 
@@ -536,7 +537,7 @@ type TypeReprEnv(reprs: Map<Stamp, uint16>, count: int, templateReplacement: (Ty
     /// Lookup a type parameter
     member _.Item(tp: Typar, m: range) =
         try
-            reprs[tp.Stamp]
+            reprs[tp.Stamp] |> fst
         with :? KeyNotFoundException ->
             errorR (InternalError("Undefined or unsolved type variable: " + showL (typarL tp), m))
             // Random value for post-hoc diagnostic analysis on generated tree *
@@ -546,7 +547,7 @@ type TypeReprEnv(reprs: Map<Stamp, uint16>, count: int, templateReplacement: (Ty
     /// then it is ignored, since it doesn't correspond to a .NET type parameter.
     member tyenv.AddOne(tp: Typar) =
         if IsNonErasedTypar tp then
-            TypeReprEnv(reprs.Add(tp.Stamp, uint16 count), count + 1, templateReplacement)
+            TypeReprEnv(reprs.Add(tp.Stamp, (uint16 count, tp)), count + 1, templateReplacement)
         else
             tyenv
 
@@ -573,6 +574,14 @@ type TypeReprEnv(reprs: Map<Stamp, uint16>, count: int, templateReplacement: (Ty
     /// Get the environment for generating a reference to items within a type definition
     member eenv.ForTyconRef(tcref: TyconRef) = eenv.ForTycon tcref.Deref
 
+    /// Get a list of the Typars in this environment
+    member eenv.AsUserProvidedTypars() =
+        reprs
+        |> Map.toList
+        |> List.map (fun (_, (_, tp)) -> tp)
+        |> List.filter (fun tp -> not tp.IsCompilerGenerated)
+        |> Zset.ofList typarOrder
+
 //--------------------------------------------------------------------------
 // Generate type references
 //--------------------------------------------------------------------------
@@ -6904,14 +6913,6 @@ and GenFreevar cenv m eenvouter tyenvinner (fv: Val) =
 and GetIlxClosureFreeVars cenv m (thisVars: ValRef list) boxity eenv takenNames expr =
     let g = cenv.g
 
-    // Choose a base name for the closure
-    let basename =
-        let boundv = eenv.letBoundVars |> List.tryFind (fun v -> not v.IsCompilerGenerated)
-
-        match boundv with
-        | Some v -> v.CompiledName cenv.g.CompilerGlobalState
-        | None -> "clo"
-
     // Get a unique stamp for the closure. This must be stable for things that can be part of a let rec.
     let uniq =
         match expr with
@@ -6921,18 +6922,34 @@ and GetIlxClosureFreeVars cenv m (thisVars: ValRef list) boxity eenv takenNames
         | _ -> newUnique ()
 
     // Choose a name for the closure
-    let ilCloTypeRef =
+    let ilCloTypeRef, initialFreeTyvars =
+        let boundvar =
+            eenv.letBoundVars |> List.tryFind (fun v -> not v.IsCompilerGenerated)
+
+        let basename =
+            match boundvar with
+            | Some v -> v.CompiledName cenv.g.CompilerGlobalState
+            | None -> "clo"
+
         // FSharp 1.0 bug 3404: System.Reflection doesn't like '.' and '`' in type names
         let basenameSafeForUseAsTypename = CleanUpGeneratedTypeName basename
 
-        let suffixmark = expr.Range
-
         let cloName =
             // Ensure that we have an g.CompilerGlobalState
             assert (g.CompilerGlobalState |> Option.isSome)
-            g.CompilerGlobalState.Value.StableNameGenerator.GetUniqueCompilerGeneratedName(basenameSafeForUseAsTypename, suffixmark, uniq)
+            g.CompilerGlobalState.Value.StableNameGenerator.GetUniqueCompilerGeneratedName(basenameSafeForUseAsTypename, expr.Range, uniq)
+
+        let ilCloTypeRef = NestedTypeRefForCompLoc eenv.cloc cloName
 
-        NestedTypeRefForCompLoc eenv.cloc cloName
+        let initialFreeTyvars =
+            match g.realsig with
+            | true ->
+                { emptyFreeTyvars with
+                    FreeTypars = eenv.tyenv.AsUserProvidedTypars()
+                }
+            | false -> emptyFreeTyvars
+
+        ilCloTypeRef, initialFreeTyvars
 
     // Collect the free variables of the closure
     let cloFreeVarResults =
@@ -6943,7 +6960,12 @@ and GetIlxClosureFreeVars cenv m (thisVars: ValRef list) boxity eenv takenNames
             | None -> opts
             | Some(tcref, _, typars, _) -> opts.WithTemplateReplacement(tyconRefEq g tcref, typars)
 
-        freeInExpr opts expr
+        accFreeInExpr
+            opts
+            expr
+            { emptyFreeVars with
+                FreeTyvars = initialFreeTyvars
+            }
 
     // Partition the free variables when some can be accessed from places besides the immediate environment
     // Also filter out the current value being bound, if any, as it is available from the "this"

src/Compiler/TypedTree/CompilerGlobalState.fs

@@ -73,4 +73,6 @@ let newUnique() = System.Threading.Interlocked.Increment &uniqueCount
 /// Unique name generator for stamps attached to to val_specs, tycon_specs etc.
 //++GLOBAL MUTABLE STATE (concurrency-safe)
 let mutable private stampCount = 0L
-let newStamp() = System.Threading.Interlocked.Increment &stampCount
+let newStamp() =
+    let stamp = System.Threading.Interlocked.Increment &stampCount
+    stamp

src/Compiler/TypedTree/TypedTreeOps.fsi

@@ -1186,6 +1186,9 @@ val accFreeInDecisionTree: FreeVarOptions -> DecisionTree -> FreeVars -> FreeVar
 /// Get the free variables in a module definition.
 val freeInModuleOrNamespace: FreeVarOptions -> ModuleOrNamespaceContents -> FreeVars
 
+/// Get the free variables in an expression with accumulator
+val accFreeInExpr: FreeVarOptions -> Expr -> FreeVars -> FreeVars
+
 /// Get the free variables in an expression.
 val freeInExpr: FreeVarOptions -> Expr -> FreeVars
 

tests/FSharp.Compiler.ComponentTests/Conformance/LexicalAnalysis/SymbolicOperators.fs

@@ -31,11 +31,38 @@ module SymbolicOperators =
 
     // This test was automatically generated (moved from FSharpQA suite - Conformance/LexicalAnalysis/SymbolicOperators)
     //<Expects status="error" id="FS0670">This code is not sufficiently generic\. The type variable  \^T when  \^T : \(static member \( \+ \) :  \^T \*  \^T ->  \^a\) could not be generalized because it would escape its scope</Expects>
-    [<Theory; Directory(__SOURCE_DIRECTORY__ + "/../../resources/tests/Conformance/LexicalAnalysis/SymbolicOperators", Includes=[|"E_LessThanDotOpenParen001.fs"|])>]
-    let ``SymbolicOperators - E_LessThanDotOpenParen001_fs - --flaterrors`` compilation =
-        compilation
-        |> asFsx
+    [<InlineData(true)>]            // RealSig
+    [<InlineData(false)>]           // Regular
+    [<Theory>]
+    let ``SymbolicOperators_E_LessThanDotOpenParen001_fs`` (realsig) =
+        Fsx """
+
+type public TestType<'T,'S>() =
+    
+    member public s.Value with get() = Unchecked.defaultof<'T>
+    static member public (+++) (a : TestType<'T,'S>, b : TestType<'T,'S>) = a.Value
+    static member public (+++) (a : TestType<'T,'S>, b : 'T) = b
+    static member public (+++) (a : 'T, b : TestType<'T,'S>) = a
+    static member public (+++) (a : TestType<'T,'S>, b : 'T -> 'S) = a.Value
+    static member public (+++) (a : 'S -> 'T, b : TestType<'T,'S>) = (a 17) + b.Value
+
+let inline (+++) (a : ^a) (b : ^b) = ((^a or ^b): (static member (+++): ^a * ^b -> ^c) (a,b) )
+
+let tt0 = TestType<int, string>()
+let tt1 = TestType<int, string>()
+
+let f (x : string) = 18
+
+let a0 = tt0 +++ tt1
+let a1 = tt0 +++ 11
+let a2 =  12 +++ tt1
+let a3 = tt0 +++ (fun x -> "18")
+let a4 = f +++ tt0
+
+let a5 = TestType<int, string>.(+++)(f, tt0)
+let a6 = TestType<int, string>.(+++)((fun (x : string) -> 18), tt0)"""
         |> withOptions ["--flaterrors"]
+        |> withRealInternalSignature realsig
         |> compile
         |> shouldFail
         |> withErrorCode 0670