Attributes from implementation file are not seen by the compiler

[auduchinok]

Consider an F# library with a signature and an implementation files:

module Module

val f: unit -> unit

module Module

[<NoDynamicInvocation>]
let f () =
    ()

When checking the implementation against the signature, the attribute is kept in the implementation and is emitted in .NET metadata. It's not copied to signature and is effectively hidden from F# point of view. This is similar to how values themselves are hidden when not present in the signature files, so it can be considered expected. However, analyzing these attributes may be needed by the compiler or the tooling, but it's not possible due to them being completely removed in F# metadata.

There's a place where attributes are copied from the signature to the implementation, so both are present in IL:

fsharp/src/Compiler/Checking/SignatureConformance.fs

Line 375 in f80d20f

else checkAttribs aenv implVal.Attribs sigVal.Attribs (fun attribs -> implVal.SetAttribs attribs)

I propose we also copy the attributes in the other direction, so the compiler and IDEs could analyze them. Are there any objections in doing so?

[T-Gro]

A part of value proposition for signature files is to expose the surface area visible from the outside (be it a different module, or a different project).

With that view, I feel like we should be doing a better job in mandating anything externally important (as in, "the caller of this function should know") to be present in the signature file.

Since using .fsi is an explicit and optional decision (i.e. user already opted in having more code to maintain, in exchange for some guarantees), preferring "make sure diff for .fsi shows anything significant" seems like a better choice.

[majocha]

In tooling scenarios we often skip checking implementations when signature is present. We would have to force implementation check to retrieve implementation only attributes.

[T-Gro]

Thats an important point - anything which affects the consumer (be it just checking of consuming code), should really be present on the .fsi declared signature.

[auduchinok]

To share some additional context, I've stumbled across this issue while working on #19548. The changes there improve debugging experience a lot, but the use of NoDynamicInvocation prevents calling the resulting methods. It makes sense for these particular functions to still be inlined and the inlining logic could check the presence of this attribute. The problem is in NativePtr it's only present in the implementation, so it's removed from the F# metadata and the compiler can't check it later.

[auduchinok]

Thats an important point - anything which affects the consumer (be it just checking of consuming code), should really be present on the .fsi declared signature.

Should we raise an error when some important attributes being present in the implementation file only? It'd be similar to how CompiledName is checked, for example.

Or do you mean we should require all attributes to be present in the signature?

[T-Gro]

Yes, or how InlineIfLambda is checked :

fsharp/src/Compiler/Checking/SignatureConformance.fs

Line 323 in d85a74c

if sigHasInlineIfLambda && not implHasInlineIfLambda then

(I guess the basis would be defining a subset from the compiler known attributes, either defining important or their complement as a subset of this for vals and similarily for entities:

fsharp/src/Compiler/TypedTree/WellKnownAttribs.fsi

Lines 74 to 117 in d85a74c

	type internal WellKnownValAttributes =
	| None = 0uL
	| DllImportAttribute = (1uL <<< 0)
	| EntryPointAttribute = (1uL <<< 1)
	| LiteralAttribute = (1uL <<< 2)
	| ConditionalAttribute = (1uL <<< 3)
	| ReflectedDefinitionAttribute_True = (1uL <<< 4)
	| RequiresExplicitTypeArgumentsAttribute = (1uL <<< 5)
	| DefaultValueAttribute_True = (1uL <<< 6)
	| SkipLocalsInitAttribute = (1uL <<< 7)
	| ThreadStaticAttribute = (1uL <<< 8)
	| ContextStaticAttribute = (1uL <<< 9)
	| VolatileFieldAttribute = (1uL <<< 10)
	| NoDynamicInvocationAttribute_True = (1uL <<< 11)
	| ExtensionAttribute = (1uL <<< 12)
	| OptionalArgumentAttribute = (1uL <<< 13)
	| InAttribute = (1uL <<< 14)
	| OutAttribute = (1uL <<< 15)
	| ParamArrayAttribute = (1uL <<< 16)
	| CallerMemberNameAttribute = (1uL <<< 17)
	| CallerFilePathAttribute = (1uL <<< 18)
	| CallerLineNumberAttribute = (1uL <<< 19)
	| DefaultParameterValueAttribute = (1uL <<< 20)
	| ProjectionParameterAttribute = (1uL <<< 21)
	| InlineIfLambdaAttribute = (1uL <<< 22)
	| OptionalAttribute = (1uL <<< 23)
	| StructAttribute = (1uL <<< 24)
	| NoCompilerInliningAttribute = (1uL <<< 25)
	| ReflectedDefinitionAttribute_False = (1uL <<< 26)
	| DefaultValueAttribute_False = (1uL <<< 27)
	| NoDynamicInvocationAttribute_False = (1uL <<< 28)
	| GeneralizableValueAttribute = (1uL <<< 29)
	| CLIEventAttribute = (1uL <<< 30)
	| NonSerializedAttribute = (1uL <<< 31)
	| MethodImplAttribute = (1uL <<< 32)
	| PreserveSigAttribute = (1uL <<< 33)
	| FieldOffsetAttribute = (1uL <<< 34)
	| CompiledNameAttribute = (1uL <<< 35)
	| WarnOnWithoutNullArgumentAttribute = (1uL <<< 36)
	| MarshalAsAttribute = (1uL <<< 37)
	| NoEagerConstraintApplicationAttribute = (1uL <<< 38)
	| ValueAsStaticPropertyAttribute = (1uL <<< 39)
	| TailCallAttribute = (1uL <<< 40)
	| NotComputed = (1uL <<< 63)