Skip to content

Commit 0abb33d

Browse files
T-GroCopilot
T-Gro
and
Copilot
authored
Address signature generation bugs (#19586)
* Fix recursive module do-binding leaking compiler-generated val in signature Filter out compiler-generated vals with '@' in their LogicalName from inferred signature printing. In 'module rec' contexts, 'do ()' bindings are compiled as TMDefRec with vals named like 'doval@3' that leaked into generated signatures. Fixes #13832 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix literal values in attribute arguments during signature generation When generating signatures, attribute arguments that reference [<Literal>] values now preserve the literal identifier name instead of showing the evaluated constant value. For example, [<Category(A)>] is preserved instead of being reduced to [<Category("A")>]. The fix works by recovering literal val references from the syntax tree during attribute checking: TcVal inlines literals to Expr.Const, but we now look up the original identifier in the name resolution environment and store Expr.Val as the source expression in AttribExpr. The Expr.Val case is then handled in layoutAttribArg in NicePrint.fs to display the literal name. Fixes #13810 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix struct signature missing NoComparison/NoEquality attributes (#15339) When generating signatures for struct types whose fields prevent comparison/equality augmentation (e.g., fields of type obj), the generated signature now includes [<NoComparison>] and [<NoEquality>] attributes. Without these, the signature fails to compile with FS0293. The fix detects when a struct type is a candidate for comparison/equality augmentation but augmentation was not generated (GeneratedCompareToValues or GeneratedHashAndEqualsValues is None), and injects synthetic attributes into the signature output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix backtick escaping in AddBackticksToIdentifierIfNeeded (#15389) Change single-backtick checks to double-backtick checks so identifiers containing backticks in their names get properly escaped with `` `` `` delimiters during signature generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix private keyword placement in prefix-style type abbreviation signatures (#15560) Move layoutAccessibility call after layoutTyparDecls so that 'private' is placed before the entire ''a P' construct rather than between the type parameter and name (producing 'type 'a private P' instead of the correct 'type private 'a P'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix missing [<Class>] attribute in signatures for types without visible constructors When generating signatures for class types whose constructors are not visible (e.g., private constructors), the [<Class>] attribute was not emitted. This caused the generated signatures to fail compilation with error FS0938 because the compiler cannot infer that the type is a class from the signature alone. The fix: 1. Always set start=Some "class" for class types regardless of printVerboseSignatures setting (was gated behind it before) 2. Suppress [<Class>] when allDecls is empty since the repr layout already produces 'class end' for empty classes 3. Remove commented-out conditions that were never active Fixes #16531 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add test coverage for literal attribute args and CompilerCompat - Test multiple literal args in tuple attribute position (names recovered) - Test qualified literal reference limitation (shows constant value) - Add CompilerCompat test: type with literal-based attribute arg exercises Expr.Val in AttribExpr.source pickling across compiler versions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guard condition and edge case tests for signature generation - Struct with explicit [<NoComparison>] does not get duplicate attribute - Struct with [<CustomComparison>] does not get [<NoComparison>] injected - Empty class with private constructor uses 'class end' repr - AbstractClass with private constructor roundtrips correctly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Guard literal recovery against VRefNonLocal to prevent transitive deps Only replace Expr.Const with Expr.Val in AttribExpr.source when the literal val reference is local (VRefLocal). Cross-assembly literals (VRefNonLocal from 'open ExternalLib') keep Expr.Const to avoid creating unresolvable transitive assembly dependencies in pickled metadata — consumers of the DLL may not reference the external assembly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Normalize Expr.Val back to Expr.Const in p_attrib_expr before pickling The literal name recovery stores Expr.Val in AttribExpr.source as an in-memory optimization for signature generation display. However, this must NOT change the pickle format: - Old compilers reading Expr.Val in AttribExpr.source would display '(* unsupported attribute argument *)' instead of the constant value in tooltips and signatures — a cross-version display regression. - Old compilers' CheckDeclarations.fs pattern-matches on the source field and would miss the AssemblyVersion format warning. By normalizing Expr.Val(literal_vref) back to Expr.Const(literal_value) before pickling, the on-disk format is identical to before this PR. The literal name display only works during the current compilation (the signature generation use case), which is the correct scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix literal recovery for named attrs and tighten doval filter Two fixes from adversarial cross-model review: 1. Add SynExpr.App traversal to literal ident collector so named attribute arguments like [<Attr(Name = MyLiteral)>] also get literal name recovery. Previously only positional args worked. 2. Replace IsCompilerGeneratedName (any '@' in name) with targeted StartsWithOrdinal("doval@") in filterVal. The broad '@' check would incorrectly drop backtick-escaped user identifiers containing '@' from module rec signatures. The doval@ prefix is the only compiler-generated name pattern that leaks through TMDefRec bindings, matching the same pattern used in SignatureHash.fs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add release notes for signature generation fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Gate [<Class>] emission on showAttributes to fix FSI printing tests The PR unconditionally set start=Some "class" for class types, causing [<Class>] to appear in FSI output. Gate on denv.showAttributes instead of the removed printVerboseSignatures check: - FSI (showAttributes=false): no [<Class>] emitted (matches old behavior) - GenerateSignature (showAttributes=true): [<Class>] emitted when needed - --sig flag (showAttributes=true): [<Class>] emitted when needed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix flaky help test: reset consolecolors before help output Add --consolecolors+ before help flags (-?, /?, --help) in help tests to ensure enableConsoleColoring is always true when help text is generated. This prevents race conditions from concurrent tests that temporarily set enableConsoleColoring to false (e.g., consolecolors switch test), which caused Linux CI failures where the baseline expected '(on by default)' but got '(off by default)'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e86257a commit 0abb33d

11 files changed

Lines changed: 505 additions & 25 deletions

File tree

‎docs/release-notes/.FSharp.Compiler.Service/11.0.100.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@
2828
* Fix box instruction for literal upcasts. (Issue [#18319](https://github.com/dotnet/fsharp/issues/18319), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
2929
* Fix Decimal Literal causes InvalidProgramException in Debug builds. (Issue [#18956](https://github.com/dotnet/fsharp/issues/18956), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
3030
* Fix `AttributeUsage.AllowMultiple` not being inherited for attributes subclassed in C#. ([Issue #17107](https://github.com/dotnet/fsharp/issues/17107), [PR #19315](https://github.com/dotnet/fsharp/pull/19315))
31+
* Fix signature generation: recursive module `do` binding leaking compiler-generated val. ([Issue #13832](https://github.com/dotnet/fsharp/issues/13832), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
32+
* Fix signature generation: literal values in attribute arguments now preserve literal identifier name. ([Issue #13810](https://github.com/dotnet/fsharp/issues/13810), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
33+
* Fix signature generation: struct types with non-comparable/non-equatable fields now include `[<NoComparison>]`/`[<NoEquality>]`. ([Issue #15339](https://github.com/dotnet/fsharp/issues/15339), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
34+
* Fix signature generation: backtick escaping for identifiers containing backticks. ([Issue #15389](https://github.com/dotnet/fsharp/issues/15389), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
35+
* Fix signature generation: `private` keyword placement for prefix-style type abbreviations. ([Issue #15560](https://github.com/dotnet/fsharp/issues/15560), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
36+
* Fix signature generation: missing `[<Class>]` attribute for types without visible constructors. ([Issue #16531](https://github.com/dotnet/fsharp/issues/16531), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
3137

3238
### Added
3339

‎src/Compiler/Checking/CheckDeclarations.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5900,7 +5900,7 @@ let CheckOneImplFile
59005900

59015901
// Warn on version attributes.
59025902
topAttrs.assemblyAttrs |> List.iter (function
5903-
| Attrib(tref, _, [ AttribExpr(Expr.Const (Const.String version, range, _), _) ], _, _, _, _) ->
5903+
| Attrib(tref, _, [ AttribExpr(_, Expr.Const (Const.String version, range, _)) ], _, _, _, _) ->
59045904
let attrName = tref.CompiledRepresentationForNamedType.FullName
59055905
let isValid() =
59065906
try parseILVersion version |> ignore; true

‎src/Compiler/Checking/Expressions/CheckExpressions.fs‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11628,8 +11628,37 @@ and TcAttributeEx canFail (cenv: cenv) (env: TcEnv) attrTgt attrEx (synAttr: Syn
1162811628

1162911629
UnifyTypes cenv env mAttr ty (tyOfExpr g expr)
1163011630

11631+
// Collect literal val references from the syntax expression to recover original names.
11632+
// TcVal inlines literal vals to Expr.Const, losing the original val reference.
11633+
// We recover it here by matching ranges from the syntax tree against the checked expression.
11634+
let literalIdents =
11635+
let rec collect (synExpr: SynExpr) acc =
11636+
match synExpr with
11637+
| SynExpr.Ident ident ->
11638+
match env.NameEnv.eUnqualifiedItems |> Map.tryFind ident.idText with
11639+
| Some(Item.Value vref) when vref.LiteralValue.IsSome ->
11640+
(ident.idRange, vref) :: acc
11641+
| _ -> acc
11642+
| SynExpr.Paren(expr = inner) -> collect inner acc
11643+
| SynExpr.Tuple(exprs = exprs) -> List.fold (fun a e -> collect e a) acc exprs
11644+
| SynExpr.App(_, _, funcExpr, argExpr, _) -> collect funcExpr (collect argExpr acc)
11645+
| _ -> acc
11646+
11647+
collect arg []
11648+
1163111649
let mkAttribExpr e =
11632-
AttribExpr(e, EvalLiteralExprOrAttribArg g e)
11650+
let sourceExpr =
11651+
match e with
11652+
| Expr.Const(_, m, _) ->
11653+
match literalIdents |> List.tryFind (fun (r, _) -> Range.equals r m) with
11654+
// Only use Expr.Val for local refs to avoid creating transitive assembly
11655+
// dependencies in pickled metadata (VRefNonLocal would require the
11656+
// external assembly to be available when importing this DLL).
11657+
| Some(_, vref) when vref.IsLocalRef -> Expr.Val(vref, NormalValUse, m)
11658+
| _ -> e
11659+
| _ -> e
11660+
11661+
AttribExpr(sourceExpr, EvalLiteralExprOrAttribArg g e)
1163311662

1163411663
let checkPropSetterAttribAccess m (pinfo: PropInfo) =
1163511664
let setterMeth = pinfo.SetterMethod

‎src/Compiler/Checking/NicePrint.fs‎

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,10 @@ module PrintTypes =
561561
/// Layout a single attribute arg, following the cases of 'gen_attr_arg' in ilxgen.fs
562562
/// This is the subset of expressions we display in the NicePrint pretty printer
563563
/// See also dataExprL - there is overlap between these that should be removed
564-
let rec layoutAttribArg denv arg =
565-
match arg with
564+
let rec layoutAttribArg denv arg =
565+
match arg with
566+
| Expr.Val (vref, _, _) when vref.LiteralValue.IsSome -> wordL (tagLocal vref.DisplayName)
567+
566568
| Expr.Const (c, _, ty) ->
567569
if isEnumTy denv.g ty then
568570
WordL.keywordEnum ^^ angleL (layoutType denv ty) ^^ bracketL (layoutConst denv.g ty c)
@@ -1986,6 +1988,47 @@ module TastDefinitionPrinting =
19861988
let isMeasure = (tycon.TypeOrMeasureKind = TyparKind.Measure)
19871989
let ty = generalizedTyconRef g tcref
19881990

1991+
// Augment tycon.Attribs with synthetic [<NoComparison>] / [<NoEquality>] when the type
1992+
// is a candidate for comparison/equality augmentation but augmentation was not generated
1993+
// (e.g. struct with non-comparable fields). This ensures generated signatures compile. (#15339)
1994+
let augmentedAttribs =
1995+
let isTrueFSharpStruct =
1996+
tycon.IsFSharpStructOrEnumTycon && not tycon.IsFSharpEnumTycon
1997+
1998+
// Only structs need synthetic NoComparison/NoEquality in signatures.
1999+
// Reference types (records, unions) compile fine without them.
2000+
let canBeAugmentedWithCompare = isTrueFSharpStruct
2001+
let canBeAugmentedWithEquals = isTrueFSharpStruct
2002+
2003+
let mkSyntheticCoreAttrib (attrName: string) =
2004+
let fsharpCorePath = [| "Microsoft"; "FSharp"; "Core" |]
2005+
let attrTcref = mkNonLocalTyconRef (mkNonLocalEntityRef g.fslibCcu fsharpCorePath) attrName
2006+
2007+
let ilTypeRef =
2008+
ILTypeRef.Create(g.ilg.fsharpCoreAssemblyScopeRef, [], "Microsoft.FSharp.Core." + attrName)
2009+
2010+
let ilMethodRef =
2011+
ILMethodRef.Create(ilTypeRef, ILCallingConv.Instance, ".ctor", 0, [], ILType.Void)
2012+
2013+
Attrib(attrTcref, ILAttrib ilMethodRef, [], [], false, None, Range.range0)
2014+
2015+
let mutable attribs = tycon.Attribs
2016+
2017+
if canBeAugmentedWithCompare
2018+
&& not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.NoComparisonAttribute tycon)
2019+
&& not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.CustomComparisonAttribute tycon)
2020+
&& tycon.GeneratedCompareToValues.IsNone then
2021+
attribs <- mkSyntheticCoreAttrib "NoComparisonAttribute" :: attribs
2022+
2023+
if canBeAugmentedWithEquals
2024+
&& not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.NoEqualityAttribute tycon)
2025+
&& not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.CustomEqualityAttribute tycon)
2026+
&& not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.ReferenceEqualityAttribute tycon)
2027+
&& tycon.GeneratedHashAndEqualsValues.IsNone then
2028+
attribs <- mkSyntheticCoreAttrib "NoEqualityAttribute" :: attribs
2029+
2030+
attribs
2031+
19892032
let start, tagger =
19902033
if isStructTy g ty && not tycon.TypeAbbrev.IsSome then
19912034
// Always show [<Struct>] whether verbose or not
@@ -1998,7 +2041,7 @@ module TastDefinitionPrinting =
19982041
elif isMeasure then
19992042
None, tagClass
20002043
elif isClassTy g ty then
2001-
if denv.printVerboseSignatures then
2044+
if denv.showAttributes then
20022045
(if simplified then None else Some "class"), tagClass
20032046
else
20042047
None, tagClass
@@ -2009,18 +2052,18 @@ module TastDefinitionPrinting =
20092052
if isFirstType then
20102053
WordL.keywordType
20112054
else
2012-
wordL (tagKeyword "and") ^^ layoutAttribs denv start false tycon.TypeOrMeasureKind tycon.Attribs emptyL
2055+
wordL (tagKeyword "and") ^^ layoutAttribs denv start false tycon.TypeOrMeasureKind augmentedAttribs emptyL
20132056

20142057
let nameL = ConvertLogicalNameToDisplayLayout (tagger >> mkNav tycon.DefinitionRange >> wordL) tycon.DisplayNameCore
20152058

2016-
let nameL = layoutAccessibility denv tycon.Accessibility nameL
2017-
let denv = denv.AddAccessibility tycon.Accessibility
2018-
20192059
let lhsL =
20202060
let tps = tycon.TyparsNoRange
20212061
let tpsL = layoutTyparDecls denv nameL tycon.IsPrefixDisplay tps
2062+
let tpsL = layoutAccessibility denv tycon.Accessibility tpsL
20222063
typewordL ^^ tpsL
20232064

2065+
let denv = denv.AddAccessibility tycon.Accessibility
2066+
20242067

20252068
let sortKey (minfo: MethInfo) =
20262069
(not minfo.IsConstructor,
@@ -2197,15 +2240,15 @@ module TastDefinitionPrinting =
21972240
let needsStartEnd =
21982241
match start with
21992242
| Some "class" ->
2243+
// When allDecls is empty, the repr layout produces 'class end' which is sufficient
2244+
not (isNil allDecls) &&
22002245
// 'inherits' is not enough for F# type kind inference to infer a class
22012246
// inherits.IsEmpty &&
22022247
ilFields.IsEmpty &&
22032248
// 'abstract' is not enough for F# type kind inference to infer a class by default in signatures
22042249
// 'static member' is surprisingly not enough for F# type kind inference to infer a class by default in signatures
22052250
// 'overrides' is surprisingly not enough for F# type kind inference to infer a class by default in signatures
2206-
//(meths |> List.forall (fun m -> m.IsAbstract || m.IsDefiniteFSharpOverride || not m.IsInstance)) &&
2207-
//(props |> List.forall (fun m -> (not m.HasGetter || m.GetterMethod.IsAbstract))) &&
2208-
//(props |> List.forall (fun m -> (not m.HasSetter || m.SetterMethod.IsAbstract))) &&
2251+
// Concrete instance methods and properties are also not enough (Error 938)
22092252
ctors.IsEmpty &&
22102253
instanceVals.IsEmpty &&
22112254
staticVals.IsEmpty
@@ -2384,7 +2427,7 @@ module TastDefinitionPrinting =
23842427
|> addLhs
23852428

23862429
typeDeclL
2387-
|> fun tdl -> if isFirstType then layoutAttribs denv start false tycon.TypeOrMeasureKind tycon.Attribs tdl else tdl
2430+
|> fun tdl -> if isFirstType then layoutAttribs denv start false tycon.TypeOrMeasureKind augmentedAttribs tdl else tdl
23882431
|> layoutXmlDocOfEntity denv infoReader tcref
23892432

23902433
// Layout: exception definition
@@ -2551,7 +2594,7 @@ module InferredSigPrinting =
25512594
let rec imdefsL denv x = aboveListL (x |> List.map (imdefL denv))
25522595

25532596
and imdefL denv x =
2554-
let filterVal (v: Val) = not v.IsCompilerGenerated && Option.isNone v.MemberInfo
2597+
let filterVal (v: Val) = not v.IsCompilerGenerated && Option.isNone v.MemberInfo && not (v.LogicalName.StartsWithOrdinal("doval@"))
25552598
let filterExtMem (v: Val) = v.IsExtensionMember
25562599

25572600
match x with

‎src/Compiler/SyntaxTree/PrettyNaming.fs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,8 +515,8 @@ let DoesIdentifierNeedBackticks (name: string) : bool =
515515
let AddBackticksToIdentifierIfNeeded (name: string) : string =
516516
if
517517
DoesIdentifierNeedBackticks name
518-
&& not (name.StartsWithOrdinal("`"))
519-
&& not (name.EndsWithOrdinal("`"))
518+
&& not (name.StartsWithOrdinal("``"))
519+
&& not (name.EndsWithOrdinal("``"))
520520
then
521521
"``" + name + "``"
522522
else

‎src/Compiler/TypedTree/TypedTreePickle.fs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2875,7 +2875,18 @@ and p_attribkind x st =
28752875
and p_attrib (Attrib(a, b, c, d, e, _targets, f)) st = // AttributeTargets are not preserved
28762876
p_tup6 (p_tcref "attrib") p_attribkind (p_list p_attrib_expr) (p_list p_attrib_arg) p_bool p_dummy_range (a, b, c, d, e, f) st
28772877

2878-
and p_attrib_expr (AttribExpr(e1, e2)) st = p_tup2 p_expr p_expr (e1, e2) st
2878+
and p_attrib_expr (AttribExpr(e1, e2)) st =
2879+
// Normalize Expr.Val back to Expr.Const before pickling.
2880+
// The literal name recovery (Expr.Val in source field) is an in-memory optimization
2881+
// for signature generation display. We must not change the pickle format, because
2882+
// old compilers reading Expr.Val in this position would show degraded attribute display.
2883+
let e1 =
2884+
match e1 with
2885+
| Expr.Val(vref, _, m) when vref.LiteralValue.IsSome ->
2886+
Expr.Const(vref.LiteralValue.Value, m, vref.Type)
2887+
| _ -> e1
2888+
2889+
p_tup2 p_expr p_expr (e1, e2) st
28792890

28802891
and p_attrib_arg (AttribNamedArg(a, b, c, d)) st =
28812892
p_tup4 p_string p_ty p_bool p_attrib_expr (a, b, c, d) st

‎tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/misc/misc.fs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ open System.IO
1111
module help_options =
1212

1313
// ReqENU SOURCE=dummy.fsx COMPILE_ONLY=1 PRECMD="\$FSC_PIPE >help40.txt -? 2>&1" POSTCMD="\$FSI_PIPE --nologo --quiet --exec ..\\..\\..\\comparer.fsx help40.txt help40.437.1033.bsl" # -?
14+
// Reset enableConsoleColoring before help tests to avoid global mutable state
15+
// pollution from concurrent tests (e.g., ``fsc --consolecolors switch``).
1416
[<Fact>]
1517
let ``Help - variant 1``() =
1618
FSharp ""
1719
|> asExe
1820
|> withBufferWidth 120
19-
|> withOptions ["-?"]
21+
|> withOptions ["--consolecolors+"; "-?"]
2022
|> compile
2123
|> verifyOutputWithBaseline (Path.Combine(__SOURCE_DIRECTORY__, "compiler_help_output.bsl"))
2224
|> shouldSucceed
@@ -27,7 +29,7 @@ module help_options =
2729
FSharp ""
2830
|> asExe
2931
|> withBufferWidth 120
30-
|> withOptions ["/?"]
32+
|> withOptions ["--consolecolors+"; "/?"]
3133
|> compile
3234
|> verifyOutputWithBaseline (Path.Combine(__SOURCE_DIRECTORY__, "compiler_help_output.bsl"))
3335
|> shouldSucceed
@@ -38,7 +40,7 @@ module help_options =
3840
FSharp ""
3941
|> asExe
4042
|> withBufferWidth 120
41-
|> withOptions ["--help"]
43+
|> withOptions ["--consolecolors+"; "--help"]
4244
|> compile
4345
|> verifyOutputWithBaseline (Path.Combine(__SOURCE_DIRECTORY__, "compiler_help_output.bsl"))
4446
|> shouldSucceed

0 commit comments

Comments
 (0)