diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 4a6110cf51..7e9cd230a4 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Fix `[]` prefix attributes being silently dropped on class members, and fix false-positive `AllowMultiple=false` errors when `[]` and `[]` are applied to the same binding. ([Issue #17904](https://github.com/dotnet/fsharp/issues/17904), [Issue #19020](https://github.com/dotnet/fsharp/issues/19020), [PR #19738](https://github.com/dotnet/fsharp/pull/19738)) * Fix attributes on return type of unparenthesized tuple methods being silently dropped from IL. ([Issue #462](https://github.com/dotnet/fsharp/issues/462), [PR #19714](https://github.com/dotnet/fsharp/pull/19714)) * Fix internal error FS0073 "Undefined or unsolved type variable" in IlxGen when nested inline SRTP functions with multiple overloads leave unsolved typars in the non-witness codegen path. ([Issue #19709](https://github.com/dotnet/fsharp/issues/19709), [PR #19710](https://github.com/dotnet/fsharp/pull/19710)) * Fix NRE when calling virtual Object methods on value types through inline SRTP functions. ([Issue #8098](https://github.com/dotnet/fsharp/issues/8098), [PR #19511](https://github.com/dotnet/fsharp/pull/19511)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 30d49e776c..6636901e46 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -11169,7 +11169,7 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt let envinner = AddDeclaredTypars NoCheckForDuplicateTypars (enclosingDeclaredTypars@declaredTypars) env match bind with - | NormalizedBinding(vis, kind, isInline, isMutable, attrs, xmlDoc, _, valSynData, pat, NormalizedBindingRhs(spatsL, rtyOpt, rhsExpr), _, debugPoint) -> + | NormalizedBinding(vis, kind, isInline, isMutable, attrs, xmlDoc, _, valSynData, pat, NormalizedBindingRhs(spatsL, _, rhsExpr), _, debugPoint) -> let (SynValData(memberFlags = memberFlagsOpt)) = valSynData let mBinding = pat.Range @@ -11223,29 +11223,17 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt errorR(Error(FSComp.SR.tcAttributesAreNotPermittedOnLetBindings(), attr.Range)) attrs - // Rotate [] from binding to return value - // Also patch the syntactic representation - let retAttribs, valAttribs, valSynData = - let attribs = TcAttrs attrTgt false attrs - let rotRetSynAttrs, rotRetAttribs, valAttribs = - // Do not rotate if some attrs fail to typecheck... - if attribs.Length <> attrs.Length then [], [], attribs - else attribs - |> List.zip attrs - |> List.partition(function | _, Attrib(_, _, _, _, _, Some ts, _) -> ts &&& AttributeTargets.ReturnValue <> enum 0 | _ -> false) - |> fun (r, v) -> (List.map fst r, List.map snd r, List.map snd v) - let retAttribs = - match rtyOpt with - | Some (SynBindingReturnInfo(attributes = Attributes retAttrs)) -> - rotRetAttribs @ TcAttrs AttributeTargets.ReturnValue true retAttrs - | None -> rotRetAttribs - let valSynData = - match rotRetSynAttrs with - | [] -> valSynData - | {Range=mHead} :: _ -> - let (SynValData(valMf, SynValInfo(args, SynArgInfo(attrs, opt, retId)), valId)) = valSynData - SynValData(valMf, SynValInfo(args, SynArgInfo({Attributes=rotRetSynAttrs; Range=mHead} :: attrs, opt, retId)), valId) - retAttribs, valAttribs, valSynData + // [] attributes are moved out of the binding's prefix and into + // SynValData.SynValInfo.retInfo by SynInfo.RotateReturnAttributes in mkSynBinding, + // alongside any attributes on the return type annotation populated by InferSynReturnData. + // Use that as the single source of truth. + let valAttribs = TcAttrs attrTgt false attrs + + let retAttribs = + let (SynValData(_, SynValInfo(_, SynArgInfo(retAttrs, _, _)), _)) = valSynData + retAttrs + |> List.collect (fun a -> a.Attributes) + |> TcAttrs AttributeTargets.ReturnValue true let valAttribFlags = computeValWellKnownFlags g valAttribs diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 54290c21bd..c2ff7d1258 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -94,11 +94,16 @@ let ActivePatternElemsOfValRef g (vref: ValRef) = ActivePatternReturnKind.RefTypeWrapper else let _, apReturnTy = stripFunTy g vref.TauType - let hasStructAttribute() = - vref.Attribs - |> List.exists (function - | Attrib(targetsOpt = Some(System.AttributeTargets.ReturnValue)) as a -> hasFlag (classifyValAttrib g a) WellKnownValAttributes.StructAttribute - | _ -> false) + let hasStructAttribute() = + // After SynInfo.RotateReturnAttributes, [] lives in + // ValReprInfo's result ArgReprInfo rather than vref.Attribs. The flag bits + // in ArgReprInfo.Attribs are computed lazily, so classify the underlying + // attribute list directly. + match vref.ValReprInfo with + | Some(ValReprInfo(_, _, retInfo)) -> + let flags = computeValWellKnownFlags g (retInfo.Attribs.AsList()) + hasFlag flags WellKnownValAttributes.StructAttribute + | None -> false if isValueOptionTy g apReturnTy || hasStructAttribute() then ActivePatternReturnKind.StructTypeWrapper elif isBoolTy g apReturnTy then ActivePatternReturnKind.Boolean else ActivePatternReturnKind.RefTypeWrapper diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs index 9ce9185289..da07055711 100644 --- a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs +++ b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs @@ -740,6 +740,48 @@ module SynInfo = let argInfos = infosForObjArgs @ infosForArgs SynValData(Some memFlags, SynValInfo(argInfos, retInfo), None) + let private isReturnTargetedAttribute (a: SynAttribute) = + match a.Target with + | Some id -> id.idText = "return" + | None -> false + + /// Rotate any `[]` attributes from a binding's prefix attribute list into the + /// arity-info return position (`SynValInfo.retInfo`). Without this, downstream code that + /// reads `Val.Attribs` would incorrectly see them alongside method-targeted attributes + /// (see issues #17904 and #19020). + let RotateReturnAttributes (attrs: SynAttributes) (valSynData: SynValData) : SynAttributes * SynValData = + // Fast path: avoid all allocation when there's nothing to rotate (the common case). + let hasReturn = + attrs + |> List.exists (fun lst -> lst.Attributes |> List.exists isReturnTargetedAttribute) + + if not hasReturn then + attrs, valSynData + else + let mutable returnTargeted = [] + + let newAttrs = + attrs + |> List.choose (fun lst -> + let ret, kept = lst.Attributes |> List.partition isReturnTargetedAttribute + returnTargeted <- returnTargeted @ ret + + if List.isEmpty kept then + None + else + Some { lst with Attributes = kept }) + + let (SynValData(memFlags, SynValInfo(args, SynArgInfo(retAttrs, opt, retId)), thisIdOpt)) = + valSynData + + let retList: SynAttributeList = + { + Attributes = returnTargeted + Range = (List.head returnTargeted).Range + } + + newAttrs, SynValData(memFlags, SynValInfo(args, SynArgInfo(retList :: retAttrs, opt, retId)), thisIdOpt) + let mkSynBindingRhs staticOptimizations rhsExpr mRhs retInfo = let rhsExpr = List.foldBack (fun (c, e1) e2 -> SynExpr.LibraryOnlyStaticOptimization(c, e1, e2, mRhs)) staticOptimizations rhsExpr @@ -759,6 +801,8 @@ let mkSynBinding let info = SynInfo.InferSynValData(memberFlagsOpt, Some headPat, Option.map snd retInfo, origRhsExpr) + let attrs, info = SynInfo.RotateReturnAttributes attrs info + let rhsExpr, retTyOpt = mkSynBindingRhs staticOptimizations origRhsExpr mRhs retInfo let mBind = unionRangeWithXmlDoc xmlDoc mBind SynBinding(vis, SynBindingKind.Normal, isInline, isMutable, attrs, xmlDoc, info, headPat, retTyOpt, rhsExpr, mBind, spBind, trivia) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs index 8262651bec..7464dec733 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs @@ -43,7 +43,114 @@ type C() = |> ignoreWarnings |> compile |> shouldSucceed - + + // Regression tests for #17904 and #19020. Both have a common root cause: + // [] prefix attributes must route to the return-value metadata slot, + // not stay on the binding alongside method-targeted attributes. + + [] + let ``Issue 19020 - [] is emitted on the return parameter of class members`` () = + FSharp """ +module M +open System +[] +type DescAttribute() = inherit Attribute() + +type T() = + [] + member _.Inst a = a + 1 + [] + static member Stat a = a + 1 + +[] +let main _ = + let inst = typeof.GetMethod("Inst").ReturnParameter.GetCustomAttributes(typeof, false).Length + let stat = typeof.GetMethod("Stat").ReturnParameter.GetCustomAttributes(typeof, false).Length + if inst <> 1 then failwithf "instance member: expected 1, got %d" inst + if stat <> 1 then failwithf "static member: expected 1, got %d" stat + 0 + """ + |> asExe + |> compile + |> shouldSucceed + |> run + |> shouldSucceed + + [] + let ``Issue 17904 - [] on method and [] on return value are not duplicates`` () = + Fsx """ +open System +[] +type AttrAttribute() = inherit Attribute() + +type T() = + [] + [] + member _.Foo() = () + """ + |> ignoreWarnings + |> compile + |> shouldSucceed + + [] + let ``Issue 17904 - two [] on the same return value remain duplicates`` () = + Fsx """ +open System +[] +type AttrAttribute() = inherit Attribute() + +type T() = + [] + [] + member _.Foo() = () + """ + |> ignoreWarnings + |> compile + |> shouldFail + |> withSingleDiagnostic (Error 429, Line 8, Col 7, Line 8, Col 19, "The attribute type 'AttrAttribute' has 'AllowMultiple=false'. Multiple instances of this attribute cannot be attached to a single language element.") + + [] + let ``Issue 17904 - [] and [] on a method remain duplicates`` () = + Fsx """ +open System +[] +type AttrAttribute() = inherit Attribute() + +type T() = + [] + [] + member _.Foo() = () + """ + |> ignoreWarnings + |> compile + |> shouldFail + |> withSingleDiagnostic (Error 429, Line 8, Col 7, Line 8, Col 19, "The attribute type 'AttrAttribute' has 'AllowMultiple=false'. Multiple instances of this attribute cannot be attached to a single language element.") + + [] + let ``CompilationRepresentation(Instance) on a union-type member is not rotated to the return value`` () = + FSharp """ +module M +type MyOption<'T> = + | MySome of 'T + | MyNone + [] + member this.Value = match this with | MySome v -> v | MyNone -> failwith "MyNone" + +[] +let main _ = + let m = typeof>.GetMethod("get_Value") + if isNull m then failwith "get_Value not found — CompilationRepresentation(Instance) was likely rotated away from the member" + if m.IsStatic then failwith "get_Value should be an instance method" + if (MySome 42).Value <> 42 then failwith "Value did not return the carried payload" + 0 + """ + |> asExe + |> compile + |> shouldSucceed + |> run + |> shouldSucceed + + [] let ``Regression: typechecker does not fail when attribute is on type variable (https://github.com/dotnet/fsharp/issues/13525)`` () = let csharpBaseClass = diff --git a/tests/FSharp.Compiler.Service.Tests/Symbols.fs b/tests/FSharp.Compiler.Service.Tests/Symbols.fs index 69bfb70519..25cc0cd4b6 100644 --- a/tests/FSharp.Compiler.Service.Tests/Symbols.fs +++ b/tests/FSharp.Compiler.Service.Tests/Symbols.fs @@ -256,6 +256,43 @@ let x = 123 |> Option.map (fun su -> su.Symbol :?> FSharpMemberOrFunctionOrValue) |> Option.iter (fun symbol -> symbol.Attributes.Count |> shouldEqual 1) + [] + let ``FCS - [] surfaces on the method's Attributes only`` () = + let source = """ +open System.ComponentModel +type Calculator() = + [] + member _.Compu{caret}te () = 1 +""" + let mfv = (Checker.getSymbolUse source).Symbol :?> FSharpMemberOrFunctionOrValue + mfv.HasAttribute() |> shouldEqual true + mfv.ReturnParameter.HasAttribute() |> shouldEqual false + + [] + let ``FCS - [] surfaces on ReturnParameter.Attributes only`` () = + let source = """ +open System.ComponentModel +type Calculator() = + [] + member _.Compu{caret}te () = 1 +""" + let mfv = (Checker.getSymbolUse source).Symbol :?> FSharpMemberOrFunctionOrValue + mfv.HasAttribute() |> shouldEqual false + mfv.ReturnParameter.HasAttribute() |> shouldEqual true + + [] + let ``FCS - [] and [] surface independently on the same member`` () = + let source = """ +open System.ComponentModel +type Calculator() = + [] + [] + member _.Compu{caret}te () = 1 +""" + let mfv = (Checker.getSymbolUse source).Symbol :?> FSharpMemberOrFunctionOrValue + mfv.HasAttribute() |> shouldEqual true + mfv.ReturnParameter.HasAttribute() |> shouldEqual true + module Types = [] let ``FSharpType.Print parent namespace qualifiers`` () =