Use of ref readonly instead of in in libraries is a breaking change for F# callers

[Tarmil]

Description

With the switch from in to ref readonly in Unsafe.AsRef (#89736), since as I understand it F# doesn't support ref readonly, it becomes impossible to take an unsafe mutable reference to an immutable record field.

Reproduction Steps

type MyRecord =
    { Value : int }

    member this.SetValue(v: int) =
        (Unsafe.AsRef<int> &this.Value) <- v

Expected behavior

The code compiles successfully, and calling SetValue() mutates the field.

Actual behavior

The code fails to compile:

Error FS0041 : No overloads match for method 'AsRef'.

Known type of argument: inref<'T>

Available overloads:
 - Unsafe.AsRef<'T>(source: byref<'T>) : byref<'T> // Argument 'source' doesn't match
 - Unsafe.AsRef<'T>(source: voidptr) : byref<'T> // Argument 'source' doesn't match

Regression?

This is a regression from .NET 7.

Known Workarounds

No response

Configuration

Using .NET 8 RC2 and the associated F# compiler.

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

With the switch from in to ref readonly in Unsafe.AsRef (#89736), since as I understand it F# doesn't support ref readonly, it becomes impossible to take an unsafe mutable reference to an immutable record field.

Reproduction Steps

type MyRecord =
    { Value : int }

    member this.SetValue(v: int) =
        (Unsafe.AsRef<int> &this.Value) <- v

Expected behavior

The code compiles successfully, and calling SetValue() mutates the field.

Actual behavior

The code fails to compile:

Error FS0041 : No overloads match for method 'AsRef'.

Known type of argument: inref<'T>

Available overloads:
 - Unsafe.AsRef<'T>(source: byref<'T>) : byref<'T> // Argument 'source' doesn't match
 - Unsafe.AsRef<'T>(source: voidptr) : byref<'T> // Argument 'source' doesn't match

Regression?

This is a regression from .NET 7.

Known Workarounds

No response

Configuration

Using .NET 8 RC2 and the associated F# compiler.

Other information

No response

Author:	Tarmil
Assignees:	-
Labels:	area-System.Runtime.CompilerServices
Milestone:	-

[Sergio0694]

If F# doesn't support ref readonly, wouldn't just every single API using it be broken there, unless the language is updated?

In other words: isn't this an issue that should be in the F# repo? The API is working as intended.

[Tarmil]

Yes, I considered posting to F# too, and I probably will. I do find it odd though that only C# was taken into consideration to determine whether this would be a breaking change.

[vzarytovskii]

If F# doesn't support ref readonly, wouldn't just every single API using it be broken there, unless the language is updated?

In other words: isn't this an issue that should be in the F# repo? The API is working as intended.

We don't explicitly support it, the fix would probably be an easy one, however I don't understand the change in existing API.

Isn't it considered breaking in C#?

[Sergio0694]

No, it's explicitly not a breaking change in C# 12, as we introduced compatibility rules for in/ref/ref readonly for this.

The goal was explicitly to allow "fixing" the signatures of existing APIs such as this, without it being a breaking change.
Which is the case, at least for C# only. You can read the spec here.

[Szer]

No, it's explicitly not a breaking change in C# 12, as we introduced compatibility rules for in/ref/ref readonly for this.

