Skip to content

Reject recursive user-defined types at type-check time - #3546

Open
Ian Davis (idavis) wants to merge 2 commits into
mainfrom
iadavis/disallow-recursive-tys
Open

Reject recursive user-defined types at type-check time#3546
Ian Davis (idavis) wants to merge 2 commits into
mainfrom
iadavis/disallow-recursive-tys

Conversation

@idavis

@idavis Ian Davis (idavis) commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

What this fixes

Closes #911.

Today you can write a Q# type that contains itself:

struct A { B : A }

Nothing stops you at compile time. The compiler accepts it and then falls over later while trying to work with a type that has no bottom — in the worst case the process dies with a stack overflow and no error message at all. That is a bad experience: no file, no line, no explanation.

This PR catches the problem where it belongs, in the type checker, and reports a normal error pointing at the field that closes the loop.

What counts as recursive

A type is rejected if it can reach itself by any path, including indirectly:

  • directly: struct A { B : A }
  • through another type: A holds a B, and B holds an A
  • around a longer chain: Leaf => Mid => Top => Leaf

Wrapping does not help. An array of itself, a tuple containing itself, or a function or operation type that takes or returns itself are all rejected.

For the value-carrying cases the reason is size: a Foo that contains a Foo[] or a (Foo, Int) has no finite layout, so there is no bottom to the type.

Function and operation types are a separate case. A Foo -> Foo field does not by itself give Foo infinite size, the signature is just a reference to the type, not an embedded copy of it. The check rejects it anyway, deliberately: it walks every type mentioned by a field, including the inputs and outputs of arrow types, and does not carve out an exception for them. Allowing arrows would mean admitting a narrow, hard-to-explain class of self-referential types that the rest of the pipeline is not built to handle. Rejecting them keeps the rule one sentence long, a type may not reach itself, and leaves the door open to relaxing it later if there is real demand.

The error names the field that closes the cycle and states that arrays, tuples, and arrow types still count as recursive, so the reason Foo[] was rejected is visible in the message itself.

What still works

Ordinary nesting is untouched. A type may contain other types as deeply as you like, may use arrays and function types, and may reference types from libraries you depend on — as long as nothing eventually points back at itself.

Breaking change

This is a language-level change: code that compiled before will now fail to compile. In practice such code could not run correctly anyway, it either crashed or misbehaved downstream, so the realistic impact is turning a crash into a readable error. Still, it needs a release note, and I'd flag it for the release/communication owner rather than letting it ship silently.

Behavior notes worth a reviewer's attention

  • Every member of a cycle is reported, not just one, since each declaration is checked independently. If a single diagnostic per cycle is preferred, that is a deliberate follow-up, not an oversight.
  • A type with several self-referencing fields produces one error, not one per field.
  • In the REPL, the error is reported once and does not re-fire on later fragments.

@domorale

Copy link
Copy Markdown

The added coverage looks good to me: it already exercises direct recursion, recursion through arrays and callables, mutual recursion, longer cycles, duplicate-field reporting, use-site behavior, acyclic cases, and the diagnostic code.

I think it would be worth adding a couple more tests before merging, mainly to cover cases where the cycle is not just a same-namespace/simple-shape recursion. In decreasing order of relevance:

  1. Cross-namespace mutual recursion

This seems like the most important missing case to me, since it would help ensure the cycle detection is based on resolved item identities rather than accidentally relying on same-namespace or unqualified-name behavior.

namespace A {
    struct Foo { B : B.Bar }
}

namespace B {
    struct Bar { F : A.Foo }
}
  1. Mutual recursion through nested callable/tuple types
     
    This would check that dependency discovery walks through callable return types and tuple components when building the recursive UDT graph, not only direct field types.
     
namespace A {
struct Foo { F : Int -> (Bool, Bar) }
struct Bar { F : Int -> (Bool, Foo) }
}
  1. Tuple-only recursion

The current Tree case covers recursion through a tuple field and an array together. A minimal tuple-only case would isolate tuple traversal more directly.

namespace A {
    newtype Foo = (Int, Foo);
}
  1. Multiple independent recursive components

This would verify that diagnostics are emitted for all independent cycles in the same compilation, not just the first recursive component encountered.

namespace A {
    struct Foo { B : Bar }
    struct Bar { F : Foo }

    struct Baz { Q : Qux }
    struct Qux { Z : Baz }
}
  1. Nested array recursion

Lower priority, but cheap coverage for repeated type constructors.

namespace A {
    struct Foo { Children : Foo[][] }
}

If we only add one more test, I think the cross-namespace mutual recursion case gives the most additional confidence beyond the existing coverage.

@idavis

Copy link
Copy Markdown
Collaborator Author

Domingo Morales Lizama (@domorale) Good ideas, I'll add the tests

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for adding the tests. They look good to me.

@amcasey Andrew Casey (amcasey) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The break seems acceptable to me and I couldn't find loopholes in local testing. I haven't reviewed the code in detail, but we've discussed the approach.

Also, if we lift this restriction later, that change won't be breaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disallow or Warn Recursive UDT's

3 participants