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 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..61b66c956aa 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/RecordTypes.fs @@ -591,4 +591,28 @@ 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") + + [] + 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 8e2939c22fe..07594cc497d 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/StructTypes/StructTypes.fs @@ -54,3 +54,58 @@ 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") + + [] + 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") + + [] + 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") diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs index 18637179b2a..ed1e358fd6f 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,29 @@ 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 ``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 =