Collection expression arguments: collection construction #9066
Conversation
|
|
||
| If *collection_arguments* is not the first element in the collection expression, an error is reported. | ||
|
|
||
| If the target type is an *array*, *span*, *array interface*, or *generic parameter type*, and the *argument list* is not empty, a binding error is reported. |
There was a problem hiding this comment.
i would be ok saying that it's an error to have the element, even if had an empty set of arguments. these guys do not take args at all, so providing them (even 0 length) seems confusing :)
There was a problem hiding this comment.
The array interface IList<T> causes us to construct List<T>, and we should allow args(capacity: ...) then. You'd mentioned wanting with(comparer: ...) for the IDictionary interface, knowing that Dictionary<K, V> was getting created.
There was a problem hiding this comment.
I think I prefer only one of these alternatives:
- always disallow
with()(i.e. require at least one arg) - always allow
with()
I don't like that allowing with() would fluctuate based on target type or based on how the compiler evolves to permit new args to existing target types in the future.
| * Otherwise, a binding error is reported. | ||
|
|
||
| If the target type is a type with a *create method*, then: | ||
| * The *argument list* is a concatenation of *some `ReadOnlySpan<T>` value (TBD)* and any explicit *argument list*. |
There was a problem hiding this comment.
are we saying 'concatenation' here because we're not certain if it is X arg1, Yarg2, ReadOnlySpan<T> values vs ReadOnlySpan<T> values, X arg, Y arg2 ?
There was a problem hiding this comment.
I was assuming the ReadOnlySpan<T> parameter is the first parameter in the create method, so the concatenated arguments should have the ReadOnlySpan<T> argument first.
There was a problem hiding this comment.
we should check with @stephentoub about his preferences here. this will affect the API signatures for the Create methods for immutable/frozen dictionaries.
There was a problem hiding this comment.
Thanks for looping me in. Can you spell this out for me? What are the ramifications we're considering?
There was a problem hiding this comment.
Effectively the shape of the Create methods we'll accept where we can both pass values (in a ROS), as well as other arguments (like perhaps comparers and other type specific args.
Ideally we have a very simple way to do this that both works for you from an API design and impl perspective, and then it's easy for us to spec and find/use in the compiler.
There was a problem hiding this comment.
Thanks.
We already have existing APIs that ideally could be used, e.g. https://github.com/dotnet/runtime/blob/1af7c2370bce80cba73d442d69f4a2f1b02dcbef/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs#L29. params parameters needing to be at the end forces us to employ a specific ordering for some of these, but in other cases where we've not relied on params, the comparer more typically comes last, e.g. it'd be nice if we could add a span-based overload of this ctor and have it be used as the target for building: https://github.com/dotnet/runtime/blob/1af7c2370bce80cba73d442d69f4a2f1b02dcbef/src/libraries/System.Collections/src/System/Collections/Generic/OrderedDictionary.cs#L200. (We'd need to address the other limitations around being able to use ctors.)
|
That's a very good point. For the interfaces where we guarantee a concrete type, we should map the with/args/init element to the constructors of that type. For the read-only versions, it gets murkier. We will likely need to special case IReadOnlyDict |
| * If the *argument list* is a single value, implicitly convertible to `IEqualityComparer<K>`, and optionally with name `comparer`, a dictionary instance is constructed with that key comparer value. | ||
| If the target type is an *interface type*, then: | ||
| * [*Overload resolution*](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#1264-overload-resolution) is used to determine the best instance constructor from the *argument list* from the following candidate signatures: | ||
| * If the target type is `IEnumerable<E>`, `ICollection<E>`, or `IList<E>`, the candidates are: |
There was a problem hiding this comment.
It's only the mutation interfaces that we guarantee building concrete List<T> for. IEnumerable<T> is in with IReadOnlyCollection<T> etc where we use a compiler-generated wrapper to prevent mutations. If the reason for exposing new(int capacity) is that users know they are getting List<T>, that would not apply to the IEnumerable<T> target. If there is a separate reason to expose capacity that applies to IEnumerable<T>, I would expect that same reason to apply to IReadOnlyCollection<T> etc.
| * If the target type is `ICollection<E>`, or `IList<E>`, the candidates are: | ||
| * `new()` | ||
| * `new(int capacity)` | ||
| * If the target type is `IEnumerable<E>`, `IReadOnlyCollection<E>`, or `IReadOnlyList<E>`, the candidates are: |
There was a problem hiding this comment.
i would prefer that tehre are no constructors for these.
| * `new()` | ||
| * `new(int capacity)` | ||
| * `new(IEqualityComparer<K, V> comparer)` | ||
| * `new(int capacity, IEqualityComparer<K, V> comparer)` |
There was a problem hiding this comment.
i would prefer we say that the candidates are the accessible (or public) constructors of Dictionary`2
|
|
||
| If the target type is an *interface type*, then: | ||
| * [*Overload resolution*](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#1264-overload-resolution) is used to determine the best instance constructor from the *argument list* from the following candidate signatures: | ||
| * If the target type is `ICollection<E>`, or `IList<E>`, the candidates are: |
There was a problem hiding this comment.
i would prefer we say that it's the constructors of List`1. Basically, we already guarantee we use that type for the mutable list collections type. So it's fine to then say that it binds to the constructors of that type (not a fixed list in the spec).
There was a problem hiding this comment.
Yeah, and on runtimes where the List<T> that is found doesn't have an int capacity ctor, then there are no issues.
There was a problem hiding this comment.
It looks like the equivalent constructors for List<T> and Dictionary<TKey, TValue> are available on all platforms since .NET Framework 2.0.
There was a problem hiding this comment.
my general point is simply that i don't want us to define teh set. we should just say that it maps to the accessible (or public) constructors of that type. :) that way you can hit other constructors if hte runtime ever adds them.
| * `new(int capacity, IEqualityComparer<K, V> comparer)` | ||
| * If the target type is `IReadOnlyDictionary<K, V>`, the candidates are: | ||
| * `new()` | ||
| * `new(IEqualityComparer<K, V> comparer)` |
There was a problem hiding this comment.
this one is interesting. i do think we need to say this.
| Within *collection arguments*, the arguments are evaluated in order, left to right. | ||
| Each element or argument is evaluated exactly once, and any further references refer to the results of this initial evaluation. | ||
|
|
||
| If *collection_arguments* is not the first element in the collection expression, a compile-time error is reported. |
There was a problem hiding this comment.
Could be read to mean "if the first element is not a collection_arguments element" which would imply an error when there is no collection_arguments element.
| If *collection_arguments* is not the first element in the collection expression, a compile-time error is reported. | |
| If *collection_arguments* appears in the collection expression and is not the first element, a compile-time error is reported. |
Specify construction rules when collection arguments are included.