Skip to content

Commit

Permalink
Extend eligible params types to array generic interfaces and span
Browse files Browse the repository at this point in the history
  • Loading branch information
alrz committed Aug 18, 2018
1 parent 4029225 commit 518a0e5
Show file tree
Hide file tree
Showing 12 changed files with 314 additions and 54 deletions.
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
32 changes: 28 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/MemberSignatureComparer.cs
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
26 changes: 19 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs
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

0 comments on commit 518a0e5

Please sign in to comment.