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

Params Collections: misc chanages #71423

Merged
Merged
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
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,11 @@ ImmutableArray<int> makeSourceIndices()
constructorArgumentSourceIndices.Count = lengthAfterRewriting;
for (int argIndex = 0; argIndex < lengthAfterRewriting; argIndex++)
{
Debug.Assert(!arguments[argIndex].IsParamsCollection || arguments[argIndex] is BoundArrayCreation);

int paramIndex = argsToParamsOpt.IsDefault ? argIndex : argsToParamsOpt[argIndex];
constructorArgumentSourceIndices[paramIndex] =
defaultArguments[argIndex] ||
// PROTOTYPE(ParamsCollections): Adjust?
(arguments[argIndex].IsParamsCollection && arguments[argIndex] is BoundArrayCreation { Bounds: [BoundLiteral { ConstantValueOpt.Value: 0 }] }) ?
-1 : argIndex;
}
Expand Down
7 changes: 3 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ private BoundCollectionExpression ConvertCollectionExpression(
return BindCollectionExpressionForErrorRecovery(node, targetType, diagnostics);
}

var syntax = (ExpressionSyntax)node.Syntax;
var syntax = node.Syntax;
if (LocalRewriter.IsAllocatingRefStructCollectionExpression(node, collectionTypeKind, elementType, Compilation))
{
diagnostics.Add(node.HasSpreadElements(out _, out _)
Expand Down Expand Up @@ -825,7 +825,7 @@ private void GenerateImplicitConversionErrorForCollectionExpression(
if (collectionTypeKind == CollectionExpressionTypeKind.CollectionBuilder)
{
Debug.Assert(elementTypeWithAnnotations.Type is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases.
if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out elementTypeWithAnnotations))
if (!TryGetCollectionIterationType(node.Syntax, targetType, out elementTypeWithAnnotations))
{
Error(diagnostics, ErrorCode.ERR_CollectionBuilderNoElementType, node.Syntax, targetType);
return;
Expand Down Expand Up @@ -1450,8 +1450,7 @@ lambdaParameter is not SourceComplexParameterSymbolBase
}
}

// PROTOTYPE(ParamsCollections): Adjust
if (lambdaParameter.IsParams && !delegateParameter.IsParams && p == lambdaSymbol.ParameterCount - 1 && lambdaParameter.Type.IsSZArray())
if (lambdaParameter.IsParams && !delegateParameter.IsParams && p == lambdaSymbol.ParameterCount - 1)
{
// Parameter {0} has params modifier in lambda but not in target delegate type.
Error(diagnostics, ErrorCode.WRN_ParamsArrayInLambdaOnly, lambdaParameter.GetFirstLocation(), p + 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3408,7 +3408,7 @@ private void CheckAndCoerceArguments<TMember>(
Debug.Assert(argument is BoundUnconvertedInterpolatedString or BoundBinaryOperator { IsUnconvertedInterpolatedStringAddition: true });
TypeWithAnnotations parameterTypeWithAnnotations = GetCorrespondingParameterTypeWithAnnotations(ref result, parameters, arg);
reportUnsafeIfNeeded(methodResult, diagnostics, argument, parameterTypeWithAnnotations);
arguments[arg] = BindInterpolatedStringHandlerInMemberCall(argument, arguments, parameters, ref result, arg, receiver, diagnostics);
arguments[arg] = BindInterpolatedStringHandlerInMemberCall(argument, parameterTypeWithAnnotations.Type, arguments, parameters, ref result, arg, receiver, diagnostics);
}
// https://github.com/dotnet/roslyn/issues/37119 : should we create an (Identity) conversion when the kind is Identity but the types differ?
else if (!kind.IsIdentity)
Expand Down
52 changes: 29 additions & 23 deletions src/Compilers/CSharp/Portable/Binder/Binder_InterpolatedString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ private ImmutableArray<BoundExpression> BindInterpolatedStringParts(BoundUnconve

private BoundExpression BindInterpolatedStringHandlerInMemberCall(
BoundExpression unconvertedString,
TypeSymbol handlerType,
ArrayBuilder<BoundExpression> arguments,
ImmutableArray<ParameterSymbol> parameters,
ref MemberAnalysisResult memberAnalysisResult,
Expand All @@ -886,36 +887,42 @@ private BoundExpression BindInterpolatedStringHandlerInMemberCall(
Debug.Assert(unconvertedString is BoundUnconvertedInterpolatedString or BoundBinaryOperator { IsUnconvertedInterpolatedStringAddition: true });
var interpolatedStringConversion = memberAnalysisResult.ConversionForArg(interpolatedStringArgNum);
Debug.Assert(interpolatedStringConversion.IsInterpolatedStringHandler);
var interpolatedStringParameter = GetCorrespondingParameter(ref memberAnalysisResult, parameters, interpolatedStringArgNum);
Debug.Assert(interpolatedStringParameter is { Type: NamedTypeSymbol { IsInterpolatedStringHandlerType: true } }
#pragma warning disable format
or
{
IsParams: true,
// PROTOTYPE(ParamsCollections): Adjust
Type: ArrayTypeSymbol { ElementType: NamedTypeSymbol { IsInterpolatedStringHandlerType: true } },
InterpolatedStringHandlerArgumentIndexes.IsEmpty: true
});
#pragma warning restore format
Debug.Assert(!interpolatedStringParameter.IsParams || memberAnalysisResult.Kind == MemberResolutionKind.ApplicableInExpandedForm);

if (interpolatedStringParameter.HasInterpolatedStringHandlerArgumentError)
Debug.Assert(handlerType is NamedTypeSymbol { IsInterpolatedStringHandlerType: true });

var correspondingParameter = GetCorrespondingParameter(ref memberAnalysisResult, parameters, interpolatedStringArgNum);
var handlerParameterIndexes = correspondingParameter.InterpolatedStringHandlerArgumentIndexes;

if (memberAnalysisResult.Kind == MemberResolutionKind.ApplicableInExpandedForm && correspondingParameter.Ordinal == parameters.Length - 1)
333fred marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(handlerParameterIndexes.IsEmpty);

// No arguments, fall back to the standard conversion steps.
return CreateConversion(
unconvertedString.Syntax,
unconvertedString,
interpolatedStringConversion,
isCast: false,
conversionGroupOpt: null,
handlerType,
diagnostics);
}

if (correspondingParameter.HasInterpolatedStringHandlerArgumentError)
{
// The InterpolatedStringHandlerArgumentAttribute applied to parameter '{0}' is malformed and cannot be interpreted. Construct an instance of '{1}' manually.
diagnostics.Add(ErrorCode.ERR_InterpolatedStringHandlerArgumentAttributeMalformed, unconvertedString.Syntax.Location, interpolatedStringParameter, interpolatedStringParameter.Type);
diagnostics.Add(ErrorCode.ERR_InterpolatedStringHandlerArgumentAttributeMalformed, unconvertedString.Syntax.Location, correspondingParameter, handlerType);
return CreateConversion(
unconvertedString.Syntax,
unconvertedString,
interpolatedStringConversion,
isCast: false,
conversionGroupOpt: null,
wasCompilerGenerated: false,
interpolatedStringParameter.Type,
handlerType,
diagnostics,
hasErrors: true);
}

var handlerParameterIndexes = interpolatedStringParameter.InterpolatedStringHandlerArgumentIndexes;
if (handlerParameterIndexes.IsEmpty)
{
// No arguments, fall back to the standard conversion steps.
Expand All @@ -925,8 +932,7 @@ private BoundExpression BindInterpolatedStringHandlerInMemberCall(
interpolatedStringConversion,
isCast: false,
conversionGroupOpt: null,
// PROTOTYPE(ParamsCollections): Adjust
interpolatedStringParameter.IsParams ? ((ArrayTypeSymbol)interpolatedStringParameter.Type).ElementType : interpolatedStringParameter.Type,
handlerType,
diagnostics);
}

Expand Down Expand Up @@ -1007,7 +1013,7 @@ private BoundExpression BindInterpolatedStringHandlerInMemberCall(
ErrorCode.ERR_InterpolatedStringHandlerArgumentOptionalNotSpecified,
unconvertedString.Syntax.Location,
parameter.Name,
interpolatedStringParameter.Name);
correspondingParameter.Name);
hasErrors = true;
}

Expand All @@ -1026,7 +1032,7 @@ private BoundExpression BindInterpolatedStringHandlerInMemberCall(
ErrorCode.ERR_InterpolatedStringHandlerArgumentLocatedAfterInterpolatedString,
arguments[argumentIndex].Syntax.Location,
parameter.Name,
interpolatedStringParameter.Name);
correspondingParameter.Name);
hasErrors = true;
}

Expand Down Expand Up @@ -1073,7 +1079,7 @@ private BoundExpression BindInterpolatedStringHandlerInMemberCall(

var interpolatedString = BindUnconvertedInterpolatedExpressionToHandlerType(
unconvertedString,
(NamedTypeSymbol)interpolatedStringParameter.Type,
(NamedTypeSymbol)handlerType,
diagnostics,
additionalConstructorArguments: argumentPlaceholdersBuilder.ToImmutableAndFree(),
additionalConstructorRefKinds: argumentRefKindsBuilder.ToImmutableAndFree());
Expand All @@ -1086,7 +1092,7 @@ private BoundExpression BindInterpolatedStringHandlerInMemberCall(
explicitCastInCode: false,
conversionGroupOpt: null,
constantValueOpt: null,
interpolatedStringParameter.Type,
handlerType,
hasErrors || interpolatedString.HasErrors);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ protected override Conversion GetCollectionExpressionConversion(

case CollectionExpressionTypeKind.CollectionBuilder:
{
_binder.TryGetCollectionIterationType((Syntax.ExpressionSyntax)syntax, targetType, out elementTypeWithAnnotations);
_binder.TryGetCollectionIterationType(syntax, targetType, out elementTypeWithAnnotations);
elementType = elementTypeWithAnnotations.Type;
if (elementType is null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1312,7 +1312,7 @@ private void BuildStoresToTemps(

Debug.Assert(arguments[p] == null);

if (argument.IsParamsCollection) // PROTOTYPE(ParamsCollections): Do we need to do the same special case for collections other than arrays?
if (argument.IsParamsCollection)
333fred marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(expanded);
Debug.Assert(p == parameters.Length - 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,9 @@ private BoundIndexerAccess TransformIndexerAccessContinued(
refKinds,
storesToTemps);

if (expanded)
if (expanded && actualArguments[actualArguments.Length - 1] is { IsParamsCollection: true } array)
{
BoundExpression array = actualArguments[actualArguments.Length - 1];
Debug.Assert(array.IsParamsCollection); // PROTOTYPE(ParamsCollections): This Assert is likely to fail for a non-array case. The code path needs an adjustment in general.
Debug.Assert(array is BoundArrayCreation);
333fred marked this conversation as resolved.
Show resolved Hide resolved

if (TryOptimizeParamsArray(array, out BoundExpression? optimized))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ private void AddObjectInitializer(

if (!memberInit.Arguments.IsDefaultOrEmpty)
{
Debug.Assert(memberInit.Arguments.Count(a => a.IsParamsCollection) == (memberInit.Expanded ? 1 : 0)); // PROTOTYPE(ParamsCollections): Adjust?
Debug.Assert(memberInit.Arguments.Count(a => a.IsParamsCollection) <= (memberInit.Expanded ? 1 : 0));

var args = EvaluateSideEffectingArgumentsToTemps(
memberInit.Arguments,
Expand Down Expand Up @@ -530,7 +530,6 @@ void addIndexes(ArrayBuilder<BoundExpression> result, BoundAssignmentOperator as
{
foreach (var argument in initializerMember.Arguments)
{
// PROTOTYPE(ParamsCollections): Do we need to do the same special case for collections other than arrays?
if (argument is BoundArrayCreation { IsParamsCollection: true, InitializerOpt: var initializers })
{
Debug.Assert(initializers is not null);
Expand Down Expand Up @@ -589,7 +588,7 @@ private ImmutableArray<BoundExpression> EvaluateSideEffectingArgumentsToTemps(

BoundExpression replacement;

if (arg.IsParamsCollection) // PROTOTYPE(ParamsCollections): Do we need to recreate the same special handling for collections other than arrays?
if (arg.IsParamsCollection)
{
// Capturing the array instead is going to lead to an observable behavior difference. Not just an IL difference,
// see Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen.ObjectAndCollectionInitializerTests.DictionaryInitializerTestSideeffects001param for example.
Expand Down
Loading