From 04cd2f69bee9bcfba017a095c41a2f0edee5ea93 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Fri, 23 Aug 2024 15:02:42 +0100 Subject: [PATCH 1/3] Account for `VolatileAttribute` when checking attribute targets --- .../Checking/Expressions/CheckExpressions.fs | 8 ++- .../AttributeUsage/AttributeUsage.fs | 56 +++++++++++++++++++ .../AttributeUsage/E_VolatileField.fs | 31 ++++++++++ .../AttributeUsage/VolatileField01.fs | 3 + tests/fsharp/typecheck/sigs/neg16.bsl | 2 - 5 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_VolatileField.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 8d6e26140e1..7499e154d67 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -11086,7 +11086,11 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt if not (isNil declaredTypars) then errorR(Error(FSComp.SR.tcLiteralCannotHaveGenericParameters(), mBinding)) - if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) && memberFlagsOpt.IsNone && not attrs.IsEmpty then + let supportEnforceAttributeTargets = + (g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) && memberFlagsOpt.IsNone && not attrs.IsEmpty) + && not isVolatile // // VolatileFieldAttribute has a special treatment(specific error FS823) + + if supportEnforceAttributeTargets then TcAttributeTargetsOnLetBindings { cenv with tcSink = TcResultsSink.NoSink } env attrs overallPatTy overallExprTy (not declaredTypars.IsEmpty) isClassLetBinding CheckedBindingInfo(inlineFlag, valAttribs, xmlDoc, tcPatPhase2, explicitTyparInfo, nameToPrelimValSchemeMap, rhsExprChecked, argAndRetAttribs, overallPatTy, mBinding, debugPoint, isCompGen, literalValue, isFixed), tpenv @@ -11114,7 +11118,7 @@ and TcAttributeTargetsOnLetBindings (cenv: cenv) env attrs overallPatTy overallE else AttributeTargets.ReturnValue ||| AttributeTargets.Field ||| AttributeTargets.Property - TcAttributes cenv env attrTgt attrs |> ignore + TcAttributesWithPossibleTargets false cenv env attrTgt attrs |> ignore and TcLiteral (cenv: cenv) overallTy env tpenv (attrs, synLiteralValExpr) = diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs index ce6efe8033e..18892cbb5f3 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs @@ -787,6 +787,62 @@ type InterruptibleLazy<'T> private (valueFactory: unit -> 'T) = // SOURCE=AllowNullLiteral01.fs # AllowNullLiteral01.fs [] let ``AllowNullLiteral01 preview`` compilation = + compilation + |> withLangVersionPreview + |> verifyCompile + |> shouldSucceed + + // SOURCE= E_VolatileField.fs # E_VolatileField.fs + [] + let ``E_VolatileField 9.0`` compilation = + compilation + |> withLangVersion90 + |> verifyCompile + |> shouldFail + |> withDiagnostics [ + (Error 823, Line 3, Col 3, Line 4, Col 16, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 823, Line 6, Col 3, Line 7, Col 14, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 879, Line 6, Col 3, Line 7, Col 14, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 823, Line 9, Col 3, Line 10, Col 9, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 879, Line 9, Col 3, Line 10, Col 9, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 823, Line 26, Col 17, Line 26, Col 18, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 879, Line 13, Col 5, Line 14, Col 19, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 879, Line 16, Col 5, Line 17, Col 20, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 879, Line 19, Col 5, Line 20, Col 11, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 879, Line 22, Col 5, Line 23, Col 13, "Volatile fields must be marked 'mutable' and cannot be thread-static") + ] + + // SOURCE=E_AllowNullLiteral.fs # E_AllowNullLiteral.fs + [] + let ``E_VolatileField preview`` compilation = + compilation + |> withLangVersionPreview + |> verifyCompile + |> shouldFail + |> withDiagnostics [ + (Error 823, Line 3, Col 3, Line 4, Col 16, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 823, Line 6, Col 3, Line 7, Col 14, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 879, Line 6, Col 3, Line 7, Col 14, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 823, Line 9, Col 3, Line 10, Col 9, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 879, Line 9, Col 3, Line 10, Col 9, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 823, Line 26, Col 17, Line 26, Col 18, "The 'VolatileField' attribute may only be used on 'let' bindings in classes") + (Error 879, Line 13, Col 5, Line 14, Col 19, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 879, Line 16, Col 5, Line 17, Col 20, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 879, Line 19, Col 5, Line 20, Col 11, "Volatile fields must be marked 'mutable' and cannot be thread-static") + (Error 879, Line 22, Col 5, Line 23, Col 13, "Volatile fields must be marked 'mutable' and cannot be thread-static") + ] + + // SOURCE= VolatileField01.fs # VolatileField01.fs + [] + let ``VolatileField01 9.0`` compilation = + compilation + |> withLangVersion90 + |> verifyCompile + |> shouldSucceed + + // SOURCE=AllowNullLiteral01.fs # AllowNullLiteral01.fs + [] + let ``VolatileField01 preview`` compilation = compilation |> withLangVersionPreview |> verifyCompile diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_VolatileField.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_VolatileField.fs new file mode 100644 index 00000000000..d35abffe475 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_VolatileField.fs @@ -0,0 +1,31 @@ +module VolatileFieldSanityChecks = begin + + [] + let mutable x = 1 + + [] + let rec f x = 1 + + [] + let x2 = 1 + + type C() = + [] + static let sx2 = 1 // expect an error - not mutable + + [] + static let f x2 = 1 // expect an error - not mutable + + [] + let x2 = 1 // expect an error - not mutable + + [] + let f x2 = 1 // expect an error - not mutable + + [] + val mutable x : int // expect an error - not supported + + member x.P = 1 + +end + \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs new file mode 100644 index 00000000000..33b02f6e3de --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs @@ -0,0 +1,3 @@ +type InterruptibleLazy (value) = + [] + let mutable valueFactory = value \ No newline at end of file diff --git a/tests/fsharp/typecheck/sigs/neg16.bsl b/tests/fsharp/typecheck/sigs/neg16.bsl index 1440ccc78d3..9cb22ec9c0a 100644 --- a/tests/fsharp/typecheck/sigs/neg16.bsl +++ b/tests/fsharp/typecheck/sigs/neg16.bsl @@ -73,8 +73,6 @@ neg16.fs(90,7,90,22): typecheck error FS0025: Incomplete pattern matches on this neg16.fs(96,3,97,16): typecheck error FS0823: The 'VolatileField' attribute may only be used on 'let' bindings in classes -neg16.fs(99,5,99,18): typecheck error FS0842: This attribute is not valid for use on this language element - neg16.fs(99,3,100,14): typecheck error FS0823: The 'VolatileField' attribute may only be used on 'let' bindings in classes neg16.fs(99,3,100,14): typecheck error FS0879: Volatile fields must be marked 'mutable' and cannot be thread-static From ce694476b4eb34e7f5b1cbb5ee5559a087531161 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Fri, 23 Aug 2024 17:04:48 +0100 Subject: [PATCH 2/3] Update tests --- .../CustomAttributes/AttributeUsage/AttributeUsage.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs index 18892cbb5f3..bbce93b8480 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs @@ -812,7 +812,7 @@ type InterruptibleLazy<'T> private (valueFactory: unit -> 'T) = (Error 879, Line 22, Col 5, Line 23, Col 13, "Volatile fields must be marked 'mutable' and cannot be thread-static") ] - // SOURCE=E_AllowNullLiteral.fs # E_AllowNullLiteral.fs + // SOURCE=E_VolatileField.fs # E_VolatileField.fs [] let ``E_VolatileField preview`` compilation = compilation @@ -840,7 +840,7 @@ type InterruptibleLazy<'T> private (valueFactory: unit -> 'T) = |> verifyCompile |> shouldSucceed - // SOURCE=AllowNullLiteral01.fs # AllowNullLiteral01.fs + // SOURCE=VolatileField01.fs # VolatileField01.fs [] let ``VolatileField01 preview`` compilation = compilation From 73edc269982f01512a744bfc6696ea62ce9a9f91 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Mon, 26 Aug 2024 13:54:53 +0100 Subject: [PATCH 3/3] add more tests --- .../AttributeUsage/VolatileField01.fs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs index 33b02f6e3de..77e2721da10 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs @@ -1,3 +1,12 @@ type InterruptibleLazy (value) = [] - let mutable valueFactory = value \ No newline at end of file + let mutable valueFactory = value + + [] + static let mutable valueFactory2 = Unchecked.defaultof<_> + + [] + let mutable add1 = fun x -> x + 1 + + [] + static let mutable add2 = fun x -> x + 1