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

Allow ref structs to implement interfaces, respect "allows ref struct" during constraints check. #72537

Merged
merged 2 commits into from
Mar 15, 2024
Merged

Allow ref structs to implement interfaces, respect "allows ref struct" during constraints check. #72537

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf36ed3
Allow ref structs to implement interfaces, respect "allows refg struc…
AlekseyTs Mar 14, 2024
d933789
PR feedback
AlekseyTs Mar 14, 2024
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
16 changes: 13 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8282,10 +8282,14 @@ private void CheckReceiverAndRuntimeSupportForSymbolAccess(SyntaxNode node, Boun
}
}

if (!Compilation.Assembly.RuntimeSupportsDefaultInterfaceImplementation && Compilation.SourceModule != symbol.ContainingModule)
if (receiverOpt is { Type: TypeParameterSymbol { AllowByRefLike: true } } &&
isNotImplementableInstanceMember(symbol))
{
if (!symbol.IsStatic && !(symbol is TypeSymbol) &&
!symbol.IsImplementableInterfaceMember())
Error(diagnostics, ErrorCode.ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike, node);
}
else if (!Compilation.Assembly.RuntimeSupportsDefaultInterfaceImplementation && Compilation.SourceModule != symbol.ContainingModule)
{
if (isNotImplementableInstanceMember(symbol))
{
Error(diagnostics, ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, node);
}
Expand All @@ -8303,6 +8307,12 @@ private void CheckReceiverAndRuntimeSupportForSymbolAccess(SyntaxNode node, Boun
}
}
}

static bool isNotImplementableInstanceMember(Symbol symbol)
{
return !symbol.IsStatic && !(symbol is TypeSymbol) &&
!symbol.IsImplementableInterfaceMember();
}
}

