[API Proposal]: Replace TFrom : struct constraint on Unsafe.BitCast with a dynamic check

[MihaZupan]

Background and motivation

In .NET 8 we've added the Unsafe.BitCast API as a less-unsafe alternative to Unsafe.As when casting between compatible structs.

One justification was that this would let us avoid codegen pessimization for values marked as address-taken in some cases. Example from the original API proposal: #81334 (comment)

Due to the TFrom : struct constraint, we can't use this API in places like the span.IndexOf* helpers.
I propose we drop the generic constraint while keeping the runtime behavior the same.

With the constraint removed, I was able to confirm that we're able to flow the constness of values through for this case: MihaZupan@65c8054.

Quoting the conclusion from the initial API review:

... we should try having it constrained, and if we run into problems then get rid of them. They're easier to remove than add.

API Proposal

namespace System.Runtime.CompilerServices;

public static class Unsafe
{
    public static TTo BitCast<TFrom, TTo>(TFrom value)
-       where TFrom : struct
        where TTo : struct;
}

API Usage

public static unsafe int IndexOf<T>(this Span<T> span, T value)
    where T : IEquatable<T>?
{
    if (RuntimeHelpers.IsBitwiseEquatable<T>() && sizeof(T) == sizeof(short))
    {
        return IndexOfValueType(
            ref Unsafe.As<T, short>(ref MemoryMarshal.GetReference(span)),
            Unsafe.BitCast<T, short>(value0), // <--- This is now legal
            span.Length);
    }

    // ...
}

private static unsafe int IndexOfValueType<T>(ref T searchSpace, T value, int length)
    where T : struct, INumber<T> => 42;

Alternative Designs

We could drop the constraint on TTo as well while we're at it?

Risks

Some erroneous usages move from being compile-time errors to run-time exceptions.
But this is an Unsafe API for a reason :)

[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

---

Background and motivation

In .NET 8 we've added the Unsafe.BitCast API as a less-unsafe alternative to Unsafe.As when casting between compatible structs.

One justification was that this would let us avoid codegen pessimization for values marked as address-taken in some cases. Example from the original API proposal: #81334 (comment)

Due to the TFrom : struct constraint, we can't use this API in places like the span.IndexOf* helpers.
I propose we drop the generic constraint while keeping the runtime behavior the same.

With the constraint removed, I was able to confirm that we're able to flow the constness of values through for this case: MihaZupan@65c8054.

Quoting the conclusion from the initial API review:

... we should try having it constrained, and if we run into problems then get rid of them. They're easier to remove than add.

API Proposal

namespace System.Runtime.CompilerServices;

public static class Unsafe
{
    public static TTo BitCast<TFrom, TTo>(TFrom value)
-       where TFrom : struct
        where TTo : struct;
}

API Usage

public static unsafe int IndexOfAny<T>(this Span<T> span, T value)
    where T : IEquatable<T>?
{
    if (RuntimeHelpers.IsBitwiseEquatable<T>() && sizeof(T) == sizeof(short))
    {
        return IndexOfValueType(
            ref Unsafe.As<T, short>(ref MemoryMarshal.GetReference(span)),
            Unsafe.BitCast<T, short>(value0), // <--- This is now legal
            span.Length);
    }

    // ...
}

private static unsafe int IndexOfValueType<T>(ref T searchSpace, T value, int length)
    where T : struct, INumber<T> => 42;

Alternative Designs

We could drop the constraint on TTo as well while we're at it?

Risks

Some erroneous usages move from being compile-time errors to run-time exceptions.
But this is an Unsafe API for a reason :)

Author:	MihaZupan
Assignees:	-
Labels:	api-suggestion, area-System.Runtime.CompilerServices
Milestone:	-

[stephentoub]

I've also bumped up against this multiple times. I agree we should remove the constraint.

[MichalPetryka]

This assumes that we won't get generic bridging anytime soon I guess?

[tannergooding]

We should really be using these scenarios as evidence to better prioritize a feature that properly resolves the issue and only removing constraints where there is sufficient justification to handle it immediately and where a reasonable fallback can be given. That is to say, lack of bridging generic constraints is a major pain point and we can't simply remove all constraint checks and make them dynamic or analyzers because we've hit some places where it'd be beneficial to go from A to B.

Removing constraints to work around the lack of bridging works today, but is also irreversible once its done and removes a level of safety. For ever case where we think removing the constraint would be beneficial, we really need to look deeper and see if there is a reasonable workaround and the implications of removing the constraint.

For this particular scenario, we explicitly have 3 related APIs:

• TTo Unsafe.As<TTo>(object obj) where TTo : class
• TTo BitCast<TFrom, TTo>(TFrom value) where TFrom : struct, where TTo : struct
• ref TTo Unsafe.As<TFrom, TTo>(ref TFrom value)

The first is for reinterpreting an object reference, the second for reinterpreting identically sized value types, and the third for reinterpreting arbitrary references.

Given that, I think its better to have devs use the trivial workaround that is ref TTo Unsafe.As<TFrom, TTo>(ref TFrom) instead. It's potentially not as optimal, but the JIT will do the right thing in most cases. Then, BitCast will become available for use in unconstrained code when bridging generic constraints is resolved and otherwise exists for correctly constrained scenarios already.

[stephentoub]

I think its better to have devs use the trivial workaround that is ref TTo Unsafe.As<TFrom, TTo>(ref TFrom)

