C# 13: Allows ref struct.

[michaelnebel]

In this PR we introduce support for the allows ref struct type parameter constraint. The language feature is described here.

A couple of notes on the implementation.

• A ref struct type can not be implicitly converted to a dynamic type, object or ValueType.
• The allows ref struct is a negative constraint meaning that it extends the number of types that can be used as type replacement for a type parameter.

The unification and dispatch call logic has been adapted to take the allows ref struct constraint into account (relevant for dynamic dispatch) for deciding relevant dispatch targets. To make things easier a new class RefStruct has been introduced in the type hierarcy. One could consider whether we want to change the type hierarchy such that RefStruct doesn't extend Struct (and thereby ValueType). Not sure whether this is worth it.

Furthermore, we also extract the notnull general type parameter constraint.

[michaelnebel]

DCA looks good.

[Copilot]

Copilot reviewed 5 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (15)

• csharp/ql/lib/semmle/code/csharp/Conversion.qll: Language not supported
• csharp/ql/lib/semmle/code/csharp/Generics.qll: Language not supported
• csharp/ql/lib/semmle/code/csharp/Type.qll: Language not supported
• csharp/ql/lib/semmle/code/csharp/Unification.qll: Language not supported
• csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll: Language not supported
• csharp/ql/test/library-tests/csharp11/PrintAst.expected: Language not supported
• csharp/ql/test/library-tests/csharp7.2/PrintAst.expected: Language not supported
• csharp/ql/test/library-tests/csharp7.2/RefStructs.ql: Language not supported
• csharp/ql/test/library-tests/dispatch/CallContext.expected: Language not supported
• csharp/ql/test/library-tests/dispatch/CallGraph.expected: Language not supported
• csharp/ql/test/library-tests/dispatch/GetADynamicTarget.expected: Language not supported
• csharp/ql/test/library-tests/typeparameterconstraints/typeParameterConstraints.expected: Language not supported
• csharp/ql/test/library-tests/typeparameterconstraints/typeParameterConstraints.ql: Language not supported
• csharp/ql/test/library-tests/unification/Unification.expected: Language not supported
• csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameterConstraints.cs: Evaluated as low risk

Comments suppressed due to low confidence (4)

csharp/ql/test/library-tests/typeparameterconstraints/TypeParameterConstraints.cs:18

• [nitpick] The term 'allows ref struct' might be confusing. Consider renaming it to something more intuitive.

public void M7<T7>(T7 x) where T7 : allows ref struct { }

csharp/ql/test/library-tests/typeparameterconstraints/TypeParameterConstraints.cs:18

• Ensure that the new 'allows ref struct' constraint is properly tested.

public void M7<T7>(T7 x) where T7 : allows ref struct { }

csharp/ql/test/library-tests/dispatch/ViableCallable.cs:617

• [nitpick] The comment could be clearer by specifying the viable callables more explicitly. Suggest changing to: 'Viable callables: A1.M(), A2.M()'.

// Viable callable: {A1, A2}.M()

csharp/ql/test/library-tests/conversion/boxing/Boxing.cs:50

• The term 'valuetype' should be 'ValueType' to maintain consistency with C# naming conventions.

ref struct S { }

Tip: Turn on automatic Copilot reviews for this repository to get quick feedback on every pull request. Learn more

[tamasvajk]

Why don't we consider this a value type?

using System;
void M(C c) 
{
    c.F = 5;
}

var c = new C();
M(c);                   // Passed by value
Console.WriteLine(c.F); // So prints 0

ref struct C 
{
    public int F;
}

[michaelnebel]

This is definitely up for debate.
As you write, the ref struct still exhibits call-by-value semantics. However, we also decided a while back that we needed to introduce post update nodes for ref structs parameters (as ref structs can have ref fields - and in this case only the reference is copied, which from a dataflow perspective means that we "can" update a ref struct (or at least update the content of the shared references)). Furthermore, ref structs can't be boxed (implicitly converted) to ValueType (or object). My thinking is that it is "something in between" and "reference like" (this is also the terminology used in Roslyn). So I thought that it was starting to get a bit dangerous to assume that it behaves like a value type both from a type and data flow perspective.

[michaelnebel]

To continue the discussion - I don't mind that we still consider the ref struct a value type - then we can modify type conversion, unification and dataflow on a case by case when we run into problems.
Maybe that is the safer choice.

[tamasvajk]

Should the same be added to class Class?

[michaelnebel]

The intention is that this should only cover types (for now there only exists ref structs which are not value types or "ordinary" ref types).

[tamasvajk]

Makes sense. I guess that is also the behavior of ITypeSymbol.IsRefLikeType, isn't it?

[michaelnebel]

Yes, I think so (it is not specifically well documented) - classes and interfaces have IsRefLikeType = false.

[hvitved]

LGTM, just one small remark.

[hvitved]

I think this comment deserves to be elaborated; right now it simply means that is is a ref struct, right?

[michaelnebel]

Yes, that is correct.
I will elaborate.