BCL is not "for C#" though, but "for .NET" (written in C# yes)

Therefore if changes in BCL are breaking for .NET they are the breaking change by definition

[Sergio0694]

I was specifically answering to the question "isn't it considered a breaking change in C#?" 😅

[Szer]

I was specifically answering to the question "isn't it considered a breaking change in C#?" 😅

My bad! There could be an angry mob of passionate F#'ers around the corner! (one of them myself)
I've come in good faith!

[AaronRobinsonMSFT]

This looks like it needs to be reverted. Thanks for reporting this.

[vzarytovskii]

This looks like it needs to be reverted. Thanks for reporting this.

We can fix it, however it will take much more time than revert.

[Sergio0694]

Two things:

• Just to clarify, this is only a source breaking change for F#, right? It should not be binary breaking (same modreq-s)
• If we do have to revert, we might not have to revert all combinations (as in, in -> ref readonly doesn't work on F#, yes, but we might want to at least double check whether ref -> ref readonly and/or ref -> in might work already?)

[AaronRobinsonMSFT]

This looks like it needs to be reverted. Thanks for reporting this.

We can fix it, however it will take much more time than revert.

Thanks @vzarytovskii. Having F# fix it seems like general goodness, but this is likely blocking people from adopting .NET 8, which is a red flag in general. If someone wants to try this API change again in .NET 9, that is fine but for now this shouldn't have been changed. The PR that did this has had other unfortunage side effects that caused other reverts. Unless there is push back from @jkotas or @stephentoub I am going to revert the change.

[stephentoub]

Unless there is push back from @jkotas or @stephentoub I am going to revert the change.

How do you propose to do that?. NET 8 is done, and changing it in the reverse direction is a breaking change.

[jkotas]

Right, I think it may be too late to revert this change. It is a discussion for shiproom.

#89736 was rushed in with a weak justification at the very last minute that I was not happy with (we had internal teams discussion about that). The change caused number of issues, and it is not surprising that we are discovering more issues with it. Something to learn from for next time.

cc @jeffhandley @artl93

[AaronRobinsonMSFT]

NET 8 is done, and changing it in the reverse direction is a breaking change.

Right, so we can keep the impact relatively narrow, sorry F# community, or we impact the larger C# community. I will defer to @jeffhandley and @artl93 if they have any opinion on this.

[artl93]

I do not have an informed opinion at this time. I am going to play my "newbie card." I do not understand right now how the ABI is composed for combability purposes vs how the compiler understands it in reality, so I'm not going to draw conclusions, but ask questions.

• What was the intended compatibility story here? Was recompile intended to be required even for C#? Would reverting this now require everyone to recompile again?
• Does reverting now have an impact on the go-live nature intended for RC2?
• Were there performance implications from the original change? Or was it simply to update the API "shape"?
• What would be the ETA to enable this in F#?
• What is the scope of the impact? Is it a few APIs, or are these so intrinsic as to make F# unusable? (It's a little tough looking at the RC change, but it looks pervasive.)

[charlesroddie]

The language of "breaking F#" is too strong, absent a valid example of code broken. The code shown in the original post describes a method for mutating an immutable field. The fact that this is possible is surely unintended by F# design and should be regarded as a bug. So the fact that this code is broken is a fortunate side-effect of the PR being discussed.

[artl93]

The language of "breaking F#" is too strong, absent a valid example of code broken. The code shown in the original post describes a method for mutating an immutable field. The fact that this is possible is surely unintended by F# design and should be regarded as a bug. So the fact that this code is broken is a fortunate side-effect of the PR being discussed.

Let me rephrase what I think I understand you're saying (I am not familiar with F# linguistics): are you saying this use pattern is essentially undefined behavior in F#? And that while it happened to work, now it does not?

[vzarytovskii]

What would be the ETA to enable this in F#?

No eta now, I haven't even looked at the code yet, I would imagine we'll target 8.0.200 for the proper fix.

What is the scope of the impact? Is it a few APIs, or are these so intrinsic as to make F# unusable?

It's a source breaking change, not a binary one (i.e. it will fail to compile, but existing libraries which use that should continue working).

I'm only aware of the change in Unsafe APIs.

Generally anything which uses ref readonly parameters will be unusable from F#.

[stephentoub]

What was the intended compatibility story here? Was recompile intended to be required even for C#?

This has been in the works for over a year (e.g. #67445 (comment)), with a language feature introduced in C# 12 (dotnet/csharplang#6010) to make it feasible for us to change some already-shipped method signatures in a way that wouldn't be breaking in C#. For example, the Volatile class had this method:

public static long Read(ref long location);

but that signature prohibits it from being used with something that's readonly, even though logically that's fine. The language change enabled this to become:

public static long Read(ref readonly long location);

without being a source-breaking change for C#. To my knowledge, the change shouldn't be binary breaking at all, regardless of language that had been used to compile the consumer.

We discussed whether there would be impact on Visual Basic and F#, but apparently we never validated, which is a miss.

Does reverting now have an impact on the go-live nature intended for RC2?

Yes. For example, changing from ref to ref readonly as in the previous example is not source or binary breaking in C#, but changing from ref readonly to ref (as would be the effect of a revert) is source breaking in C#, as, for example, a call site could have then be written as Read(in _something), which works fine with a ref readonly parameter, but that will fail to compile with an error if it were just ref in the signature.

Were there performance implications from the original change?

It's primarily about usability.

[charlesroddie]

Let me rephrase what I think I understand you're saying (I am not familiar with F# linguistics): are you saying this use pattern is essentially undefined behavior in F#? And that while it happened to work, now it does not?

This could be considered undefined behaviour.

F# would like to define the pattern as impossible. The F# specification (and all documentation) defines records as immutable (by default - i.e. unless the keyword mutable is used). It does the best that it can to ensure that immutability of immutable constructs can be relied on in normal use.

However in practice F# is unable to guarantee the soundness of its constructs in all contexts, and there is an acknowledgement that these constructs can be broken, for example by reflection. So the F# spec does not guarantee that the pattern will not work.

[Tarmil]

@charlesroddie I think you're focusing too much on the fact that I used a record field in my example. The same issue happens no matter what is behind the inref.

[vzarytovskii]

This could be considered undefined behaviour.

It's not undefined, it's rather unverifiable.

[Szer]

F# would like to define the pattern as impossible.

All bets are off with Unsafe, System.Reflection or Unchecked

[tannergooding]

It’s worth noting that the use of Unsafe.AsRef is a low level feature specifically designed to bypass language safety semantics. It’s usage is already fairly low/niche in C# and is even lower in F#. A simple search for Unsafe.AsRef on GitHub shows 52.2k C# usages and only 12 F# usages. While this might not be fully representative, it does give a some scope of potential short term impact:
• C# - https://github.com/search?q=Unsafe.AsRef+language%3AC%23&type=code&l=C%23
• F# - https://github.com/search?q=Unsafe.AsRef+language%3AF%23&type=code&l=F%23
• grep.app shows similarly low numbers: https://grep.app/search?q=Unsafe.AsRef&filter[lang][0]=C%23&filter[lang][1]=F%23

There had been some testing with F# and C++/CLI at the time the change went in, but apparently the F# side of things either wasn’t done correctly or didn’t cover sufficient scenarios to hit this particular case.

The general purpose of the change to ref readonly (from either ref or in) is one of improving safety and reliability. .NET has always had ref (read/write) and out (write first, then read/write), but only got in (readonly) in C# 7.2. When in was introduced, there was no way to migrate the 15 or so years of existing ref parameters that should have been readonly to the new syntax. The libraries had a workaround in that we exposed ref T Unsafe.AsRef<T>(in T source) and this allowed you to strip the readonlyness away (similar to const_cast in C++). There was also a consideration that in allowed rvalues, and so you could do something like Unsafe.AsRef(5). This was potentially dangerous for APIs where the parameter being a byref was important and so there were various APIs that didn’t use in even if they only read the parameter.

C# 12 has introduced ref readonlyto fix some of these problems. ref readonly differs from in in that it requires an lvalue and so Unsafe.AsRef(5) could be disallowed and the user could only do Unsafe.AsRef(in x) instead. Additionally, it fixed the gap in the language and now allows existing APIs to migrate from ref T to either ref readonly T or in T without it being a source breaking change. This improves type safety and means that users need to use Unsafe.AsRef less to succeed when using various APIs. This works by allowing Method(ref x) to compile for a Method(in T x) or Method(ref readonly T x). The compiler may simply issue a warning in some cases where the byref kind of the method declaration and that used at the callsite don’t line up.

This issue has ultimately fallen out with F# because of how in vs ref readonly is encoded in metadata. ref is just ref, in is [IsReadOnly] ref, and ref readonly is [RequiresLocation] ref. If ref readonly had instead been encoded as [IsReadonly, RequiresLocation] ref, then F# would have continued seeing this as inref<T> rather than as byref<T>.

[Sergio0694]

"If ref readonly had instead been encoded as [IsReadonly, RequiresLocation] ref"

For additional context, that was the original proposed encoding in the spec, but it was later changed to just be [RequiresLocation] ref. This was done to reduce the metadata size when using ref readonly (since [RequiresLocation] alone is enough to distinguish between different byref parameter types, as the only case where that's used is for ref readonly, given that eg. ref and out implicitly already disallow rvalues).

[tannergooding]

F# very much is considered with regards to breaking changes, and we do our best to avoid breaking any language.

But, avoiding all breaks is effectively impossible as simply exposing a new API can break user code. It can change overload resolution in C#, it can change type inference in F#, it can cause an extension API to no longer be invoked, it can cause an ambiguity with certain types of edge cases, and so on.

We ultimately try to move the core .NET Libraries forward in a direction that is a positive for the ecosystem as a whole, and that includes changes like #89736 which are designed to improve safety, remove reliance on unsafe code, and help users avoid pits of failure.

We will need to do better about testing such changes in the future to ensure that the two edge cases caught don't happen again.

• The first edge was that it caused issues in the dotnet/sdk because the SDK hadn't quite updated to the newer C# compiler version yet and this negatively impacted source build. Due to the general timeframe difference between the runtime and the source build ingesting new versions of Roslyn, this means we need to be mindful of any new C# features used in the .NET Libraries. This was a relatively new thing and not something that we had to be mindful of in previous years
• The second edge was this case in F#, which we believe was tested but apparently wasn't tested sufficiently. It has negatively impacted the ability to unsafely convert an inref to a byref, which was already a dangerous operation. There are then 2 other cases that moved from in to ref readonly that would also be impacted. The first is new ReadOnlySpan(ref readonly T) and the other Nullable.GetValueRefOrDefaultRef(ref readonly T?). The change from ref to ref readonly would be invisible to F# users (it currently sees it as byref still, and will see it as inref after the language support is added), as will the change from ref to in (since F# allows implicit byref -> inref)

[thinkbeforecoding]

I'm using it in a high performance protobuf serializer/deserializer in F#, and there is no current workaround to mitigate the problem. I'm stuck with waiting for the next release to be able tu use net 8.0 😥
And this is a problem as net7. 0 end of line is quite short.

[vzarytovskii]

Was inserted into 8.0.1xx in dotnet/sdk#36836

@baronfel might know approximate release window for it.

[baronfel]

It should be part of the 8.0.101 servicing release that'll drop next month.