Account for VolatileAttribute when checking attribute targets (#17595)

* Account for `VolatileAttribute`  when checking attribute targets

* Update tests

* add more tests

---------

Co-authored-by: Kevin Ransom (msft) <[email protected]>

Author: edgarfgp

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) =
 

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
     [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"AllowNullLiteral01.fs"|])>]
     let ``AllowNullLiteral01 preview`` compilation =
+        compilation
+        |> withLangVersionPreview
+        |> verifyCompile
+        |> shouldSucceed
+        
+    // SOURCE= E_VolatileField.fs	# E_VolatileField.fs
+    [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"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_VolatileField.fs	# E_VolatileField.fs
+    [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_VolatileField.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
+    [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"VolatileField01.fs"|])>]
+    let ``VolatileField01 9.0`` compilation =
+        compilation
+        |> withLangVersion90
+        |> verifyCompile
+        |> shouldSucceed
+    
+    // SOURCE=VolatileField01.fs	# VolatileField01.fs
+    [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"VolatileField01.fs"|])>]
+    let ``VolatileField01 preview`` compilation =
         compilation
         |> withLangVersionPreview
         |> verifyCompile

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_VolatileField.fs

@@ -0,0 +1,31 @@
+module VolatileFieldSanityChecks = begin
+
+  [<VolatileField>]
+  let mutable x = 1
+
+  [<VolatileField>]
+  let rec f x = 1
+
+  [<VolatileField>]
+  let x2 = 1
+
+  type C() = 
+    [<VolatileField>]
+    static let sx2 = 1   // expect an error - not mutable
+
+    [<VolatileField>]
+    static let f x2 = 1   // expect an error - not mutable
+
+    [<VolatileField>]
+    let x2 = 1   // expect an error - not mutable
+
+    [<VolatileField>]
+    let f x2 = 1   // expect an error - not mutable
+
+    [<VolatileField>]
+    val mutable x : int // expect an error - not supported
+
+    member x.P = 1
+
+end
+           

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/VolatileField01.fs

@@ -0,0 +1,12 @@
+type InterruptibleLazy (value) =
+    [<VolatileField>]
+    let mutable valueFactory = value
+
+    [<VolatileField>]
+    static let mutable valueFactory2 = Unchecked.defaultof<_>
+
+    [<VolatileField>]
+    let mutable add1 = fun x -> x + 1
+
+    [<VolatileField>]
+    static let mutable add2 = fun x -> x + 1

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