Infer isreadonly from structs ("readonly struct" in C#) to avoid struct-copies just like we do with readonly methods

[T-Gro]

In C#, [Readonly]can be applied both on method level as well as struct level.
When [Readonly] is on entire struct, typically via "readonly struct" definition, semantically it is equivalent to having [Readonly] on every method. However, F# only reads [Readonly] at method level at the moment.

This PR changes the behaviour of IlMethodyInfo.IsReadOnly to also detect [Readonly] on struct level.
That way, the MethodCall will be understood by F# compiler as 'NeverMutates' and therefore enable performance optimization by avoiding copies.

[vzarytovskii]

Wondering if this whould go under a feature flag, since technically it may change the codegen.

[T-Gro]

I consider it more of a fix. Basically the current way does not fully understand what Roslyn can do with respect to the "readonly" keyword and possible placement of [ReadOnly] attribute.

See example here - even placement of readonly keyword at the method level (which existing code is supposed to support) gets overriden by readonly on struct level, and therefore method does not get the [ReadOnly] by Roslyn.

[kerams]

Wouldn't IL tests be more reliable? The warning could theoretically be broken (or its behavior changed), independent of whether or not the defense copy is emitted.

[T-Gro]

This was the only part of the test suite that failed when adding this feature, that is why considered it.
You mean searching IL output and scanning that there was no copy instruction at all?
Or comparing strict equality of the entire generated IL?

[kerams]

Just something simple like https://github.com/dotnet/fsharp/blob/main/tests/FSharp.Compiler.ComponentTests/EmittedIL/EmptyArray.fs, to show the instruction is present when expected, and missing when dealing with readonly structs.

[T-Gro]

Just something simple like https://github.com/dotnet/fsharp/blob/main/tests/FSharp.Compiler.ComponentTests/EmittedIL/EmptyArray.fs, to show the instruction is present when expected, and missing when dealing with readonly structs.

Thanks, this is the way.

[dsyme]

I do agree this can count as a bug fix

Could you add test cases for extension methods on readonly structs please?

[T-Gro]

I do agree this can count as a bug fix

Could you add test cases for extension methods on readonly structs please?

Works.
For F# defined (even if C#-style) extension methods, the entire call gets inlined.
For C#-lib defined extension method, and consumed from F#, it works as desired = defensive copy avoided for readonly struct.