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

Specially handle more scenarios for type parameters with 'allows ref struct' constraint #73111

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 2 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4025,7 +4025,7 @@ private BoundExpression BindImplicitArrayCreationExpression(ImplicitArrayCreatio
bestType = CreateErrorType();
}

if (bestType.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Test this for 'allows ref struct' type parameters?
if (bestType.IsRestrictedType())
{
// CS0611: Array elements cannot be of type '{0}'
Error(diagnostics, ErrorCode.ERR_ArrayElementCantBeRefAny, node, bestType);
Expand Down Expand Up @@ -7503,7 +7503,7 @@ private BoundExpression BindDynamicMemberAccess(
for (int i = 0; i < typeArgumentsWithAnnotations.Length; ++i)
{
var typeArgument = typeArgumentsWithAnnotations[i];
if (typeArgument.Type.IsPointerOrFunctionPointer() || typeArgument.Type.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing?
if (typeArgument.Type.IsPointerOrFunctionPointer() || typeArgument.Type.IsRestrictedType())
{
// "The type '{0}' may not be used as a type argument"
Error(diagnostics, ErrorCode.ERR_BadTypeArgument, typeArgumentsSyntax[i], typeArgument.Type);
Expand Down Expand Up @@ -10858,7 +10858,6 @@ private BoundConditionalAccess BindConditionalAccessExpression(ConditionalAccess
// - access cannot have unconstrained generic type
// - access cannot be a pointer
// - access cannot be a restricted type
// PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if ((!accessType.IsReferenceType && !accessType.IsValueType) || accessType.IsPointerOrFunctionPointer() || accessType.IsRestrictedType())
{
// Result type of the access is void when result value cannot be made nullable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,6 @@ private void CheckRestrictedTypeReceiver(BoundExpression expression, CSharpCompi
// error CS0029: Cannot implicitly convert type 'A' to 'B'

// Case 1: receiver is a restricted type, and method called is defined on a parent type
// PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (call.ReceiverOpt.Type.IsRestrictedType() && !call.Method.ContainingType.IsInterface && !TypeSymbol.Equals(call.Method.ContainingType, call.ReceiverOpt.Type, TypeCompareKind.ConsiderEverything2))
{
SymbolDistinguisher distinguisher = new SymbolDistinguisher(compilation, call.ReceiverOpt.Type, call.Method.ContainingType);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ private static bool IsLegalDynamicOperand(BoundExpression operand)

// Pointer types and very special types are not convertible to object.

return !type.IsPointerOrFunctionPointer() && !type.IsRestrictedType() && !type.IsVoidType(); // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
return !type.IsPointerOrFunctionPointer() && !type.IsRestrictedType() && !type.IsVoidType();
}

private BoundExpression BindDynamicBinaryOperator(
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ private TypeWithAnnotations BindArrayType(
// Restricted types cannot be on the heap, but they can be on the stack, so are allowed in a stackalloc
if (ShouldCheckConstraints)
{
if (type.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (type.IsRestrictedType())
{
// CS0611: Array elements cannot be of type '{0}'
Error(diagnostics, ErrorCode.ERR_ArrayElementCantBeRefAny, node.ElementType, type.Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ private static Conversion ToConversion(OverloadResolutionResult<MethodSymbol> re
}

//cannot capture stack-only types.
if (method.RequiresInstanceReceiver && methodGroup.Receiver?.Type?.IsRestrictedType() == true) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (method.RequiresInstanceReceiver && methodGroup.Receiver?.Type?.IsRestrictedType() == true)
{
return Conversion.NoConversion;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2829,7 +2829,8 @@ public bool HasImplicitTypeParameterConversion(TypeParameterSymbol source, TypeS
return true;
}

if ((destination.TypeKind == TypeKind.TypeParameter) &&
if (destination is TypeParameterSymbol { AllowByRefLike: false } &&
!source.AllowByRefLike &&
Copy link
Member

@cston cston Apr 22, 2024

Choose a reason for hiding this comment

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

!source.AllowByRefLike &&

Are we testing this case? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we testing this case?

I tried, but given the convoluted nature of the conversion classification algorithms, I was not able to find a scenario where we get here with source.AllowByRefLike equals true. Nevertheless, I am fairly comfortable making this change regardless of the fact.

source.DependsOn((TypeParameterSymbol)destination))
{
return true;
Expand All @@ -2848,7 +2849,10 @@ private bool HasImplicitReferenceTypeParameterConversion(TypeParameterSymbol sou
return false; // Not a reference conversion.
}

// PROTOTYPE(RefStructInterfaces): Check for AllowByRefLike?
if (source.AllowByRefLike)
{
return false;
}

// The following implicit conversions exist for a given type parameter T:
//
Expand All @@ -2868,7 +2872,7 @@ private bool HasImplicitReferenceTypeParameterConversion(TypeParameterSymbol sou
}

// * From T to a type parameter U, provided T depends on U.
if ((destination.TypeKind == TypeKind.TypeParameter) &&
if (destination is TypeParameterSymbol { AllowByRefLike: false } &&
Copy link
Member

@cston cston Apr 22, 2024

Choose a reason for hiding this comment

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

if (destination is TypeParameterSymbol { AllowByRefLike: false } &&

Why is AllowByRefLike: false required? Isn't it sufficient to check source.AllowByRefLike (above) and source.DependsOn((TypeParameterSymbol)destination)? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is AllowByRefLike: false required? Isn't it sufficient to check source.AllowByRefLike (above) and source.DependsOn((TypeParameterSymbol)destination)?

Why do you think it should be sufficient? Dependency between type parameters says nothing about their AllowByRefLike property (dependent type parameters can have different values for it) and reference conversions do not exist for those that have AllowByRefLike: true.

source.DependsOn((TypeParameterSymbol)destination))
{
return true;
Expand Down Expand Up @@ -3221,8 +3225,8 @@ private bool HasImplicitBoxingTypeParameterConversion(TypeParameterSymbol source
}

// SPEC: From T to a type parameter U, provided T depends on U
if ((destination.TypeKind == TypeKind.TypeParameter) &&
source.DependsOn((TypeParameterSymbol)destination))
if (destination is TypeParameterSymbol { AllowByRefLike: false } d &&
Copy link
Member

@cston cston Apr 22, 2024

Choose a reason for hiding this comment

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

AllowByRefLike: false

Why is AllowByRefLike: false required? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is AllowByRefLike: false required?

Same response as for the change on line 2875

source.DependsOn(d))
{
return true;
}
Expand Down Expand Up @@ -3280,7 +3284,7 @@ public bool HasBoxingConversion(TypeSymbol source, TypeSymbol destination, ref C
// There are a couple of exceptions. The very special types ArgIterator, ArgumentHandle and
// TypedReference are not boxable:

if (source.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (source.IsRestrictedType())
{
return false;
}
Expand Down Expand Up @@ -3476,6 +3480,11 @@ private bool HasExplicitReferenceTypeParameterConversion(TypeSymbol source, Type
TypeParameterSymbol s = source as TypeParameterSymbol;
TypeParameterSymbol t = destination as TypeParameterSymbol;

if (s?.AllowByRefLike == true || t?.AllowByRefLike == true)
{
return false;
}

// SPEC: The following explicit conversions exist for a given type parameter T:

// SPEC: If T is known to be a reference type, the conversions are all classified as explicit reference conversions.
Expand Down Expand Up @@ -3523,6 +3532,11 @@ private bool HasUnboxingTypeParameterConversion(TypeSymbol source, TypeSymbol de
TypeParameterSymbol s = source as TypeParameterSymbol;
TypeParameterSymbol t = destination as TypeParameterSymbol;

if (s?.AllowByRefLike == true || t?.AllowByRefLike == true)
{
return false;
}

// SPEC: The following explicit conversions exist for a given type parameter T:

// SPEC: If T is known to be a reference type, the conversions are all classified as explicit reference conversions.
Expand Down Expand Up @@ -3753,7 +3767,7 @@ private bool HasUnboxingConversion(TypeSymbol source, TypeSymbol destination, re
}

// Ref-like types cannot be boxed or unboxed
if (destination.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (destination.IsRestrictedType())
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected override DiagnosticInfo GetInstanceWithSeverityCore(DiagnosticSeverity

protected override DiagnosticInfo ResolveInfo()
{
if (_possiblyRestrictedTypeSymbol.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (_possiblyRestrictedTypeSymbol.IsRestrictedType())
{
return new CSDiagnosticInfo(ErrorCode.ERR_ArrayElementCantBeRefAny, _possiblyRestrictedTypeSymbol.Type);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ public override BoundNode Visit(BoundNode node)
!(node is BoundConversion) &&
node is BoundExpression expr &&
expr.Type is TypeSymbol type &&
type.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
type.IsRestrictedType())
{
Error(ErrorCode.ERR_ExpressionTreeCantContainRefStruct, node, type.Name);
}
Expand Down Expand Up @@ -576,7 +576,7 @@ public override BoundNode VisitLambda(BoundLambda node)
{
_diagnostics.Add(ErrorCode.ERR_ByRefParameterInExpressionTree, location);
}
if (p.TypeWithAnnotations.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (p.TypeWithAnnotations.IsRestrictedType())
{
_diagnostics.Add(ErrorCode.ERR_ExpressionTreeCantContainRefStruct, p.GetFirstLocation(), p.Type.Name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public override bool AllowByRefLike
{
get
{
return _container.IsDelegateType() && _container.Manager.Compilation.SourceAssembly.RuntimeSupportsByRefLikeGenerics;
return _container.IsDelegateType() && ContainingAssembly.RuntimeSupportsByRefLikeGenerics;
}
}

Expand Down
Loading