Roslyn: Shouldn't the use of the new C# 7.3 `unmanaged` constraint cause a CLS compliance error?

[stakx]

This is a follow-up to https://github.com/dotnet/roslyn/issues/26751#issuecomment-388084986.

I am a bit surprised that the following program won't trigger a CLS compliance error:

using System;

[assembly: CLSCompliant(true)]

public class Type
{
    public void Method<T>() where T : unmanaged { }
}

Method's signature involves a modreq of the publicly visible type System.Runtime.InteropServices.UnmanagedType from mscorlib. In IL:

.method public hidebysig instance void Method<valuetype .ctor (class [mscorlib]System.ValueType modreq([mscorlib]System.Runtime.InteropServices.UnmanagedType)) T>() cil managed

My understanding of CLS rule 35 (cited below) is that publicly visible modreqs are disallowed. Yet I do not get any CLS warning nor error for the above program. How come?

(Click to expand:) Citation from ECMA-335, section I.9.7.

In addition to CTS type extensibility, it is possible to emit custom modifiers into member signatures (see Types in Partition II). The CLI will honor these modifiers for purposes of method overloading and hiding, as well as for binding, but will not enforce any of the language-specific semantics. These modifiers can reference the return type or any parameter of a method, or the type of a field. They come in two kinds: required modifiers that anyone using the member must understand in order to correctly use it, and optional modifiers that can be ignored if the modifier is not understood.

CLS Rule 35: The CLS does not allow publicly visible required modifiers (modreq, see Partition II), but does allow optional modifiers (modopt, see Partition II) it does not understand.

[Note:

CLS (consumer): Shall be able to read metadata containing optional modifiers and correctly copy signatures that include them. Can ignore these modifiers in type matching and overload resolution. Can ignore types that become ambiguous when the optional modifiers are ignored, or that use required modifiers.

CLS (extender): Shall be able to author overrides for inherited methods with signatures that include optional modifiers. Consequently, an extender must be able to copy such modifiers from metadata that it imports. There is no requirement to support required modifiers, nor to author new methods that have any kind of modifier in their signature.

CLS (framework): Shall not use required modifiers in externally visible signatures unless they are marked as not CLS-compliant. Shall not expose two members on a class that differ only by the use of optional modifiers in their signature, unless only one is marked CLS-compliant. end note]

(Note that this doesn't say that modreqs may target a generic type parameter, which could be due to this rule having been authored at a time when the CLI didn't yet have generics.)

[MichalStrehovsky]

This also breaks the file format because it's not legal to put the modifier where the C# compiler is placing it (per II.23.2.14 of the ECMA-335 spec a TypeSpec can only be one of the forms defined in the specification, but what the compiler is emitting would require an extra | CustomMod * Type line in the spec to be legal).

It would be better if the compiler only emitted the custom attribute (it's a custom attribute + modifier currently) and switched the detection logic to ignore the modifier. It would both fix the issue with CLS compliance and stop breaking tools that conform to the spec.

[tmat]

The ECMA specification should be corrected to allow custom mods in more places. See
https://github.com/dotnet/corefx/blob/master/src/System.Reflection.Metadata/specs/Ecma-335-Issues.md#3-custom-modifiers-can-go-in-more-places-than-specified

[MichalStrehovsky]

Yes, we can, but currently the spec doesn't allow it and tools that conform to the spec have trouble with this. If done properly, this would be a format version increment. We have seen exceptions/crashes in many runtimes and tools for modopt in the location where the C# compiler is placing it (Mono had trouble with it, .NET Native had trouble with it (we patched it), etc.).

C++/CLI historically emitted the modopts in a couple illegal locations too (not this one though), but C++/CLI has a buch of other undocumented extensions to the format and it never pretended to be spec compliant (it really targets the CLR, not ECMA-335).

Seems like ref struct managed to prevent old C# compilers from using new things in a "wrong way" using the ObsoleteAttribute - maybe we could have used that for the unmanaged constraint too. As a runtime dev, I know little about how ObsoleteAttribute works and why it was a good choice for ref struct but not for unmanaged.

[mletterle]

FWIW, strictly as a data point, the v2.0 of ilasm doesn't seem to have an issue with assembling that construct.

[MichalStrehovsky]

FWIW, strictly as a data point, the v2.0 of ilasm doesn't seem to have an issue with assembling that construct.

ILASM also lets you define a class that derives from a struct. ILASM does very little validation because it wants to allow roundripping well-formed binaries (ILDASM of a binary followed by ILASM). But well-formed doesn't mean valid.