-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Collection literals: address comments #68793
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
Collection literals: address comments #68793
Conversation
| } | ||
|
|
||
| var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder(syntax, isNewInstance: true, targetType) { WasCompilerGenerated = true }; | ||
| // PROTOTYPE: When generating a List<T>, should we use the well-known |
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.
Added open issue to test plan #66418.
| var result = element switch | ||
| { | ||
| BoundBadExpression => element, | ||
| // PROTOTYPE: Should spread elements support target type? |
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.
Added open issue to test plan #66418.
| } | ||
| } | ||
| """; | ||
| // PROTOTYPE: Should compile and run successfully: expectedOutput: "[1, 2, 3], " |
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.
| CompileAndVerify(new[] { source, s_collectionExtensions }, expectedOutput: "(System.Int32[]) [1, 2, 3], "); | ||
| } | ||
|
|
||
| // PROTOTYPE: Test other variance cases. |
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.
| } | ||
| } | ||
| """; | ||
| // PROTOTYPE: Confirm we should generate ListBase<int> instances for both x and y. |
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.
| [ConditionalTheory(typeof(CoreClrOnly))] | ||
| [CombinatorialData] | ||
| public void SpreadElement_01( | ||
| [CombinatorialValues("IEnumerable", "IEnumerable<int>", "int[]", "List<int>", "Span<int>", "ReadOnlySpan<int>")] string spreadType, |
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.
| } | ||
| """; | ||
| var comp = CreateCompilation(source); | ||
| // PROTOTYPE: Should spread elements support target type? (Should we infer IEnumerable<int>?) |
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.
Added open issue to test plan #66418.
| } | ||
| } | ||
| """; | ||
| // PROTOTYPE: Should spread elements support target type? (Should we infer IEnumerable<string>?) |
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.
Added open issue to test plan #66418.
| } | ||
|
|
||
| [ConditionalFact(typeof(CoreClrOnly), AlwaysSkip = "PROTOTYPE: 'IAsyncEnumerable<int>' does not contain a definition for 'GetAwaiter'")] | ||
| public void Async_03() |
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.
Added open issue to test plan #66418: support IAsyncEnumerable<T> for spread element?
…nto collections-comments
| { | ||
| // PROTOTYPE: This is not a great diagnostic. Users could easily run into this and be confused. Can we do | ||
| // better. For example: | ||
| // This is not a great diagnostic. Users could easily run into this and be confused. Can we do |
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.
Tracking issue?
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.
This particular case involves a dictionary element which is not currently supported. I've logged an issue to improve the diagnostic for the previous example (A)[1]: #68862.
333fred
left a comment
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.
One comment, otherwise LGTM
| return GetValEscape(switchExpr.SwitchArms.SelectAsArray(a => a.Value), scopeOfTheContainingExpression); | ||
|
|
||
| case BoundKind.CollectionLiteralExpression: | ||
| // PROTOTYPE: Revisit if spans may be optimized to avoid heap allocation. |
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 have a test plan #66418 item tracking this now.
…nto collections-comments
…nto collections-comments
Address PROTOTYPE comments.