Skip to content

Address additional signature generation roundtrip bugs#19609

Open
T-Gro wants to merge 19 commits intomainfrom
sig-roundtrip-sweep
Open

Address additional signature generation roundtrip bugs#19609
T-Gro wants to merge 19 commits intomainfrom
sig-roundtrip-sweep

Conversation

@T-Gro
Copy link
Copy Markdown
Member

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

Follow-up to #19586. Corpus-wide roundtrip sweep of 1,483 standalone test files found additional bugs where generated signatures fail to compile with their source.

Fixes #19592
Fixes #19593
Fixes #19594
Fixes #19595
Fixes #19597

Notable: namespace global detection moved from FSharpCheckerResults.fs into NicePrint.layoutImpliedSignatureOfModuleOrNamespace — now works for fsc --sig path too, not just FCS API.

Display changes in NicePrint/PrettyNaming leak to tooltips — all improvements, covered by 4 new tooltip tests (37 pass, 0 regressions).

@T-Gro T-Gro requested a review from a team as a code owner April 17, 2026 19:58
@T-Gro T-Gro force-pushed the sig-roundtrip-sweep branch from 2cfd821 to 36ba04a Compare April 17, 2026 19:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2026 •
edited
Loading

✅ No release notes required

@T-Gro T-Gro force-pushed the sig-roundtrip-sweep branch from 95a197e to 5570959 Compare April 19, 2026 17:10
This was referenced Apr 19, 2026
Closed
Open
@T-Gro T-Gro requested a review from abonie April 20, 2026 12:56
T-Gro added a commit that referenced this pull request Apr 20, 2026
@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 19 commits April 20, 2026 20:38
@T-Gro
Add failing test: namespace global roundtrip (found by corpus sweep)
bf2a7b8 
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)
73636b2 
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
2abe16d 
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abonie abonie Awaiting requested review from abonie

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

F# Compiler and Tooling
Status: New

Milestone

No milestone

1 participant

@T-Gro