Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C# 13: Allows ref struct. #18385

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ public override void Populate(TextWriter trapFile)
if (Symbol.ReferenceTypeConstraintNullableAnnotation == NullableAnnotation.Annotated)
trapFile.general_type_parameter_constraints(this, 5);

if (Symbol.HasNotNullConstraint)
trapFile.general_type_parameter_constraints(this, 6);

if (Symbol.AllowsRefLikeType)
trapFile.general_type_parameter_constraints(this, 7);

foreach (var abase in Symbol.GetAnnotatedTypeConstraints())
{
var t = Type.Create(Context, abase.Symbol);
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/lib/change-notes/2025-01-03-allow-ref-struct.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* C# 13: Added extractor support and call dispatch logic (data flow) for the (negative) type parameter constraint `allows ref struct`. Added extractor support for the type parameter constraint `notnull`.
13 changes: 8 additions & 5 deletions csharp/ql/lib/semmle/code/csharp/Conversion.qll
Original file line number Diff line number Diff line change
Expand Up @@ -649,11 +649,14 @@ predicate convBoxing(Type fromType, Type toType) {
}

private predicate convBoxingValueType(ValueType fromType, Type toType) {
toType instanceof ObjectType
or
toType instanceof DynamicType
or
toType instanceof SystemValueTypeClass
(
toType instanceof ObjectType
or
toType instanceof DynamicType
or
toType instanceof SystemValueTypeClass
) and
not fromType.isRefLikeType()
or
toType = fromType.getABaseInterface+()
}
Expand Down
6 changes: 6 additions & 0 deletions csharp/ql/lib/semmle/code/csharp/Generics.qll
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@ class TypeParameterConstraints extends Element, @type_parameter_constraints {
/** Holds if these constraints include a nullable reference type constraint. */
predicate hasNullableRefTypeConstraint() { general_type_parameter_constraints(this, 5) }

/** Holds if these constraints include a notnull type constraint. */
predicate hasNotNullTypeConstraint() { general_type_parameter_constraints(this, 6) }

/** Holds if these constraints include a `allows ref struct` constraint. */
predicate hasAllowRefLikeTypeConstraint() { general_type_parameter_constraints(this, 7) }

/** Gets a textual representation of these constraints. */
override string toString() { result = "where " + this.getTypeParameter().getName() + ": ..." }

Expand Down
32 changes: 30 additions & 2 deletions csharp/ql/lib/semmle/code/csharp/Type.qll
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ class Type extends Member, TypeContainer, @type {

/** Holds if this type is a value type, or a type parameter that is a value type. */
predicate isValueType() { none() }

/**
* Holds if this type is a ref like type.
*/
predicate isRefLikeType() { none() }
}

pragma[nomagic]
Expand Down Expand Up @@ -704,15 +709,38 @@ class Enum extends ValueType, @enum_type {
* ```
*/
class Struct extends ValueType, @struct_type {
/** Holds if this `struct` has a `ref` modifier. */
predicate isRef() { this.hasModifier("ref") }
/**
* DEPRECATED: Use `instanceof RefStruct` instead.
*
* Holds if this `struct` has a `ref` modifier.
*/
deprecated predicate isRef() { this.hasModifier("ref") }

/** Holds if this `struct` has a `readonly` modifier. */
predicate isReadonly() { this.hasModifier("readonly") }

override string getAPrimaryQlClass() { result = "Struct" }
}

/**
* A `ref struct`, for example
*
* ```csharp
* ref struct S {
* ...
* }
* ```
*/
class RefStruct extends Struct {
RefStruct() { this.hasModifier("ref") }

override string getAPrimaryQlClass() { result = "RefStruct" }

override predicate isValueType() { none() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


override predicate isRefLikeType() { any() }
}

/**
* A `record struct`, for example
* ```csharp
Expand Down
38 changes: 29 additions & 9 deletions csharp/ql/lib/semmle/code/csharp/Unification.qll
Original file line number Diff line number Diff line change
Expand Up @@ -522,16 +522,21 @@ module Gvn {

/** Provides definitions related to type unification. */
module Unification {
/** A type parameter that is compatible with any type. */
/** A type parameter that is compatible with any type except `ref struct`. */
class UnconstrainedTypeParameter extends TypeParameter {
UnconstrainedTypeParameter() { not exists(getATypeConstraint(this)) }
UnconstrainedTypeParameter() {
not exists(getATypeConstraint(this)) and not exists(getANegativeTypeConstraint(this))
}
}

/** A type parameter that is constrained. */
class ConstrainedTypeParameter extends TypeParameter {
int constraintCount;

ConstrainedTypeParameter() { constraintCount = strictcount(getATypeConstraint(this)) }
ConstrainedTypeParameter() {
constraintCount = count(getATypeConstraint(this)) + count(getANegativeTypeConstraint(this)) and
constraintCount > 0
}

/**
* Holds if this type parameter is unifiable with type `t`.
Expand Down Expand Up @@ -559,29 +564,31 @@ module Unification {
bindingset[this]
pragma[inline_late]
override predicate unifiable(Type t) {
exists(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
forall(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
ttc = TRefTypeConstraint() and
t.isRefType()
or
ttc = TValueTypeConstraint() and
t.isValueType()
or
typeConstraintUnifiable(ttc, t)
)
) and
(t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
}

bindingset[this]
pragma[inline_late]
override predicate subsumes(Type t) {
exists(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
forall(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
ttc = TRefTypeConstraint() and
t.isRefType()
or
ttc = TValueTypeConstraint() and
t.isValueType()
or
typeConstraintSubsumes(ttc, t)
)
) and
(t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
}
}

Expand All @@ -603,7 +610,8 @@ module Unification {
t.isValueType()
or
typeConstraintUnifiable(ttc, t)
)
) and
(t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
}

bindingset[this]
Expand All @@ -617,7 +625,8 @@ module Unification {
t.isValueType()
or
typeConstraintSubsumes(ttc, t)
)
) and
(t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
}
}

Expand All @@ -632,6 +641,9 @@ module Unification {
not t instanceof TypeParameter
}

cached
newtype TTypeParameterNegativeConstraint = TAllowRefTypeConstraint()

cached
TTypeParameterConstraint getATypeConstraint(TypeParameter tp) {
exists(TypeParameterConstraints tpc | tpc = tp.getConstraints() |
Expand All @@ -650,6 +662,14 @@ module Unification {
)
}

cached
TTypeParameterNegativeConstraint getANegativeTypeConstraint(TypeParameter tp) {
exists(TypeParameterConstraints tpc | tpc = tp.getConstraints() |
tpc.hasAllowRefLikeTypeConstraint() and
result = TAllowRefTypeConstraint()
)
}

cached
predicate typeConstraintUnifiable(TTypeConstraint ttc, Type t) {
exists(Type t0 | ttc = TTypeConstraint(t0) | implicitConversionRestricted(t, t0))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ module LocalFlow {
or
t = any(TypeParameter tp | not tp.isValueType())
or
t.(Struct).isRef()
t.isRefLikeType()
) and
not exists(getALastEvalNode(result))
}
Expand Down
3 changes: 3 additions & 0 deletions csharp/ql/test/library-tests/conversion/boxing/Boxing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ void M()
x1 = x15; // not a boxing conversion
}
}

// Ref structs can't be converted to a dynamic, object or valuetype.
ref struct S { }
4 changes: 2 additions & 2 deletions csharp/ql/test/library-tests/csharp11/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ RequiredMembers.cs:
# 40| 0: [Parameter] value
Scoped.cs:
# 1| [Struct] S1
# 2| [Struct] S2
# 2| [RefStruct] S2
# 7| [Class] ScopedModifierTest
# 9| 5: [Method] M1
# 9| -1: [TypeMention] int
Expand Down Expand Up @@ -1402,7 +1402,7 @@ Strings.cs:
Struct.cs:
# 1| [NamespaceDeclaration] namespace ... { ... }
# 3| 1: [Class] MyEmptyClass
# 5| 2: [Struct] RefStruct
# 5| 2: [RefStruct] RefStruct
# 7| 5: [Field] MyInt
# 7| -1: [TypeMention] int
# 8| 6: [Field] MyByte
Expand Down
4 changes: 2 additions & 2 deletions csharp/ql/test/library-tests/csharp7.2/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ csharp72.cs:
# 26| 0: [FieldAccess] access to field s
# 29| 7: [DelegateType] Del
# 32| [Struct] ReadonlyStruct
# 36| [Struct] RefStruct
# 40| [Struct] ReadonlyRefStruct
# 36| [RefStruct] RefStruct
# 40| [RefStruct] ReadonlyRefStruct
# 44| [Class] NumericLiterals
# 46| 5: [Field] binaryValue
# 46| -1: [TypeMention] int
Expand Down
6 changes: 2 additions & 4 deletions csharp/ql/test/library-tests/csharp7.2/RefStructs.ql
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import csharp

from Struct s
where
s.fromSource() and
s.isRef()
from RefStruct s
where s.fromSource()
select s
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ mayBenefitFromCallContext
| ViableCallable.cs:576:18:576:22 | call to operator / |
| ViableCallable.cs:579:26:579:30 | call to operator checked / |
| ViableCallable.cs:585:9:585:15 | call to method M12 |
| ViableCallable.cs:618:9:618:13 | call to method M |
3 changes: 3 additions & 0 deletions csharp/ql/test/library-tests/dispatch/CallGraph.expected
Original file line number Diff line number Diff line change
Expand Up @@ -259,3 +259,6 @@
| ViableCallable.cs:555:10:555:15 | Run`1 | ViableCallable.cs:550:40:550:40 | checked / |
| ViableCallable.cs:555:10:555:15 | Run`1 | ViableCallable.cs:552:17:552:19 | M11 |
| ViableCallable.cs:555:10:555:15 | Run`1 | ViableCallable.cs:553:17:553:19 | M12 |
| ViableCallable.cs:609:17:609:23 | Run1`1 | ViableCallable.cs:601:21:601:21 | M |
| ViableCallable.cs:615:17:615:23 | Run2`1 | ViableCallable.cs:601:21:601:21 | M |
| ViableCallable.cs:615:17:615:23 | Run2`1 | ViableCallable.cs:606:21:606:21 | M |
Original file line number Diff line number Diff line change
Expand Up @@ -505,3 +505,6 @@
| ViableCallable.cs:585:9:585:15 | call to method M12 | C20.M12() |
| ViableCallable.cs:585:9:585:15 | call to method M12 | I3<T>.M12() |
| ViableCallable.cs:588:9:588:15 | call to method M13 | I3<T>.M13() |
| ViableCallable.cs:612:9:612:13 | call to method M | C21+A1.M() |
| ViableCallable.cs:618:9:618:13 | call to method M | C21+A1.M() |
| ViableCallable.cs:618:9:618:13 | call to method M | C21+A2.M() |
30 changes: 30 additions & 0 deletions csharp/ql/test/library-tests/dispatch/ViableCallable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -588,3 +588,33 @@ void Run<T>(T c) where T : I3<T>
c.M13();
}
}

public class C21
{
public interface I
{
void M();
}

public class A1 : I
{
public void M() { }
}

public ref struct A2 : I
{
public void M() { }
}

public void Run1<T>(T t) where T : I
{
// Viable callable: A1.M()
t.M();
}

public void Run2<T>(T t) where T : I, allows ref struct
{
// Viable callable: {A1, A2}.M()
t.M();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;
using System.Collections.Generic;

public class TestClass
{
public void M1<T1>(T1 x) where T1 : class { }

public void M2<T2>(T2 x) where T2 : struct { }

public void M3<T3>(T3 x) where T3 : unmanaged { }

public void M4<T4>(T4 x) where T4 : new() { }

public void M5<T5>(T5 x) where T5 : notnull { }

public void M6<T6>(T6 x) where T6 : IList<object> { }

public void M7<T7>(T7 x) where T7 : allows ref struct { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
typeParameterContraints
| TypeParameterConstraints.cs:6:20:6:21 | T1 | file://:0:0:0:0 | where T1: ... |
| TypeParameterConstraints.cs:8:20:8:21 | T2 | file://:0:0:0:0 | where T2: ... |
| TypeParameterConstraints.cs:10:20:10:21 | T3 | file://:0:0:0:0 | where T3: ... |
| TypeParameterConstraints.cs:12:20:12:21 | T4 | file://:0:0:0:0 | where T4: ... |
| TypeParameterConstraints.cs:14:20:14:21 | T5 | file://:0:0:0:0 | where T5: ... |
| TypeParameterConstraints.cs:16:20:16:21 | T6 | file://:0:0:0:0 | where T6: ... |
| TypeParameterConstraints.cs:18:20:18:21 | T7 | file://:0:0:0:0 | where T7: ... |
specificParameterConstraints
| TypeParameterConstraints.cs:16:20:16:21 | T6 | IList<object> |
hasConstructorConstraint
| TypeParameterConstraints.cs:12:20:12:21 | T4 | file://:0:0:0:0 | where T4: ... |
hasRefTypeConstraint
| TypeParameterConstraints.cs:6:20:6:21 | T1 | file://:0:0:0:0 | where T1: ... |
hasValueTypeConstraint
| TypeParameterConstraints.cs:8:20:8:21 | T2 | file://:0:0:0:0 | where T2: ... |
| TypeParameterConstraints.cs:10:20:10:21 | T3 | file://:0:0:0:0 | where T3: ... |
hasUnmanagedTypeConstraint
| TypeParameterConstraints.cs:10:20:10:21 | T3 | file://:0:0:0:0 | where T3: ... |
hasNullableRefTypeConstraint
hasNotNullConstraint
| TypeParameterConstraints.cs:14:20:14:21 | T5 | file://:0:0:0:0 | where T5: ... |
hasAllowRefLikeTypeConstraint
| TypeParameterConstraints.cs:18:20:18:21 | T7 | file://:0:0:0:0 | where T7: ... |
Loading
Loading