Skip to content

Fix #19596: unit-parameter member conformance with signature#19615

Open
T-Gro wants to merge 44 commits intosig-roundtrip-sweepfrom
fix/unit-member-conformance
Open

Fix #19596: unit-parameter member conformance with signature#19615
T-Gro wants to merge 44 commits intosig-roundtrip-sweepfrom
fix/unit-member-conformance

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented Apr 19, 2026 •
edited
Loading

Fixes #19596

Stacked on #19609.

member M(()) produces ValReprInfo [[]] (empty arg group), but sig member M: unit -> unit parses to [[unit_arg]]. Both produce identical IL (.method instance void M()). The conformance checker rejected them because TotalArgCount differs (1 vs 2 including this).

Fix: Relax signature conformance matching for the unit-parameter case:

  • checkValInfo: allow empty impl group [] to match singleton sig group [_]
  • Overloaded matching: fallback using typeAEquivAux EraseAll + TotalArgCount diff=1 guard
  • Single-val [av],[fv] path: same relaxation

Safety:

  • IL verified identical via verifyILContains test
  • EraseAll matches the erasure mode used by valLinkageAEquiv in the primary matching path
  • TotalArgCount diff=1 guard restricts to unit-param case only
  • Existing E_StructConstructor01 conformance test still correctly fails
  • Both directions tested with consumer: sig+impl+consumer compiles correctly

T-Gro and others added 17 commits April 17, 2026 15:45
@T-Gro
Add failing test: namespace global roundtrip (found by corpus sweep)
475b111 
Swept 1483 .fs files from tests/fsharp/ and tests/service/data/.
Results: 89 pass, 1 real sig-gen failure, 159 reference-resolution
issues (test infra limitation), 1234 skip (source errors).

The one real bug: 'namespace global' + class type generates an
unparseable signature (FS0010). Marked as Skip for tracking.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add sig-gen roundtrip failures from corpus sweep (positive code only)
cf2775c 
Swept 1483 standalone .fs files using local fsc with full BCL refs.
Results: 274 pass, 6 legit failures, 4 negative-test (excluded),
4 namespace-global (known), 2 infra, 1188 skip.

The 6 legit failures in positive test code:
- pos36-srtp-lib: SRTP multi-witness constraint lost (FS0340)
- pos35: SRTP constraint mismatch (FS0340)
- pos34: type application syntax error in sig (FS0010)
- pos16: unexpected identifier in value sig (FS0010)
- pos08: member missing from sig (FS0193)
- fsfromfsviacs/lib: DU base type mismatch (FS0300)

Added 3 representative skipped tests covering distinct bug categories.
Negative tests (neg*.fs) excluded — broken code is not expected to
produce valid signatures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19597: single-case struct DU gets spurious bar in signature
1f36673 
For single-case struct DUs like [<Struct>] type U0 = U0, the bar prefix
changes the base type semantics (FS0300). Only omit the bar for
single-case unions when the type is a struct. Non-struct single-case
unions keep the bar to avoid parse errors in edge cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19592: backticked active pattern case names lose escaping in sig
34e3868 
Active pattern case names containing spaces were rendered without
backtick escaping in generated signatures, producing unparseable
output. Fix: in ConvertValLogicalNameToDisplayNameCore, split active
pattern names on bar, backtick-escape each case name that is not a
valid identifier, rejoin.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19595: type params with special chars get backtick escaping
66a2bc5 
Type parameters with names containing angle brackets (e.g. Monad<'T>
from SRTP) are now backtick-escaped in layoutTyparRef using
NormalizeIdentifierBackticks. This prevents parse errors when such
type params appear in generated signatures via GenerateSignature.

Note: the --sig flag uses a separate code path (SignatureWriter) that
is not fixed by this change. Only the FCS GenerateSignature API path
benefits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19593: namespace global dropped from generated signature
985f830 
In GenerateSignature, detect when the typed tree has types directly
at root level (TMDefRec with tycons, not wrapped in a Module binding)
and prepend 'namespace global' to the layout output.

Simple case (types only) roundtrips. Complex case (types + nested
module) needs further layout investigation and is tracked as skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add skipped test for #19596: overloaded member with unit parameter
91f19b1 
Generated sig text is CORRECT (fsc --sig roundtrips fine).
The failure is specific to the FCS Compile API path used by the
test infrastructure. This is an FCS conformance issue, not a sig
generation bug.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19594: SRTP constraints use explicit type param syntax in signat…
8325bc8 
…ures

