[API Proposal]: Add mechanism for indicating a method/constructor/property violates constraints

[AaronRobinsonMSFT]

Background and motivation

Implementing support for ByRefLike types as generic parameters exposes some existing APIs to unsafe behavior. The safety of these types is enforced at run-time, but violate compilation rules. This attribute let's existing APIs indicate the check will be handled at run-time and the compiler can ignore the potential violation. Additional details can be found in the design document - byreflike-generics.md.

Troublesome APIs:

• Span<T>
  • public Span(T[]? array);
  • public Span(T[]? array, int start, int length);
  • public T[] ToArray();
  • public static implicit operator Span<T>(ArraySegment<T> segment);
  • public static implicit operator Span<T>(T[]? array);
• ReadOnlySpan<T>
  • public ReadOnlySpan(T[]? array);
  • public ReadOnlySpan(T[]? array, int start, int length);
  • public T[] ToArray();
  • public static implicit operator ReadOnlySpan<T>(ArraySegment<T> segment);
  • public static implicit operator ReadOnlySpan<T>(T[]? array);

API Proposal

namespace System.Runtime.CompilerServices
{
    /// <summary>
    /// Indicates to the compiler that constraint checks should be suppressed
    /// and will instead be enforced at run-time.
    /// </summary>
    [AttributeUsage(AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property)]
    public sealed class SuppressConstraintChecksAttribute : Attribute
    { }
}

API Usage

Note the addition of the new ByRefLike constraint.

    public readonly ref struct ReadOnlySpan<T> where T: allows ref struct
    {
        ...

        [SuppressConstraintChecks]
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public ReadOnlySpan(T[]? array)

        ...
    }

Alternative Designs

No response

Risks

No response

[AaronRobinsonMSFT]

/cc @AlekseyTs @jaredpar @davidwrighton @jkotas @stephentoub @sbomer

[MichalStrehovsky]

Do we really want AttributeTargets.Property? It's inconvenient to check for that for the runtime (we'd probably still want this attribute on getters and setters as well).

[jkotas]

Is this attribute meant for everybody out there to use, or is it meant to be only used on the few methods on Span?

[AaronRobinsonMSFT]

Is this attribute meant for everybody out there to use, or is it meant to be only used on the few methods on Span?

My initial perspective was no and I planed to just mark it internal. However, I did get the below offline comment and figured it was reasonable to see the appetite for a public API.

There could be user-defined types out there, that are in the same situation that span types are. And users might want to allow ref structs for their type parameters

[AlekseyTs]

Do we really want AttributeTargets.Property? It's inconvenient to check for that for the runtime (we'd probably still want this attribute on getters and setters as well).

Is runtime going to pay attention to the attribute? Aren't constraints checked anyway?

[jkotas]

I understand how the attribute applies to the Span case. I think the general use case needs more clarification. For example:

class G<T> where T: allows ref struct
{
    [SuppressConstraintChecks]
    object M(int x)
    {
        if (x > 0)
        {
             return new T[x];
        }
        else
        {
             return null;
        }
    }
}

Does this compile? If it does compile, what is the runtime behavior of new G<Span<int>>().M(0) and new G<Span<int>>().M(1)?

[AlekseyTs]

I assume the proposal is to suppress compile time generic constraints checks in the signature and in the body of the method/property. If that is the case, consider stating that explicitly.

Do we want checks for other specific restrictions to be suppressed as well? For example, disallowing boxing for an "allows ref struct" type parameter isn't really a generic constraint check. Do we want the boxing be allowed as well? Some other "unsafe" operations?

Do we want impact of the attribute be limited only to violations around "allows ref struct"? A very broad interpretation of the attribute might lead to some surprising consequences. For example. overload resolution might start reporting an ambiguity error for some calls, because candidates that would violate generic constraints (like a class constraint, etc.) remain among the valid candidates. A different overload might be chosen, the one that will always going to violate constraints at runtime.

[AlekseyTs]

Does this compile? If it does compile, what is the runtime behavior of new G().M(0) and new G().M(1)?

At the moment C# doesn't support "ref" modifier (and any other modifiers) for type arguments. And there is no plan to change that in the current release.

[jkotas]

At the moment C# doesn't support "ref" modifier (and any other modifiers) for type arguments. And there is no plan to change that in the current release.

I have edited my example to use Span instead.

[AlekseyTs]

I assume the proposal is to suppress compile time generic constraints checks in the signature and in the body of the method/property. If that is the case, consider stating that explicitly.

Or, perhaps, the assumption is wrong and the attribute is supposed to affect the signature only. The method body is supposed to obey all the constraints and restrictions. This option is much simpler, I think.

[jkotas]

The method body is supposed to obey all the constraints and restrictions.

If the compiler allows constraints violations in the signature, it has to allow them in the method body too. Otherwise, the Span case won't work. For example, Span.ToArray violates the constraints in both method signature and method implementation.

We would need a constraint bridging feature to make it possible for the method body to obey all constraints and restrictions.

[AaronRobinsonMSFT]

If it does compile, what is the runtime behavior of new G<Span>().M(0) and new G<Span>().M(1)?

@jkotas The JIT of M() would failure.

[AaronRobinsonMSFT]

I assume the proposal is to suppress compile time generic constraints checks in the signature and in the body of the method/property.

@AlekseyTs Yes, that is the expectation. The runtime will do the check where appropriate - type load or method JIT time.

[jkotas]

@jkotas The JIT of M() would failure.

Would it be deterministic failure? I think the behavior would be non-deterministic. For example, when the JIT sees M(0), it can inline it, dead-code eliminate the unreachable branch, and never hit the type load exception from loading array of Spans. There are other situations like that.

This attribute gives Roslyn a permission to produce invalid IL. The problem is that the runtime behavior on invalid IL is undefined. We do not guarantee that the runtime is going to throw an exception or where it is going to throw the exception. Do we like that the behavior of methods marked with this attribute is undefined when called on invalid instantiation?

If we do not like the undefined behavior, I see two options:
(1) Make the attribute internal and apply it only to selected CoreLib methods that we are going to carefully implement to avoid the undefined behavior.
(2) Couple this attribute with constraints bridging and require use of constraint bridging within the body of the method to produce deterministic behavior on invalid instantiations.
These two options are not mutually exclusive. We can start with (1) and do (2) later.

[AaronRobinsonMSFT]

We do not guarantee that the runtime is going to throw an exception or where it is going to throw the exception.

Okay. So then it seems like going with option (1), which was the original plan, is the most appropriate. That plan will allow us to unblock the priority usage for [ReadOnly]Span<T> in the runtime and we can, as you mention, expand support in the future as needed.

@AlekseyTs I will update the spec and put out a PR for this soon. Let me know if you have any concerns.

[jaredpar]

public readonly ref struct ReadOnlySpan where T: allows ref struct

The Span/ReadOnlySpan<T> types cannot be marked allows ref struct. This, and the reasons for it, are called out in the spec.