private BoundExpression BindEventAccess(
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2921,6 +2921,11 @@ private bool HasImplicitEffectiveInterfaceSetConversion(TypeParameterSymbol sour
}

private bool HasAnyBaseInterfaceConversion(TypeSymbol derivedType, TypeSymbol baseType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
return ImplementsVarianceCompatibleInterface(derivedType, baseType, ref useSiteInfo);
}

internal bool ImplementsVarianceCompatibleInterface(TypeSymbol derivedType, TypeSymbol baseType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert((object)derivedType != null);
Debug.Assert((object)baseType != null);
Expand Down
12 changes: 9 additions & 3 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2105,9 +2105,6 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_StaticClassInterfaceImpl" xml:space="preserve">
<value>'{0}': static classes cannot implement interfaces</value>
</data>
<data name="ERR_RefStructInterfaceImpl" xml:space="preserve">
<value>'{0}': ref structs cannot implement interfaces</value>
</data>
<data name="ERR_OperatorInStaticClass" xml:space="preserve">
<value>'{0}': static classes cannot contain user-defined operators</value>
</data>
Expand Down Expand Up @@ -7917,4 +7914,13 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ClassIsCombinedWithRefStruct" xml:space="preserve">
<value>Cannot allow ref structs for a type parameter known from other constraints to be a class</value>
</data>
<data name="ERR_NotRefStructConstraintNotSatisfied" xml:space="preserve">
<value>The type '{2}' may not be a ref struct or a type parameter allowing ref structs in order to use it as parameter '{1}' in the generic type or method '{0}'</value>
Copy link
Member

@cston cston Mar 14, 2024 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

The type '{2}' may not be a ref struct or a type parameter allowing ref structs in order to use it as parameter '{1}' in the generic type or method '{0}'

Perhaps: "The type ... must not be a ref struct ... in order to use it as type parameter ..." #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.

For "may not be a", I think I used the following message as a template:

  <data name="ERR_LambdaInIsAs" xml:space="preserve">
    <value>The first operand of an 'is' or 'as' operator may not be a lambda expression, anonymous method, or method group.</value>
  </data>

For "in order to use it as parameter", there is a "prior art" too:

  <data name="ERR_RefConstraintNotSatisfied" xml:space="preserve">
    <value>The type '{2}' must be a reference type in order to use it as parameter '{1}' in the generic type or method '{0}'</value>
  </data>

I can make the suggested change if you still think it is needed. Please let me know.

</data>
<data name="ERR_RefStructDoesNotSupportDefaultInterfaceImplementationForMember" xml:space="preserve">
<value>'{0}' cannot implement interface member '{1}' for ref struct '{2}'.</value>
</data>
<data name="ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike" xml:space="preserve">
<value>A non-virtual instance interface member cannot be accessed on a type parameter that allows ref struct.</value>
</data>
</root>
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1528,7 +1528,7 @@ internal enum ErrorCode
ERR_FieldsInRoStruct = 8340,
ERR_AutoPropsInRoStruct = 8341,
ERR_FieldlikeEventsInRoStruct = 8342,
ERR_RefStructInterfaceImpl = 8343,
// ERR_RefStructInterfaceImpl = 8343,
ERR_BadSpecialByRefIterator = 8344,
ERR_FieldAutoPropCantBeByRefLike = 8345,
ERR_StackAllocConversionNotPossible = 8346,
Expand Down Expand Up @@ -2304,6 +2304,9 @@ internal enum ErrorCode
ERR_RefStructConstraintAlreadySpecified = 9501,
ERR_AllowsClauseMustBeLast = 9502,
ERR_ClassIsCombinedWithRefStruct = 9503,
ERR_NotRefStructConstraintNotSatisfied = 9504,
ERR_RefStructDoesNotSupportDefaultInterfaceImplementationForMember = 9505,
ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike = 9506,

#endregion

Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1839,7 +1839,6 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_FieldsInRoStruct:
case ErrorCode.ERR_AutoPropsInRoStruct:
case ErrorCode.ERR_FieldlikeEventsInRoStruct:
case ErrorCode.ERR_RefStructInterfaceImpl:
case ErrorCode.ERR_BadSpecialByRefIterator:
case ErrorCode.ERR_FieldAutoPropCantBeByRefLike:
case ErrorCode.ERR_StackAllocConversionNotPossible:
Expand Down Expand Up @@ -2434,6 +2433,9 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_RefStructConstraintAlreadySpecified:
case ErrorCode.ERR_AllowsClauseMustBeLast:
case ErrorCode.ERR_ClassIsCombinedWithRefStruct:
case ErrorCode.ERR_NotRefStructConstraintNotSatisfied:
case ErrorCode.ERR_RefStructDoesNotSupportDefaultInterfaceImplementationForMember:
case ErrorCode.ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
45 changes: 38 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,13 +927,37 @@ private static bool CheckBasicConstraints(
ArrayBuilder<TypeParameterDiagnosticInfo> nullabilityDiagnosticsBuilderOpt,
ref ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder)
{
if (typeArgument.Type.IsPointerOrFunctionPointer() || typeArgument.IsRestrictedType() || typeArgument.IsVoidType())
if (typeArgument.Type.IsPointerOrFunctionPointer() || typeArgument.IsRestrictedType(ignoreSpanLikeTypes: true) || typeArgument.IsVoidType())
{
// "The type '{0}' may not be used as a type argument"
diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new UseSiteInfo<AssemblySymbol>(new CSDiagnosticInfo(ErrorCode.ERR_BadTypeArgument, typeArgument.Type))));
return false;
}

if (typeArgument.IsRefLikeType() || typeArgument.Type is TypeParameterSymbol { AllowByRefLike: true })
{
if (typeParameter.AllowByRefLike)
{
if (args.CurrentCompilation.SourceModule != typeParameter.ContainingModule)
{
if (MessageID.IDS_FeatureRefStructInterfaces.GetFeatureAvailabilityDiagnosticInfo(args.CurrentCompilation) is { } diagnosticInfo)
{
diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new UseSiteInfo<AssemblySymbol>(diagnosticInfo)));
}

if (!args.CurrentCompilation.Assembly.RuntimeSupportsByRefLikeGenerics)
{
diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new UseSiteInfo<AssemblySymbol>(new CSDiagnosticInfo(ErrorCode.ERR_RuntimeDoesNotSupportByRefLikeGenerics))));
}
}
}
else
{
diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new UseSiteInfo<AssemblySymbol>(new CSDiagnosticInfo(ErrorCode.ERR_NotRefStructConstraintNotSatisfied, containingSymbol.ConstructedFrom(), typeParameter, typeArgument.Type))));
return false;
}
}

