-
Notifications
You must be signed in to change notification settings - Fork 1k
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 expression arguments: open questions #9158
Changes from 1 commit
287f082
c837efb
3af4fd1
1ec2b68
e9cc48f
74f6d2b
5942f56
b7c4f72
0d5e1f2
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 |
---|---|---|
|
@@ -398,13 +398,56 @@ Should arguments with `dynamic` type be allowed? That might require using the ru | |
|
||
## Open questions | ||
|
||
### Target types where arguments are *required* | ||
### Conversions | ||
|
||
Should collection arguments and the applicable methods affect convertibility of the collection expression? | ||
|
||
```csharp | ||
Print([with(comparer: null), 1, 2, 3]); // ambiguous or Print<int>(HashSet<int>)? | ||
|
||
static void Print<T>(List<T> list) { ... } | ||
static void Print<T>(HashSet<T> set) { ... } | ||
``` | ||
|
||
### Type inference | ||
|
||
Should collection arguments affect type inference of the collection expression? | ||
|
||
For example, consider the following type where the constructor is called directly: | ||
```csharp | ||
UseMyCollection([with(default)]); // error: cannot infer T | ||
UseMyCollection([with(1)]); // ok: UseMyCollection<int>()? | ||
|
||
static void UseMyCollection<T>(MyCollection<T> c) { ... } | ||
|
||
class MyCollection<T> : IEnumerable<T> | ||
{ | ||
public MyCollection(T arg = default) { ... } | ||
public void Add(T t) { ... } | ||
// ... | ||
} | ||
``` | ||
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. similar to above, the following is not legal today: |
||
|
||
The same question applies if the type is constructed from a builder method, or even `List<T>`: | ||
```csharp | ||
UseList([with(collection: [])]); // error: cannot infer T | ||
UseList([with(collection: [1])]); // ok: UseList<int>()? | ||
|
||
static void UseList<T>(List<T> list) { ... } | ||
``` | ||
|
||
### Support target types where arguments are *required*? | ||
|
||
Should collection expression conversions be supported to target types where arguments must be supplied because all of the constructors or factory methods require at least one argument? | ||
Such types could be used with collection expressions that include explicit `with()` arguments but the types could not be used for `params` parameters. | ||
|
||
A collection type where the constructor is called directly: | ||
For example, consider the following type where the constructor is called directly: | ||
```csharp | ||
MyCollection<object> c; | ||
c = []; // error: no arguments | ||
c = [with()]; // error: no 'capacity' | ||
c = [with(capacity: 1)]; // ok | ||
|
||
class MyCollection<T> : IEnumerable<T> | ||
{ | ||
public MyCollection(int capacity) { ... } | ||
|
@@ -413,23 +456,36 @@ class MyCollection<T> : IEnumerable<T> | |
} | ||
``` | ||
|
||
A collection type constructed with a builder method: | ||
The same question applies if the type is constructed from a builder method such as: | ||
```csharp | ||
[CollectionBuilder(typeof(MyBuilder), "Create")] | ||
class MyCollection<T> : IEnumerable<T> { ... } | ||
|
||
class MyBuilder | ||
{ | ||
public static MyCollection<T> Create<T>(ReadOnlySpan<T> items, int capacity) { ... } | ||
} | ||
``` | ||
|
||
For either of those cases, arguments are required: | ||
### Collection builder parameter order | ||
|
||
For *collection builder* methods, should the elements parameter be before or after any parameters for collection arguments? | ||
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. @stephentoub for thoughts. Are there different API shapes you want CBA to be able to point to wrt the elemtns vs other arguments? 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. We effectively have both orderings in the core libraries. Collection types like Dictionary typically have a constructor that takes a collection of source elements and then a comparer, e.g. But Create methods like these: To best take advantage of existing Create methods, we probably want to use the comparer-then-elements ordering (even though I don't particularly like that ordering, especially now that we have collection expressions and params isn't as valuable as it used to be). |
||
|
||
Elements first allows arguments to be optional. | ||
```csharp | ||
class MyDictionaryBuilder | ||
{ | ||
public static MyDictionary<K, V> Create<K, V>( | ||
ReadOnlySpan<KeyValuePair<K, V>> items, | ||
IEqualityComparer<K> comparer = null) { ... } | ||
} | ||
``` | ||
|
||
Arguments first allows elements to be a `params` parameter, to support calling directly. | ||
```csharp | ||
MyCollection<object> x; | ||
x = []; // error: no arguments | ||
x = [with()]; // error: no 'capacity' | ||
x = [with(capacity: 1)]; // ok | ||
class MyDictionaryBuilder | ||
{ | ||
public static MyDictionary<K, V> Create<K, V>( | ||
IEqualityComparer<K> comparer, | ||
params ReadOnlySpan<KeyValuePair<K, V>> items) { ... } | ||
} | ||
``` | ||
|
||
### Construction overloads for *interface types* | ||
|
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.
Print<int>(new(comparer: null))
gives:If that's ambiguous, it would be intuitive to me for
Print<int>([with(comparer: null)])
to also be ambiguous. The commonality being target-typing while providing constructor arguments.Given that, then since the presence of items is unrelated to the differentiation between the overloads, I don't know what the motivating reason would be to make
Print([with(comparer: null), 1, 2, 3])
be able to resolve the overload ambiguity.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.
agreed. and i imagine the same holds true of
Print<int>(new(comparer: null) { 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.
Thanks. I've added those examples.