Add support for with(...) elements in collection expressions (3). #80568
Conversation
Co-authored-by: Julien Couvreur <jcouv@microsoft.com>
|
|
||
| var argsToParams = projectionCall.ArgsToParamsOpt; | ||
| if (!argsToParams.IsDefault) | ||
| argsToParams = argsToParams.Add(collectionBuilderMethod.ParameterCount - 1); |
There was a problem hiding this comment.
A collection builder method is not going to be a new extension method, right? That would make some of this projection stuff extra painful.
There was a problem hiding this comment.
yeah. that's not supported now. it's supposed to point at a normal static method.
| type: collectionBuilderMethod.ReturnType).MakeCompilerGenerated(); | ||
|
|
||
| // Wrap in a conversion if necessary. Note that GetCollectionBuilderMethods guarantees that either | ||
| // return and target type are identical, or that a valid implicit conversion exists between them. |
There was a problem hiding this comment.
Consider asserting this (potentially in follow up PR)
| Debug.Assert(!collectionBuilderMethod.GetIsNewExtensionMember()); | ||
|
|
||
| return collectionBuilderMethod; | ||
| Debug.Assert(!collectionBuilderMethod.GetIsNewExtensionMember()); |
| else if (targetType is TypeParameterSymbol typeParameter) | ||
| { | ||
| collectionCreation = BindTypeParameterCreationExpression(syntax, typeParameter, analyzedArguments, initializerOpt: null, typeSyntax: syntax, wasTargetTyped: true, diagnostics); | ||
| collectionCreation = BindTypeParameterCreationExpression(syntax, typeParameter, analyzedArguments, initializerOpt: null, typeSyntax: syntax, wasTargetTyped: false, diagnostics); |
There was a problem hiding this comment.
It looks like wasTargetTyped: true was just the wrong thing to be using here? I am assuming this is for when the target collection type is a type parameter constrained to something which allows us to know what its construction methods are?
…-arguments' into withElementCreate
|
|
||
| var withArgumentsBuilder = ArrayBuilder<BoundExpression>.GetInstance(withArguments.Length); | ||
| foreach (var argument in withArguments) | ||
| withArgumentsBuilder.Add(BindToNaturalType(argument, diagnostics, reportNoTargetType: !targetType.IsErrorType())); |
There was a problem hiding this comment.
nit: perhaps !targetType.IsErrorType() should be extracted. can happen in later PR or never. up to you.
Co-authored-by: Rikki Gibson <rikkigibson@gmail.com>
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Constructor.cs
Show resolved
Hide resolved
…/roslyn into withElementCreate
| static void Main() | ||
| { | ||
| string? s = null; | ||
| Identity<int>([with((s = "").Length)]); |
There was a problem hiding this comment.
It would be good to test a scenario like the following (can be done in follow up)
public static MyCollection<T> Create<T>(T value, ReadOnlySpan<T> items) => new(items);
MyCollection<T> Identity<T>(MyCollection<T> collection) => collection;
string? x = null;
var collection = M1([with(x), "a"]);
collection[0].ToString(); // should this warn or not?| Diagnostic(ErrorCode.ERR_ValConstraintNotSatisfied, "params MyCollection<T> c").WithArguments("MyBuilder.Create<T>(System.ReadOnlySpan<T>)", "T", "T").WithLocation(12, 51), | ||
| // (14,53): error CS0453: The type 'T' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'MyBuilder.Create<T>(ReadOnlySpan<T>)' | ||
| // static MyCollection<T> ClassConstraintParams<T>(params MyCollection<T> c) where T : class => c; | ||
| Diagnostic(ErrorCode.ERR_ValConstraintNotSatisfied, "params MyCollection<T> c").WithArguments("MyBuilder.Create<T>(System.ReadOnlySpan<T>)", "T", "T").WithLocation(14, 53)); |
There was a problem hiding this comment.
It would be useful to also know what error is reported for empty with() here.
|
I'm having issues with the review tool (both vscode and github side review tools). I have the following comments which I will indicate here with the test name associated with the comment. These are non-blocking comments. Sorry for the inconvenience. Test |
621cc77
into
dotnet:features/collection-expression-arguments
Followup to #80554.
Adds support for the collection-builder pattern.
Relates to test plan #80613