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

Extend eligible params types to array generic interfaces and span #24080

Closed
Closed
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
8 changes: 3 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2572,11 +2572,9 @@ private void CoerceArguments<TMember>(
private static TypeSymbol GetCorrespondingParameterType(ref MemberAnalysisResult result, ImmutableArray<ParameterSymbol> parameters, int arg)
{
int paramNum = result.ParameterFromArgument(arg);
var type =
(paramNum == parameters.Length - 1 && result.Kind == MemberResolutionKind.ApplicableInExpandedForm) ?
((ArrayTypeSymbol)parameters[paramNum].Type).ElementType :
parameters[paramNum].Type;
return type;
return (paramNum == parameters.Length - 1 && result.Kind == MemberResolutionKind.ApplicableInExpandedForm)
? parameters[paramNum].Type.GetElementTypeOfParamsType()
: parameters[paramNum].Type;
}

private BoundExpression BindArrayCreationExpression(ArrayCreationExpressionSyntax node, DiagnosticBag diagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ public static bool IsValidParams(Symbol member)
// Note: we need to confirm the "arrayness" on the original definition because
// it's possible that the type becomes an array as a result of substitution.
ParameterSymbol final = member.GetParameters().Last();
return final.IsParams && ((ParameterSymbol)final.OriginalDefinition).Type.IsSZArray();
return final.IsParams && final.OriginalDefinition.Type.IsPossibleParamsType();
}

private static bool IsOverride(Symbol overridden, Symbol overrider)
Expand Down Expand Up @@ -2874,7 +2874,8 @@ private EffectiveParameters GetEffectiveParametersInExpandedForm<TMember>(
var refs = ArrayBuilder<RefKind>.GetInstance();
bool anyRef = false;
var parameters = member.GetParameters();
var elementType = ((ArrayTypeSymbol)parameters[parameters.Length - 1].Type).ElementType;
var paramsType = parameters[parameters.Length - 1].Type;
var elementType = paramsType.GetElementTypeOfParamsType();
bool hasAnyRefArg = argumentRefKinds.Any();
hasAnyRefOmittedArgument = false;

Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ internal enum MessageID
IDS_FeatureIndexingMovableFixedBuffers = MessageBase + 12744,
IDS_FeatureRecursivePatterns = MessageBase + 12745,
IDS_FeatureNestedStackalloc = MessageBase + 12746,

IDS_FeatureParamsArrayInterfaceAndSpan = MessageBase + 12747,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -221,6 +223,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
// C# 8.0 features.
case MessageID.IDS_FeatureRecursivePatterns:
case MessageID.IDS_FeatureNestedStackalloc:
case MessageID.IDS_FeatureParamsArrayInterfaceAndSpan: // semantic check
return LanguageVersion.CSharp8;

// C# 7.3 features.
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/LanguageVersion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -360,5 +360,10 @@ internal static bool AllowImprovedOverloadCandidates(this LanguageVersion self)
{
return self >= MessageID.IDS_FeatureImprovedOverloadCandidates.RequiredVersion();
}

internal static bool AllowParamsArrayInterfaceAndSpan(this LanguageVersion self)
{
return self >= MessageID.IDS_FeatureParamsArrayInterfaceAndSpan.RequiredVersion();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,12 @@ private BoundExpression BuildParamsArray(
}
}

var paramArrayType = parameters[paramsParam].Type;
var paramsType = parameters[paramsParam].Type;
if (paramsType.IsPossibleArrayGenericInterface())
{
paramsType = ArrayTypeSymbol.CreateSZArray(_compilation.Assembly, paramsType.GetElementTypeOfParamsType());
}

var arrayArgs = paramArray.ToImmutableAndFree();

// If this is a zero-length array, rather than using "new T[0]", optimize with "Array.Empty<T>()"
Expand All @@ -896,14 +901,13 @@ private BoundExpression BuildParamsArray(
// of expression lambdas will appropriately understand Array.Empty<T>().
if (arrayArgs.Length == 0 && !_inExpressionLambda)
{
ArrayTypeSymbol ats = paramArrayType as ArrayTypeSymbol;
if (ats != null) // could be null if there's a semantic error, e.g. the params parameter type isn't an array
if (paramsType is ArrayTypeSymbol arrayType) // could be false if there's a semantic error, or if the params parameter type isn't an array
{
MethodSymbol arrayEmpty = _compilation.GetWellKnownTypeMember(WellKnownMember.System_Array__Empty) as MethodSymbol;
if (arrayEmpty != null) // will be null if Array.Empty<T> doesn't exist in reference assemblies
{
// return an invocation of "Array.Empty<T>()"
arrayEmpty = arrayEmpty.Construct(ImmutableArray.Create(ats.ElementType));
arrayEmpty = arrayEmpty.Construct(ImmutableArray.Create(arrayType.ElementType));
return new BoundCall(
syntax,
null,
Expand All @@ -922,25 +926,77 @@ private BoundExpression BuildParamsArray(
}
}

return CreateParamArrayArgument(syntax, paramArrayType, arrayArgs, this, null);
return CreateParamArrayArgument(syntax, paramsType, arrayArgs, this, null);
}

private static BoundExpression CreateParamArrayArgument(SyntaxNode syntax,
TypeSymbol paramArrayType,
private static BoundExpression CreateParamArrayArgument(
SyntaxNode syntax,
TypeSymbol paramsType,
ImmutableArray<BoundExpression> arrayArgs,
LocalRewriter localRewriter,
Binder binder)
{
Debug.Assert(localRewriter == null ^ binder == null);

TypeSymbol int32Type = (localRewriter != null ? localRewriter._compilation : binder.Compilation).GetSpecialType(SpecialType.System_Int32);
CSharpCompilation compilation = localRewriter != null ? localRewriter._compilation : binder.Compilation;
TypeSymbol int32Type = compilation.GetSpecialType(SpecialType.System_Int32);
BoundExpression arraySize = MakeLiteral(syntax, ConstantValue.Create(arrayArgs.Length), int32Type, localRewriter);

return new BoundArrayCreation(
syntax,
ImmutableArray.Create(arraySize),
new BoundArrayInitialization(syntax, arrayArgs) { WasCompilerGenerated = true },
paramArrayType) { WasCompilerGenerated = true };
var initializer = new BoundArrayInitialization(syntax, arrayArgs) { WasCompilerGenerated = true };

if (paramsType.IsSZArray())
{
return new BoundArrayCreation(syntax, ImmutableArray.Create(arraySize), initializer, paramsType) { WasCompilerGenerated = true };
}

TypeSymbol elementType = paramsType.GetElementTypeOfParamsType();
bool isReferenceType = elementType.IsReferenceType;

BoundExpression[] args;
if (isReferenceType)
{
args = new BoundExpression[]
{
new BoundArrayCreation(
syntax,
ImmutableArray.Create(arraySize),
initializer,
ArrayTypeSymbol.CreateSZArray(compilation.Assembly, elementType))
{ WasCompilerGenerated = true }
};
}
else
{
args = new BoundExpression[]
{
new BoundConvertedStackAllocExpression(
syntax,
elementType,
arraySize,
initializer,
paramsType)
{ WasCompilerGenerated = true },
arraySize,
};
}

WellKnownMember constructor = compilation.IsReadOnlySpanType(paramsType)
? isReferenceType ? WellKnownMember.System_ReadOnlySpan_T__ctor2 : WellKnownMember.System_ReadOnlySpan_T__ctor
: isReferenceType ? WellKnownMember.System_Span_T__ctor2 : WellKnownMember.System_Span_T__ctor;

// TODO use TryGetWellKnownTypeMember instead, should test with a missing ctor
var spanConstructor = compilation.GetWellKnownTypeMember(constructor) as MethodSymbol;
if (spanConstructor == null)
{
return new BoundBadExpression(
syntax: syntax,
resultKind: LookupResultKind.NotInvocable,
symbols: ImmutableArray<Symbol>.Empty,
childBoundNodes: ImmutableArray<BoundExpression>.Empty,
type: ErrorTypeSymbol.UnknownResultType);
}

return new BoundObjectCreationExpression(syntax, spanConstructor.AsMember((NamedTypeSymbol)paramsType), binder, arguments: args);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ internal class MemberSignatureComparer : IEqualityComparer<Symbol>
considerTypeConstraints: false,
considerCallingConvention: false,
considerRefKindDifferences: false,
considerCustomModifiers: false); //shouldn't actually matter for source members
considerCustomModifiers: false, //shouldn't actually matter for source members
considerParamsElementTypes: true);

/// <summary>
/// This instance is used to determine if a partial method implementation matches the definition.
Expand Down Expand Up @@ -302,6 +303,8 @@ internal class MemberSignatureComparer : IEqualityComparer<Symbol>
// Ignore the names of the tuple elements
private readonly bool _ignoreTupleNames;

private readonly bool _considerParamsElementTypes;

private MemberSignatureComparer(
bool considerName,
bool considerExplicitlyImplementedInterfaces,
Expand All @@ -310,6 +313,7 @@ private MemberSignatureComparer(
bool considerCallingConvention,
bool considerRefKindDifferences,
bool considerCustomModifiers,
bool considerParamsElementTypes = false,
bool ignoreDynamic = true,
bool ignoreTupleNames = true)
{
Expand All @@ -324,6 +328,7 @@ private MemberSignatureComparer(
_considerCustomModifiers = considerCustomModifiers;
_ignoreDynamic = ignoreDynamic;
_ignoreTupleNames = ignoreTupleNames;
_considerParamsElementTypes = considerParamsElementTypes;
}

#region IEqualityComparer<Symbol> Members
Expand Down Expand Up @@ -374,10 +379,21 @@ public bool Equals(Symbol member1, Symbol member2)
return false;
}

if (member1.GetParameterCount() > 0 && !HaveSameParameterTypes(member1.GetParameters(), typeMap1, member2.GetParameters(), typeMap2,
_considerRefKindDifferences, _considerCustomModifiers, _ignoreDynamic, _ignoreTupleNames))
var parameters1 = member1.GetParameters();
var parameters2 = member2.GetParameters();

if (member1.GetParameterCount() > 0)
{
return false;
if (!HaveSameParameterTypes(parameters1, typeMap1, parameters2, typeMap2, _considerRefKindDifferences, _considerCustomModifiers, _ignoreDynamic, _ignoreTupleNames))
{
return false;
}

if (_considerParamsElementTypes && member1.HasParamsParameter() && member2.HasParamsParameter() &&
!HaveSameParamsElementTypes(parameters1, parameters2))
{
return false;
}
}

if (_considerCallingConvention)
Expand Down Expand Up @@ -693,6 +709,14 @@ private static bool HaveSameParameterTypes(ImmutableArray<ParameterSymbol> param
return true;
}

private static bool HaveSameParamsElementTypes(ImmutableArray<ParameterSymbol> parameters1, ImmutableArray<ParameterSymbol> parameters2)
{
Debug.Assert(parameters1.Length == parameters2.Length);
var elementType1 = parameters1.Last().Type.GetElementTypeOfParamsType();
var elementType2 = parameters2.Last().Type.GetElementTypeOfParamsType();
return elementType1.Equals(elementType2, TypeCompareKind.IgnoreTupleNames);
}

private static TypeWithModifiers SubstituteType(TypeMap typeMap, TypeWithModifiers typeSymbol)
{
return typeMap == null ? typeSymbol : typeSymbol.SubstituteType(typeMap);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static ImmutableArray<ParameterSymbol> MakeParameters(
addRefReadOnlyModifier,
diagnostics);

ReportParameterErrors(owner, parameterSyntax, parameter, thisKeyword, paramsKeyword, firstDefault, diagnostics);
ReportParameterErrors(owner, parameterSyntax, parameter, thisKeyword, paramsKeyword, firstDefault, binder.Compilation, diagnostics);

builder.Add(parameter);
++parameterIndex;
Expand Down Expand Up @@ -269,13 +269,13 @@ private static void CheckParameterModifiers(
}
}

private static void ReportParameterErrors(
Symbol owner,
private static void ReportParameterErrors(Symbol owner,
ParameterSyntax parameterSyntax,
SourceParameterSymbol parameter,
SyntaxToken thisKeyword,
SyntaxToken paramsKeyword,
int firstDefault,
CSharpCompilation compilation,
DiagnosticBag diagnostics)
{
TypeSymbol parameterType = parameter.Type;
Expand All @@ -295,12 +295,24 @@ private static void ReportParameterErrors(
// error CS1670: params is not valid in this context
diagnostics.Add(ErrorCode.ERR_IllegalParams, paramsKeyword.GetLocation());
}
else if (parameter.IsParams && !parameterType.IsSZArray())
else if (parameter.IsParams)
{
// error CS0225: The params parameter must be a single dimensional array
diagnostics.Add(ErrorCode.ERR_ParamsMustBeArray, paramsKeyword.GetLocation());
if (!parameterType.IsPossibleParamsType())
{
// error CS0225: The params parameter must be assignable from array.
diagnostics.Add(ErrorCode.ERR_ParamsMustBeArray, paramsKeyword.GetLocation());
return;
}

if (!parameterType.IsSZArray() && !compilation.LanguageVersion.AllowParamsArrayInterfaceAndSpan())
{
diagnostics.Add(ErrorCode.ERR_ParamsMustBeArray, paramsKeyword.GetLocation(),
new CSharpRequiredLanguageVersion(MessageID.IDS_FeatureParamsArrayInterfaceAndSpan.RequiredVersion()));
return;
}
}
else if (parameter.Type.IsStatic && !parameter.ContainingSymbol.ContainingType.IsInterfaceType())

if (parameter.Type.IsStatic && !parameter.ContainingSymbol.ContainingType.IsInterfaceType())
{
// error CS0721: '{0}': static types cannot be used as parameters
diagnostics.Add(ErrorCode.ERR_ParameterIsStaticClass, owner.Locations[0], parameter.Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1551,9 +1551,7 @@ private void CheckMemberNameConflicts(DiagnosticBag diagnostics)
// That takes care of the first category of conflict; we detect the
// second and third categories as follows:

var conversion = symbol as SourceUserDefinedConversionSymbol;
var method = symbol as SourceMemberMethodSymbol;
if ((object)conversion != null)
if (symbol is SourceUserDefinedConversionSymbol conversion)
{
// Does this conversion collide *as a conversion* with any previously-seen
// conversion?
Expand Down Expand Up @@ -1584,7 +1582,7 @@ private void CheckMemberNameConflicts(DiagnosticBag diagnostics)
// Do not add the conversion to the set of previously-seen methods; that set
// is only non-conversion methods.
}
else if ((object)method != null)
else if (symbol is SourceMemberMethodSymbol method)
{
// Does this method collide *as a method* with any previously-seen
// conversion?
Expand Down
Loading