For inline functions/members with statically resolved type parameters,
use explicit type param declarations (M< ^T when constraint >) instead
of postfix constraints (M: ... when constraint). The postfix form is
not accepted by the conformance checker for SRTP constraints.

Changes:
- layoutMemberName: trigger layoutTyparDecls when SRTP typars present
- prettyLayoutOfValOrMemberNoInst: same for module-level vals
- prettyLayoutOfCurriedMemberSig: exclude SRTP typar constraints from
  postfix position when they're on explicit type param declarations
- Operator names excluded (can't have explicit type params in .fsi)
- Updated baseline for type extension SRTP test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add tooltip tests for signature generation display changes
72fe41f 
Verify that tooltip/quickinfo display is correct after sig-gen fixes:
- Backticked active pattern case names preserved in tooltip
- SRTP inline function type params displayed with requires clause
- Single-case struct DU displayed without bar prefix
- Inline function type params shown properly

These tests explicitly cover the display-layer leaks identified
by the isolation audit, confirming all are improvements.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Remove sweep tooling (investigation artifacts, not production test in…
3aca69b 
…fra)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add skipped test for #19596: overloaded member with unit parameter
6f0cda6 
Roundtrip fails: member M(()) generates sig 'member M: unit -> unit'
but conformance checker can't match it when M is overloaded.
Proven NOT FCS-specific: fsc CLI also fails when .fsi/.fs are paired.
The sig text is correct; the bug is in SignatureConformance matching
for unit-parameter overloads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add missing roundtrip test for #19595: type param backtick escaping
ec0dd10 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix namespace global + module layout and unskip 3 passing tests
8784370 
- Move namespace global detection from FSharpCheckerResults.fs into
  NicePrint.layoutImpliedSignatureOfModuleOrNamespace so all callers
  (FCS API, fsc --sig, FSI) benefit
- Fix module rendering inside namespace global: modules were rendered
  without '=' and indentation because empty outerPath was treated as
  standalone module. Now check isGlobalNamespace flag.
- Unskip pos34 (type application) and pos16 (multi-case AP) tests —
  already fixed by SRTP and backtick escaping changes
- Unskip namespace global + nested module test — now fixed

Remaining skip: #19596 (unit param overload conformance bug)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add release notes for additional signature generation fixes
36ba04a 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Merge branch 'main' into sig-roundtrip-sweep
9e388f8
@T-Gro
Fix formatting in PrettyNaming.fs
5570959 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix PR numbers in release notes
53b9e61 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro requested a review from a team as a code owner April 19, 2026 17:23
@T-Gro T-Gro changed the base branch from sig-roundtrip-sweep to main April 19, 2026 17:30
@T-Gro T-Gro changed the base branch from main to sig-roundtrip-sweep April 19, 2026 17:37
T-Gro and others added 5 commits April 20, 2026 15:32
@T-Gro
Fix CI failures from signature roundtrip changes
4a0fc41 
- NicePrint: null-safe typar name handling in layoutTyparRef to avoid
  NRE when rendering anonymous typars during overload resolution.
- NicePrint: deduplicate SRTP constraints in layoutNonMemberVal so they
  appear only in typar decl brackets, not duplicated in the postfix
  'when' clause.
- PrettyNaming: revert active-pattern case backtick escaping from
  ConvertValLogicalNameToDisplayNameCore (which is also used by
  tryParseActivePatternName parsing path). Apply backtick escaping only
  at final display layer via new escapeActivePatternCaseNames helper in
  ConvertValLogicalNameToDisplayName and ConvertValLogicalNameToDisplayLayout.
- Update IWSAMsAndSRTPs signature test expectations to match new
  val inline f<^T ...> : ... format.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19596: overloaded member with unit parameter conformance check
c075528 
SignatureConformance.fs: when matching overloaded members, add relaxed
fallback for unmatched pairs using type equivalence. This handles
member M(()) (argInfos=[[]]) vs sig member M: unit->unit (argInfos=[[unit_arg]])
where types are equivalent but TotalArgCount differs.

Also relax checkValInfo arg group check: empty impl group is compatible
with singleton sig group for unit-parameter members.

Tests: roundtrip test, handwritten sig+impl+consumer (both directions).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Update release notes: add #19596 fix, fix PR numbers
c373cc0 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add IL verification: M(()) and M() produce identical IL method signat…
03521b2 
…ures

Proves the conformance relaxation is safe at IL level — both member M(())
and member M() compile to '.method public hidebysig instance int32 M()',
confirming the representation difference is compile-time only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add inverse conformance tests for unit param overload
e4bc6ae 
- Consumer cannot call d.M() when impl is M(()) with overloads (FS0503 expected)
- Consumer can call d.M() when impl is M() with sig M: unit -> unit

Covers both directions of sig↔impl conformance with consumer validation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the fix/unit-member-conformance branch from e8e6743 to e4bc6ae Compare April 20, 2026 13:50
@T-Gro
Fix release note PR reference and add trailing newline
9e5e4d3 
- Release note for #19596 now correctly references PR #19615 (not #19609)
- Add missing trailing newline to TypeTests.fs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the sig-roundtrip-sweep branch from 4a0fc41 to c31cc8f Compare April 20, 2026 15:22
T-Gro and others added 16 commits April 20, 2026 20:38
@T-Gro
Fix #19592: backticked active pattern case names lose escaping in sig
6e3c142 
Active pattern case names containing spaces were rendered without
backtick escaping in generated signatures, producing unparseable
output. Fix: in ConvertValLogicalNameToDisplayNameCore, split active
pattern names on bar, backtick-escape each case name that is not a
valid identifier, rejoin.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19595: type params with special chars get backtick escaping
bbaac92 
Type parameters with names containing angle brackets (e.g. Monad<'T>
from SRTP) are now backtick-escaped in layoutTyparRef using
NormalizeIdentifierBackticks. This prevents parse errors when such
type params appear in generated signatures via GenerateSignature.

Note: the --sig flag uses a separate code path (SignatureWriter) that
is not fixed by this change. Only the FCS GenerateSignature API path
benefits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19593: namespace global dropped from generated signature
9e56e09 
In GenerateSignature, detect when the typed tree has types directly
at root level (TMDefRec with tycons, not wrapped in a Module binding)
and prepend 'namespace global' to the layout output.

Simple case (types only) roundtrips. Complex case (types + nested
module) needs further layout investigation and is tracked as skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add skipped test for #19596: overloaded member with unit parameter
b129563 
Generated sig text is CORRECT (fsc --sig roundtrips fine).
The failure is specific to the FCS Compile API path used by the
test infrastructure. This is an FCS conformance issue, not a sig
generation bug.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19594: SRTP constraints use explicit type param syntax in signat…
b639193 
…ures

For inline functions/members with statically resolved type parameters,
use explicit type param declarations (M< ^T when constraint >) instead
of postfix constraints (M: ... when constraint). The postfix form is
not accepted by the conformance checker for SRTP constraints.

Changes:
- layoutMemberName: trigger layoutTyparDecls when SRTP typars present
- prettyLayoutOfValOrMemberNoInst: same for module-level vals
- prettyLayoutOfCurriedMemberSig: exclude SRTP typar constraints from
  postfix position when they're on explicit type param declarations
- Operator names excluded (can't have explicit type params in .fsi)
- Updated baseline for type extension SRTP test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add tooltip tests for signature generation display changes
c90c6e4 
Verify that tooltip/quickinfo display is correct after sig-gen fixes:
- Backticked active pattern case names preserved in tooltip
- SRTP inline function type params displayed with requires clause
- Single-case struct DU displayed without bar prefix
- Inline function type params shown properly

These tests explicitly cover the display-layer leaks identified
by the isolation audit, confirming all are improvements.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Remove sweep tooling (investigation artifacts, not production test in…
7958943 
…fra)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add skipped test for #19596: overloaded member with unit parameter
21cf652 
Roundtrip fails: member M(()) generates sig 'member M: unit -> unit'
but conformance checker can't match it when M is overloaded.
Proven NOT FCS-specific: fsc CLI also fails when .fsi/.fs are paired.
The sig text is correct; the bug is in SignatureConformance matching
for unit-parameter overloads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add missing roundtrip test for #19595: type param backtick escaping
6f09ad5 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix namespace global + module layout and unskip 3 passing tests
1394182 
- Move namespace global detection from FSharpCheckerResults.fs into
  NicePrint.layoutImpliedSignatureOfModuleOrNamespace so all callers
  (FCS API, fsc --sig, FSI) benefit
- Fix module rendering inside namespace global: modules were rendered
  without '=' and indentation because empty outerPath was treated as
  standalone module. Now check isGlobalNamespace flag.
- Unskip pos34 (type application) and pos16 (multi-case AP) tests —
  already fixed by SRTP and backtick escaping changes
- Unskip namespace global + nested module test — now fixed

Remaining skip: #19596 (unit param overload conformance bug)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add release notes for additional signature generation fixes
d4781a0 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix formatting in PrettyNaming.fs
4401842 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix PR numbers in release notes
ef3d97c 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix CI failures: NRE null guard, SRTP constraint dedup, active patter…
4b01962 
…n escaping scope

- NicePrint.fs: Add null guard in layoutTyparRef for NormalizeIdentifierBackticks
  (typar.Name can be null for synthetic type parameters during SRTP resolution)
- NicePrint.fs: Filter SRTP constraints from postfix in layoutNonMemberVal
  (prevents constraint duplication in let-bound SRTP value signatures)
- PrettyNaming.fs: Move active pattern case escaping from ConvertValLogicalNameToDisplayNameCore
  to display-only functions (EscapeActivePatternCases) to avoid breaking name resolution
- Update SRTP test expectations for new explicit type param format
- Update SurfaceArea baseline for new EscapeActivePatternCases API
- Remove tooltip test for backtick AP names (QuickParse limitation)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix tooltip display: skip prefix SRTP form in shortConstraints mode
405668d 
The hasStaticallyResolvedTypars prefix display should only affect
.fsi signature output, not tooltips. Tooltips use shortConstraints=true
and should keep the existing '(requires member Sin)' postfix format.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Update baselines for SRTP prefix constraint display format
3608686 
Update 6 baseline files to reflect the new SRTP constraint display:
- neg117.bsl: Transform method with (TargetX or ^f) constraints
- neg119a.bsl, neg119b.bsl: Ap.Return with ^R constraints
- neg131.bsl, neg132.bsl: SomeMethod with ^T member constraints
- neg_known_return_type_and_known_type_arguments.bsl: Zero overloads

SRTP constraints now appear in prefix form on members:
  old: Method: params -> ret when ^T: (constraint)
  new: Method<^T when ^T: (constraint)> : params -> ret

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the sig-roundtrip-sweep branch from 4b5e839 to 3608686 Compare April 20, 2026 18:38
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Apr 21, 2026

@copilot resolve the merge conflicts in this pull request

@T-Gro
Merge origin/sig-roundtrip-sweep: resolve conflicts in NicePrint.fs, …
66e6da3 
…PrettyNaming.fs, TypeTests.fs, TooltipTests.fs

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit 66e6da3. Here's what was done for each conflict:

  • NicePrint.fs: Took base branch's cleaner Option.defaultValue/IsNullOrEmpty null handling
  • PrettyNaming.fs: Took base branch's public EscapeActivePatternCases and removed the redundant second escape application from my branch's private escapeActivePatternCaseNames
  • TypeTests.fs: Kept my HEAD tests (the unit-param conformance fix tests) and removed the Skip attribute from the roundtrip test since the issue is now fixed
  • TooltipTests.fs: Took base branch's removal of the infeasible backtick tooltip test

Build verified: 0 warnings/errors.

@T-Gro T-Gro force-pushed the sig-roundtrip-sweep branch from 3608686 to 8923561 Compare April 28, 2026 09:15
@T-Gro T-Gro requested a review from abonie April 28, 2026 09:56
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Apr 28, 2026

@copilot resolve the merge conflicts in this pull request

@T-Gro
Merge origin/sig-roundtrip-sweep: make EscapeActivePatternCases inter…
e1bf547 
…nal, add tooltip/method-property-tag tests

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 28, 2026

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit e1bf547:

  • PrettyNaming.fsi: Made EscapeActivePatternCases internal (matching the base branch review fix)
  • TypeTests.fs: Kept my unit-param conformance tests; removed stray <<<<<<< HEAD marker left from the previous merge; kept roundtrip test enabled (no Skip)
  • TooltipTests.fs: Included all new method/property tagging tooltip tests from the base branch

Build: 0 warnings/errors.

abonie
abonie reviewed Apr 28, 2026
View reviewed changes
Comment thread tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs
Copy link
Copy Markdown
Member

@abonie abonie Apr 28, 2026

Choose a reason for hiding this comment

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

Similar example but without overloads also fails with the same error:
impl:

type D() =
    member _.M(()) = ()

sig:

type D =
    new : unit -> D
    member M: unit -> unit

Should this be address in the same PR?

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

Reviewers

@abonie abonie abonie left review comments

Assignees

No one assigned

Labels

None yet

Projects

F# Compiler and Tooling
Status: New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Signature generation roundtrip issue: overloaded member with unit parameter

3 participants

@T-Gro @abonie