It's annoying to use in that capacity when dealing with the return value of a call, forcing you to store into a local. And a key reason BitCast was added was to handle the scenarios where Unsafe.As was already available and being used. Further, the constraint already isn't complete, in that you can still meet the constraint but be using args that fail the actual requirements of the method and still get an exception.

Even if/when bridging support is available, it's simpler to consume without the constraint, and is already defined in a type that yells "there be dragons". If you get an exception from using it incorrectly, as you already can, so be it.

[tannergooding]

I think this is going to come down to a difference of opinion.

I agree some of the annoyances exist and that it would be great to get rid of them, I don't think those annoyances are sufficient justification to make the API even more unsafe, particularly where a valid workaround exists and we know we need a longer term language/runtime feature in this space.

[stephentoub]

I don't think those annoyances are sufficient justification to make the API even more unsafe

It doesn't make it any more unsafe, though. Invalid types would still produce an exception. It's purely a matter of whether misuse compiles or not, trading off whether valid use compiles or not, too.

[EgorBo]

👍 to the proposal, the only two cases I ever needed this API I bumped into the same annoying limitation.

[jkotas]

It doesn't make it any more unsafe, though. Invalid types would still produce an exception.

Would Unsafe.BitCast<MyClass, MyStruct>(c) be valid or invalid? In other words, is the proposal to drop the constraint completely, or replace the constraint with a dynamic check?

[stephentoub]

replace the constraint with a dynamic check?

This, which would evaporate for valid use via it being a JIT intrinsic.

[MihaZupan]

Invalid. It would throw at runtime, same as BitCast<MyStruct1, MyStruct2>() does today if the two types aren't the same size, or their layout isn't compatible.

That is, there are already cases where you can compile a call to this API and have it throw at runtime, this just adds one more.

[MichalPetryka]

their layout isn't compatible.

That's not actually the case, it lets you reinterpret (object, object) to (nuint, nuint) and such today.

[jkotas]

replace the constraint with a dynamic check?

This, which would evaporate for valid use via it being a JIT intrinsic.

Ok, that sounds reasonable to me.

[hamarb123]

Today our logic is basically this:

• If sizeof(TFrom) != sizeof(TTo), then throw an exception
• If layouts (where GC holes are) don't match, we effectively do Unsafe.As with unaligned read as required
• Otherwise, we usually generate more optimised code if possible
• Implemented here:

  runtime/src/coreclr/jit/importercalls.cpp

  Line 4456 in 8bd4f6f

  case NI_SRCS_UNSAFE_BitCast:

Do we plan to keep this logic when expanding to general TFrom and TTo? I think we should personally.
It would mean the following new operations should work if so (assuming expected layouts of structs):

• BitCast<Class1, Class2>
• BitCast<object, ValueTuple<object>>
• BitCast<int?, long>
• BitCast<object, nuint>
• BitCast<ImmutableArray<T>, T[]>

(obviously, the safety of each of the above is the same as the safety of Unsafe.As up to alignment and sizes)

This would result in the following APIs:

• As<TFrom, TTo>(TFrom&) - reinterpret references (of any type)
• BitCast<TFrom, TTo>(TFrom) - reinterpret values (of any type, to same size)
• As<T>(object) - reinterpret class types (to other class types) - useful to avoid the extra generic parameter when it's not needed

I think this is the set of APIs that makes the most sense conceptually and would be the most useful practically.

[MihaZupan]

My proposal is to keep the supported scenarios the same as part of this change. Passing in a TFrom that isn't a struct doesn't compile today and would now predictably throw at runtime.

The intention of the API as I understood it was to offer "safer" casting that would reject likely erroneous uses.
During the implementation, the decision was made to also support types that contain references.
I think the current behavior of allowing casts between incompatible types with references is a bug that should be fixed instead of doubling down and allowing even more cases (e.g. (object, object) to (nuint, nuint) as Michał mentioned).

[hamarb123]

I think the current behavior of allowing casts between incompatible types with references is a bug that should be fixed instead of doubling down and allowing even more cases (e.g. (object, object) to (nuint, nuint) as Michał mentioned).

This is already allowed...

The intention of the API as I understood it was to offer "safer" casting that would reject likely erroneous uses

I personally never understood this to be the intent - the only safety I'm aware of is only allowing types with the same size, since that's required for the operation to even make sense. It's on the Unsafe class anyway as far as I'm concerned, callers should be checking that their operation is valid in advance anyway - if we wanted it to be safe in every case, we should have put it on a different type to indicate that.

The intent of the API as I understood it was to allow value reinterpretation; As is designed for reference reinterpretation; the same annoyance of having to create a temp local to use As that apply for structs also apply for all types - why is it not worth simplifying for the general case? And why didn't that logic apply to all structs also when we first added the BitCast method?

My proposal is to keep the supported scenarios the same as part of this change

So would you want to make Unsafe.BitCast<int?, long> throw (since it's currently not allowed)? What would that achieve?

[bartonjs]

Video

• Once we're removing one constraint we should remove both.
• We briefly entertained renaming the Generic Type Parameters (e.g. TStructFrom), but didn't feel it had sufficient value.

namespace System.Runtime.CompilerServices;

public static class Unsafe
{
    public static TTo BitCast<TFrom, TTo>(TFrom value)
-       where TFrom : struct
-       where TTo : struct;
}