if (typeArgument.IsStatic)
{
// "'{0}': static types cannot be used as type arguments"
Expand Down Expand Up @@ -1307,13 +1331,20 @@ private static bool SatisfiesConstraintType(
return true;
}

// "... A boxing conversion (6.1.7), provided that type A is a non-nullable value type. ..."
// NOTE: we extend this to allow, for example, a conversion from Nullable<T> to object.
if (typeArgument.Type.IsValueType &&
conversions.HasBoxingConversion(typeArgument.Type.IsNullableType() ? ((NamedTypeSymbol)typeArgument.Type).ConstructedFrom : typeArgument.Type,
constraintType.Type, ref useSiteInfo))
if (typeArgument.Type.IsValueType)
{
return true;
// "... A boxing conversion (6.1.7), provided that type A is a non-nullable value type. ..."
// NOTE: we extend this to allow, for example, a conversion from Nullable<T> to object.
if (conversions.HasBoxingConversion(typeArgument.Type.IsNullableType() ? ((NamedTypeSymbol)typeArgument.Type).ConstructedFrom : typeArgument.Type,
constraintType.Type, ref useSiteInfo))
{
return true;
}

if (typeArgument.Type.IsRefLikeType && conversions.ImplementsVarianceCompatibleInterface(typeArgument.Type, constraintType.Type, ref useSiteInfo))
Copy link
Member

@jjonescz jjonescz Mar 15, 2024 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

Why is this check needed specifically for ref struct type arguments - i.e., why they need different handling then normal structs for example? #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 this check needed specifically for ref struct type arguments - i.e., why they need different handling then normal structs for example?

There are no boxing conversion for ref structs, therefore, the conversions.HasBoxingConversion check above fails.

{
return true;
}
}

if (typeArgument.TypeKind == TypeKind.TypeParameter)
Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,7 @@ private Tuple<NamedTypeSymbol, ImmutableArray<NamedTypeSymbol>> MakeOneDeclaredB

if (this.IsRefLikeType)
{
// '{0}': ref structs cannot implement interfaces
diagnostics.Add(ErrorCode.ERR_RefStructInterfaceImpl, location, this);
Binder.CheckFeatureAvailability(typeSyntax, MessageID.IDS_FeatureRefStructInterfaces, diagnostics);
}

if (baseType.ContainsDynamic())
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ public static Symbol ConstructedFrom(this Symbol symbol)
switch (symbol.Kind)
{
case SymbolKind.NamedType:
case SymbolKind.ErrorType:
return ((NamedTypeSymbol)symbol).ConstructedFrom;

case SymbolKind.Method:
Expand Down
52 changes: 31 additions & 21 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1618,34 +1618,44 @@ private static void CheckForImplementationOfCorrespondingPropertyOrEvent(MethodS

private static void ReportDefaultInterfaceImplementationMatchDiagnostics(Symbol interfaceMember, TypeSymbol implementingType, Symbol implicitImpl, BindingDiagnosticBag diagnostics)
{
if (interfaceMember.Kind == SymbolKind.Method && implementingType.ContainingModule != implicitImpl.ContainingModule)
if (interfaceMember.Kind == SymbolKind.Method)
{
// The default implementation is coming from a different module, which means that we probably didn't check
// for the required runtime capability or language version
bool isStatic = implicitImpl.IsStatic;
var feature = isStatic ? MessageID.IDS_FeatureStaticAbstractMembersInInterfaces : MessageID.IDS_DefaultInterfaceImplementation;

LanguageVersion requiredVersion = feature.RequiredVersion();
LanguageVersion? availableVersion = implementingType.DeclaringCompilation?.LanguageVersion;
if (requiredVersion > availableVersion)
if (!isStatic && implementingType.IsRefLikeType)
{
diagnostics.Add(ErrorCode.ERR_LanguageVersionDoesNotSupportInterfaceImplementationForMember,
diagnostics.Add(ErrorCode.ERR_RefStructDoesNotSupportDefaultInterfaceImplementationForMember,
GetInterfaceLocation(interfaceMember, implementingType),
implicitImpl, interfaceMember, implementingType,
feature.Localize(),
availableVersion.GetValueOrDefault().ToDisplayString(),
new CSharpRequiredLanguageVersion(requiredVersion));
implicitImpl, interfaceMember, implementingType);
}

if (!(isStatic ?
implementingType.ContainingAssembly.RuntimeSupportsStaticAbstractMembersInInterfaces :
implementingType.ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation))
else if (implementingType.ContainingModule != implicitImpl.ContainingModule)
{
diagnostics.Add(isStatic ?
ErrorCode.ERR_RuntimeDoesNotSupportStaticAbstractMembersInInterfacesForMember :
ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementationForMember,
GetInterfaceLocation(interfaceMember, implementingType),
implicitImpl, interfaceMember, implementingType);
// The default implementation is coming from a different module, which means that we probably didn't check
// for the required runtime capability or language version
var feature = isStatic ? MessageID.IDS_FeatureStaticAbstractMembersInInterfaces : MessageID.IDS_DefaultInterfaceImplementation;

LanguageVersion requiredVersion = feature.RequiredVersion();
LanguageVersion? availableVersion = implementingType.DeclaringCompilation?.LanguageVersion;
if (requiredVersion > availableVersion)
{
diagnostics.Add(ErrorCode.ERR_LanguageVersionDoesNotSupportInterfaceImplementationForMember,
GetInterfaceLocation(interfaceMember, implementingType),
implicitImpl, interfaceMember, implementingType,
feature.Localize(),
availableVersion.GetValueOrDefault().ToDisplayString(),
new CSharpRequiredLanguageVersion(requiredVersion));
}

if (!(isStatic ?
implementingType.ContainingAssembly.RuntimeSupportsStaticAbstractMembersInInterfaces :
implementingType.ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation))
{
diagnostics.Add(isStatic ?
ErrorCode.ERR_RuntimeDoesNotSupportStaticAbstractMembersInInterfacesForMember :
ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementationForMember,
GetInterfaceLocation(interfaceMember, implementingType),
implicitImpl, interfaceMember, implementingType);
}
}
}
}
Expand Down
20 changes: 15 additions & 5 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading