From e173e343036d9966eaddaf74749e315042fe0e05 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Sat, 25 Oct 2025 12:18:02 -0400 Subject: [PATCH 1/4] Disallow recursive structs with lifted type parameters --- src/Compiler/Checking/CheckDeclarations.fs | 6 +++++- .../Conformance/Types/RecordTypes/RecordTypes.fs | 14 +++++++++++++- .../Conformance/Types/StructTypes/StructTypes.fs | 12 ++++++++++++ .../Types/UnionTypes/UnionStructTypes.fs | 13 ++++++++++++- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index cbf79fbdfb3..86b780b251c 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -3961,8 +3961,12 @@ module EstablishTypeDefinitionCores = // collect edges from the fields of a given struct type. and accStructFields includeStaticFields ty (structTycon: Tycon) tinst (doneTypes, acc) = - if List.exists (typeEquiv g ty) doneTypes then + if // This type (type instance) has been seen before, so no need to collect the same edges again (and avoid loops!) + List.exists (typeEquiv g ty) doneTypes + // This tycon is the outer tycon with a lifted generic argument, e.g., `'a list`. + || not (isNil doneTypes) && tryTcrefOfAppTy g ty |> ValueOption.bind _.TryDeref |> ValueOption.exists ((===) tycon) + then doneTypes, acc else // Only collect once from each type instance. diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs index da18bda2f33..e530a1d0f25 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs @@ -591,4 +591,16 @@ module RecordTypes = (Error 668, Line 5, Col 9, Line 5, Col 10, "The field 'A' appears multiple times in this record expression or pattern") (Error 668, Line 5, Col 24, Line 5, Col 25, "The field 'A' appears multiple times in this record expression or pattern") (Error 668, Line 5, Col 31, Line 5, Col 32, "The field 'B' appears multiple times in this record expression or pattern") - ] \ No newline at end of file + ] + + [] + let ``Cyclic reference check works for recursive reference with a lifted generic argument`` () = + Fsx + """ + namespace Foo + [] + type NestedRecord<'a> = { A : int; B : NestedRecord<'a list> } + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 30, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs index 8e2939c22fe..af309c971ce 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs @@ -54,3 +54,15 @@ module StructTypes = (Error 946, Line 6, Col 12, Line 6, Col 20, "Cannot inherit from interface type. Use interface ... with instead.") ] + [] + let ``Cyclic reference check works for recursive reference with a lifted generic argument`` () = + Fsx + """ + namespace Foo + [] + type NestedRegularStruct<'a> = val A : NestedRegularStruct<'a list> + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 37, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs index 18637179b2a..5192a64a6c5 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs @@ -1,4 +1,4 @@ -namespace Conformance.Types +namespace Conformance.Types open Xunit open FSharp.Test.Compiler @@ -675,6 +675,17 @@ type StructUnion = A of X:int | B of Y:StructUnion (Error 954, Line 4, Col 6, Line 4, Col 17, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") ] + [] + let ``Cyclic reference check works for recursive reference with a lifted generic argument`` () = + Fsx + """ + namespace Foo + [] + type NestedUnion<'a> = Cons of 'a * NestedUnion<'a list> | Nil + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 29, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") let createMassiveStructDuProgram countOfCases = let codeSb = From cf2d872157474255e975668f1a1f3009606b354b Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Sat, 25 Oct 2025 12:26:05 -0400 Subject: [PATCH 2/4] Update release notes --- docs/release-notes/.FSharp.Compiler.Service/11.0.0.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md index 655ad02ad2a..219ee92ba9f 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md @@ -6,6 +6,7 @@ * Fix name is bound multiple times is not reported in 'as' pattern ([PR #18984](https://github.com/dotnet/fsharp/pull/18984)) * Fix: warn FS0049 on upper union case label. ([PR #19003](https://github.com/dotnet/fsharp/pull/19003)) * Type relations cache: handle potentially "infinite" types ([PR #19010](https://github.com/dotnet/fsharp/pull/19010)) +* Disallow recursive structs with lifted type parameters ([Issue #18993](https://github.com/dotnet/fsharp/issues/18993), [PR #19031](https://github.com/dotnet/fsharp/pull/19031)) ### Added @@ -13,4 +14,4 @@ * Parallel compilation stabilised and enabled by default ([PR #18998](https://github.com/dotnet/fsharp/pull/18998)) -### Breaking Changes \ No newline at end of file +### Breaking Changes From fdbaa664e2b162a7d6dc3210a34e47a97129d33c Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Sat, 25 Oct 2025 22:33:09 -0400 Subject: [PATCH 3/4] fsi --- .../Conformance/Types/RecordTypes/RecordTypes.fs | 12 ++++++++++++ .../Conformance/Types/StructTypes/StructTypes.fs | 12 ++++++++++++ .../Conformance/Types/UnionTypes/UnionStructTypes.fs | 12 ++++++++++++ 3 files changed, 36 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs index e530a1d0f25..61b66c956aa 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs @@ -604,3 +604,15 @@ module RecordTypes = |> typecheck |> shouldFail |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 30, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + + [] + let ``Cyclic reference check works for recursive reference with a lifted generic argument: signature`` () = + Fsi + """ + namespace Foo + [] + type NestedRecord<'a> = { A : int; B : NestedRecord<'a list> } + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 30, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs index af309c971ce..a67046d8d5d 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs @@ -66,3 +66,15 @@ module StructTypes = |> shouldFail |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 37, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + [] + let ``Cyclic reference check works for recursive reference with a lifted generic argument: signature`` () = + Fsi + """ + namespace Foo + [] + type NestedRegularStruct<'a> = val A : NestedRegularStruct<'a list> + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 37, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs index 5192a64a6c5..ed1e358fd6f 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs @@ -687,6 +687,18 @@ type StructUnion = A of X:int | B of Y:StructUnion |> shouldFail |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 29, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + [] + let ``Cyclic reference check works for recursive reference with a lifted generic argument: signature`` () = + Fsi + """ + namespace Foo + [] + type NestedUnion<'a> = Cons of 'a * NestedUnion<'a list> | Nil + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 29, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + let createMassiveStructDuProgram countOfCases = let codeSb = System.Text.StringBuilder(""" From ca03e8f2297beed8356889112f240a4c2d925bf8 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Mon, 27 Oct 2025 20:30:58 -0400 Subject: [PATCH 4/4] Add test showing that #6069 is fixed --- .../Types/StructTypes/StructTypes.fs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs index a67046d8d5d..07594cc497d 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs @@ -78,3 +78,34 @@ module StructTypes = |> shouldFail |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 37, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + [] + let ``Cyclic reference check works for mutually-recursive reference with a lifted generic argument`` () = + Fsx + """ + namespace Foo + [] + type MyStruct<'T> = + val field : YourStruct>> + + and [] YourStruct<'T> = + val field : 'T + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 26, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation") + + [] + let ``Cyclic reference check works for mutually-recursive reference with a lifted generic argument: signature`` () = + Fsi + """ + namespace Foo + [] + type MyStruct<'T> = + val field : YourStruct>> + + and [] YourStruct<'T> = + val field : 'T + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Error 954, Line 4, Col 18, Line 4, Col 26, "This type definition involves an immediate cyclic reference through a struct field or inheritance relation")