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.

Sign up for GitHub

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 15 commits into
base: main
Choose a base branch
from
Open

C# 13: Allows ref struct. #18385

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c0974f3
C#: General type constraints tests.
michaelnebel Jan 2, 2025
958d8f1
C#: Add extractor support for the notnull general type parameter cons…
michaelnebel Jan 2, 2025
d9158c8
Fixup of second commit.
michaelnebel Jan 3, 2025
41dc4a5
C#: Add extractor support for the allows ref struct general type para…
michaelnebel Jan 2, 2025
9a2edc3
C#: Add ref struct boxing example (false positive).
michaelnebel Jan 3, 2025
ef9f09e
C#: Do not consider ref struct as being convertible to object, dynami…
michaelnebel Jan 3, 2025
b9fce5e
C#: Update boxing conversion expected output.
michaelnebel Jan 3, 2025
ef5ae3f
C#: Add some unification and viable callable test cases.
michaelnebel Jan 3, 2025
33939a8
C#: Update test expected output.
michaelnebel Jan 3, 2025
c439beb
C#: Introduce a class for ref structs.
michaelnebel Jan 3, 2025
5ddc378
C#: Update test expected output.
michaelnebel Jan 3, 2025
cac1e04
C#: Improve unification logic to handle ref structs.
michaelnebel Jan 3, 2025
ff32a38
C#: Update test expected output.
michaelnebel Jan 3, 2025
caaf291
C#: Add change note.
michaelnebel Jan 3, 2025
b358f33
C#: Address review comment.
michaelnebel Jan 10, 2025
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
6 changes: 6 additions & 0 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameterConstraints.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


override predicate isRefLikeType() { any() }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the same be added to class Class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

}

/**
* A `record struct`, for example
* ```csharp
Expand Down
38 changes: 29 additions & 9 deletions csharp/ql/lib/semmle/code/csharp/Unification.qll
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/dispatch/CallContext.expected
View file
Open in desktop
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
View file
Open in desktop
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 |
3 changes: 3 additions & 0 deletions csharp/ql/test/library-tests/dispatch/GetADynamicTarget.expected
View file
Open in desktop
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
View file
Open in desktop
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();
}
}
19 changes: 19 additions & 0 deletions csharp/ql/test/library-tests/typeparameterconstraints/TypeParameterConstraints.cs
View file
Open in desktop
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 { }
}
24 changes: 24 additions & 0 deletions csharp/ql/test/library-tests/typeparameterconstraints/typeParameterConstraints.expected
View file
Open in desktop
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