-
Notifications
You must be signed in to change notification settings - Fork 4.3k
STALE: Add IOp support for with(...) elements.
#81058
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
Changes from all commits
ab6f95f
47ffa73
e26f05b
f781408
f489a02
4fb9888
c5f3662
13baeda
e431b33
d6078c5
d3946ca
51b41e2
d4da134
5d69b09
2860c92
9ad9bff
912a038
a63fa85
a5f83e7
3374363
0bf7671
801968e
b7f723f
d4e451b
3f34140
aa06ca1
ab9c232
d3146db
dabb261
a5db155
010c783
3002bcb
0161bfb
1a41d02
c433eb3
26f6f95
ad0a637
ca616ed
d9722d8
288f14a
b0eb162
643233a
783edae
8eb9343
9b10c18
e1492ce
c1f51e4
b4ca300
a3b2e5f
9f7cce5
1db3441
dacba39
4772ed6
b21f43c
a0b96a4
83c1141
7bdad0d
9727a59
7ce3cef
7a2b9fc
511ca83
1505fdd
5aa324d
1ce3ae2
24b8c48
759a887
4190ad6
006b6db
b293d53
0514752
f33bfe6
9d182f8
5daf3a8
9769388
e07de89
9f12cd5
7d8fec2
8847d30
8393c63
033fa45
393762f
dfe52e6
db3fba0
70a5174
bddf433
c59c928
7a67ee4
ad1e9c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -835,7 +835,7 @@ private readonly struct CollectionExpressionConverter( | |
| private readonly BindingDiagnosticBag _diagnostics = diagnostics; | ||
|
|
||
| private BoundCollectionExpression CreateCollectionExpression( | ||
| CollectionExpressionTypeKind collectionTypeKind, ImmutableArray<BoundNode> elements, BoundObjectOrCollectionValuePlaceholder? placeholder = null, BoundExpression? collectionCreation = null, MethodSymbol? collectionBuilderMethod = null, BoundValuePlaceholder? collectionBuilderElementsPlaceholder = null) | ||
| CollectionExpressionTypeKind collectionTypeKind, ImmutableArray<BoundNode> elements, BoundObjectOrCollectionValuePlaceholder? placeholder = null, BoundExpression? collectionCreation = null, MethodSymbol? collectionBuilderMethod = null, BoundCollectionBuilderElementsPlaceholder? collectionBuilderElementsPlaceholder = null) | ||
| { | ||
| return new BoundCollectionExpression( | ||
| _node.Syntax, | ||
|
|
@@ -1147,13 +1147,23 @@ static BoundCollectionExpressionSpreadElement bindSpreadElement( | |
| ErrorCode.ERR_CollectionArgumentsNotSupportedForType, | ||
| _node.WithElement.Syntax.GetFirstToken().GetLocation(), | ||
| _targetType); | ||
| return null; | ||
| } | ||
|
|
||
| return CreateCollectionExpression(collectionTypeKind, elements); | ||
| } | ||
|
|
||
| private readonly BoundCollectionExpression? TryConvertCollectionExpressionArrayInterfaceType(ImmutableArray<BoundNode> elements) | ||
| { | ||
| if (_node.WithElement?.Arguments.Length > 0 && | ||
| _targetType.IsReadOnlyArrayInterface(out _)) | ||
| { | ||
| // For the read-only array interfaces (IEnumerable<E>, IReadOnlyCollection<E>, IReadOnlyList<E>), only | ||
| // the parameterless `with()` is allowed. | ||
| _diagnostics.Add(ErrorCode.ERR_CollectionArgumentsMustBeEmpty, _node.WithElement.Syntax.GetFirstToken().GetLocation()); | ||
| return null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please provide the name of the test?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TestReadOnlyInterface_Empty, TestReadOnlyInterface_SingleArg, TestReadOnlyInterface_NamedArg |
||
| } | ||
|
|
||
| return CreateCollectionExpression( | ||
| CollectionExpressionTypeKind.ArrayInterface, | ||
| elements, | ||
|
|
@@ -1170,13 +1180,6 @@ static BoundCollectionExpressionSpreadElement bindSpreadElement( | |
| var withSyntax = withElement.Syntax; | ||
| if (@this._targetType.IsReadOnlyArrayInterface(out _)) | ||
| { | ||
| // For the read-only array interfaces (IEnumerable<E>, IReadOnlyCollection<E>, IReadOnlyList<E>), only | ||
| // the parameterless `with()` is allowed. | ||
| if (withElement.Arguments.Length > 0) | ||
| { | ||
| @this._diagnostics.Add(ErrorCode.ERR_CollectionArgumentsMustBeEmpty, withSyntax.GetFirstToken().GetLocation()); | ||
| } | ||
|
|
||
| // Note: we intentionally report null here. Even though the code has `with()` in it, we're not actually | ||
| // going to call a particular constructor. The lowering phase will properly handle creating a read-only | ||
| // interface instance. | ||
|
|
@@ -1251,7 +1254,7 @@ static BoundCollectionExpressionSpreadElement bindSpreadElement( | |
| collectionBuilderMethod: collectionBuilderMethod, | ||
| collectionBuilderElementsPlaceholder: collectionBuilderElementsPlaceholder); | ||
|
|
||
| static (BoundExpression? collectionCreation, MethodSymbol? collectionBuilderMethod, BoundValuePlaceholder? elementsPlaceholder) bindCollectionBuilderInfo( | ||
| static (BoundExpression? collectionCreation, MethodSymbol? collectionBuilderMethod, BoundCollectionBuilderElementsPlaceholder? elementsPlaceholder) bindCollectionBuilderInfo( | ||
| ref readonly CollectionExpressionConverter @this) | ||
| { | ||
| var namedType = (NamedTypeSymbol)@this._targetType; | ||
|
|
@@ -1334,7 +1337,7 @@ static BoundCollectionExpressionSpreadElement bindSpreadElement( | |
| // CollectionBuilder.Create<T1, T2, ..>(a, b, c, <placeholder for [x, y, z]>). | ||
|
|
||
| var readonlySpanParameter = collectionBuilderMethod.Parameters.Last(); | ||
| var collectionBuilderElementsPlaceholder = new BoundValuePlaceholder(syntax, readonlySpanParameter.Type) { WasCompilerGenerated = true }; | ||
| var collectionBuilderElementsPlaceholder = new BoundCollectionBuilderElementsPlaceholder(syntax, readonlySpanParameter.Type) { WasCompilerGenerated = true }; | ||
|
|
||
| var arguments = projectionCall.Arguments; | ||
| var argumentNames = projectionCall.ArgumentNamesOpt; | ||
|
|
@@ -1365,8 +1368,9 @@ static BoundCollectionExpressionSpreadElement bindSpreadElement( | |
| invokedAsExtensionMethod: false, | ||
| argsToParamsOpt: argsToParams, | ||
| defaultArguments: projectionCall.DefaultArguments, | ||
| resultKind: LookupResultKind.Viable, | ||
| type: collectionBuilderMethod.ReturnType) | ||
| resultKind: projectionCall.ResultKind, | ||
| type: collectionBuilderMethod.ReturnType, | ||
| hasErrors: projectionCall.HasErrors) | ||
|
RikkiGibson marked this conversation as resolved.
|
||
| { | ||
| WasCompilerGenerated = @this._node.WithElement is null, | ||
| }; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a test verifying what happens with
WithElementin IOperation tree when we hit this code path. #ClosedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
511ca83
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please provide the name of the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestArray_Empty, TestArray_SingleArg, TestArray_NamedArg