Make trivial Params Collections scenarios working end to end #71136
Conversation
dotnet-issue-labeler
bot
added
the
untriaged
|
@RikkiGibson, @333fred, @dotnet/roslyn-compiler Please review |
… ParamsCollections_03
| { | ||
| var type = parameter.Type; | ||
| if (result.Kind == MemberResolutionKind.ApplicableInExpandedForm && | ||
| parameter.IsParams && type.IsSZArray()) | ||
| parameter.Ordinal == parameterCount - 1) |
There was a problem hiding this comment.
note: This change relates to issue #70908. The parameter can differ on params modifier on virtual and override methods, for example, so checking ApplicableAndExpandedForm and Ordinal is complete and more robust than checking IsParams on a single symbol. #Resolved
| @@ -368,7 +368,7 @@ private BoundIndexerAccess BindIndexerDefaultArgumentsAndParamsArray(BoundIndexe | |||
| } | |||
| } | |||
|
|
|||
| BindDefaultArgumentsAndParamsArray(indexerAccess.Syntax, parameters, argumentsBuilder, refKindsBuilderOpt, namesBuilder, ref argsToParams, out defaultArguments, indexerAccess.Expanded, enableCallerInfo: true, diagnostics); | |||
There was a problem hiding this comment.
Consider also adjusting the name of BindIndexerDefaultArgumentsAndParamsArray. #Resolved
| @@ -6602,15 +6602,24 @@ static void expandParamsArray(ref ImmutableArray<BoundExpression> arguments, ref | |||
| // At the moment, there is only one test that gets here like that - Microsoft.CodeAnalysis.CSharp.UnitTests.AttributeTests.TestBadParamsCtor. | |||
| // And we get here for the erroneous attribute application, constructor is inaccessible. | |||
| // Perhaps that shouldn't cancel the default values / params array processing? | |||
| Debug.Assert(arguments.Count(a => a.IsParamsArray) <= 1); | |||
| Debug.Assert(arguments.Count(a => a.IsParamsCollection) <= 1); | |||
There was a problem hiding this comment.
Consider renaming function expandParamsArray. #Resolved
| } | ||
| else | ||
| { | ||
| elements = ((BoundCollectionExpression)((BoundConversion)argument).Operand).UnconvertedCollectionExpression.Elements.CastArray<BoundExpression>(); |
There was a problem hiding this comment.
Do we have tests showing that NullableWalker is correctly visiting the conversions of params collection elements? This could be added to test plan and/or in a future PR. #Resolved
| @@ -1006,14 +1006,12 @@ private void ReferToTempIfReferenceTypeReceiver(BoundLocal receiverTemp, ref Bou | |||
| BoundExpression? optimized; | |||
|
|
|||
| Debug.Assert(expanded ? rewrittenArguments.Length == parameters.Length : rewrittenArguments.Length >= parameters.Length); | |||
| Debug.Assert(rewrittenArguments.Count(a => a.IsParamsArray) == (expanded ? 1 : 0)); | |||
| Debug.Assert(rewrittenArguments.Count(a => a.IsParamsCollection) <= (expanded ? 1 : 0)); | |||
There was a problem hiding this comment.
note: it looks like in the params-collections (non-array) case, the IsParamsCollection flag is lost after the collection is lowered. Hence relaxing the assert here. #Resolved
| @@ -495,6 +495,18 @@ internal bool ConversionHasSideEffects() | |||
|
|
|||
| return true; | |||
| } | |||
|
|
|||
| public new bool IsParamsCollection | |||
There was a problem hiding this comment.
note: containing type is BoundConversion here #Resolved
|
@333fred, @dotnet/roslyn-compiler For the second review. |
|
@333fred, @dotnet/roslyn-compiler For the second review. |
src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
Show resolved
Hide resolved
AlekseyTs
merged commit 569dcb7
into
dotnet:features/ParamsCollections
No description provided.