Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 27 additions & 0 deletions DEVGUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,33 @@ Linux/macOS:
export TEST_UPDATE_BSL=1
```

## Retain Test run built artifacts

When investigating tests issues it is sometimes usefull to examine the artifacts built whe running tests. Those built using the newer test framework are usually,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When investigating tests issues it is sometimes usefull to examine the artifacts built whe running tests. Those built using the newer test framework are usually,
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.
Expand Down
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
4 changes: 2 additions & 2 deletions eng/ilverify_FSharp.Compiler.Service_Release_net9.0.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
[IL]: Error [StackUnexpected]: : FSharp.Compiler.ParseAndCheckInputs+CheckMultipleInputsUsingGraphMode@1865::Invoke(int32)][offset 0x0000003A][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.CompilerConfig+TcConfig::.ctor([FSharp.Compiler.Service]FSharp.Compiler.CompilerConfig+TcConfigBuilder, bool)][offset 0x0000059C][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.CompilerConfig+TcConfig::.ctor([FSharp.Compiler.Service]FSharp.Compiler.CompilerConfig+TcConfigBuilder, bool)][offset 0x000005A5][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000011][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1850-1<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,int32>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000012][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1850<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,T0>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000011][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1859-1<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,int32>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000012][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1859<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,T0>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.PatternMatchCompilation::isProblematicClause([FSharp.Compiler.Service]FSharp.Compiler.PatternMatchCompilation+MatchClause)][offset 0x00000040][found Byte] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$FSharp.Compiler.PatternMatchCompilation::.cctor()][offset 0x0000000B][found Boolean] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.TypeProviders::ValidateExpectedName([FSharp.Compiler.Service]FSharp.Compiler.Text.Range, string[], string, [FSharp.Compiler.Service]FSharp.Compiler.Tainted`1<FSharp.Compiler.TypeProviders+ProvidedType>)][offset 0x000000A8][found Char] Unexpected type on the stack.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
[IL]: Error [StackUnexpected]: : FSharp.Compiler.CompilerImports+TcConfig-TryResolveLibWithDirectories@558-1::Invoke([FSharp.Core]Microsoft.FSharp.Core.Unit)][offset 0x0000003B][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.CompilerConfig+TcConfig::.ctor([FSharp.Compiler.Service]FSharp.Compiler.CompilerConfig+TcConfigBuilder, bool)][offset 0x0000059C][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.CompilerConfig+TcConfig::.ctor([FSharp.Compiler.Service]FSharp.Compiler.CompilerConfig+TcConfigBuilder, bool)][offset 0x000005A5][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000011][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1850-1<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,int32>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000012][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1850<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,T0>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000011][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1859-1<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,int32>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.IlxGen::HashRangeSorted([S.P.CoreLib]System.Collections.Generic.IDictionary`2<!!0,System.Tuple`2<int32,!!1>>)][offset 0x00000012][found ref '[FSharp.Compiler.Service]FSharp.Compiler.IlxGen+HashRangeSorted@1859<T1>'][expected ref '[FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`2<int32,T0>,T0>'] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.PatternMatchCompilation::isProblematicClause([FSharp.Compiler.Service]FSharp.Compiler.PatternMatchCompilation+MatchClause)][offset 0x00000040][found Byte] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$FSharp.Compiler.PatternMatchCompilation::.cctor()][offset 0x0000000B][found Boolean] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : FSharp.Compiler.NicePrint+TastDefinitionPrinting+meths@2092-3::Invoke([FSharp.Compiler.Service]FSharp.Compiler.Infos+MethInfo)][offset 0x000000B3][found Char] Unexpected type on the stack.
Expand Down
56 changes: 39 additions & 17 deletions src/Compiler/CodeGen/IlxGen.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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 *
Expand All @@ -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

Expand All @@ -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
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -6892,14 +6901,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
Expand All @@ -6909,18 +6910,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 =
Expand All @@ -6931,7 +6948,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"
Expand Down
4 changes: 3 additions & 1 deletion src/Compiler/TypedTree/CompilerGlobalState.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions src/Compiler/TypedTree/TypedTreeOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,43 @@ 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 """// #Regression #Conformance #LexicalAnalysis #Operators
// Regression test for FSHARP1.0:4805
// We are not really after the actual error messages here (some of them have been omitted), rather we
// want to verify we do not crash!
//<Expects status="warning" id="FS0064">This construct causes code to be less generic than indicated by the type annotations\. The type variable 'S has been constrained to be type 'int'</Expects>
//<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>
KevinRansom marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
Loading
Loading