Skip to content
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

Determine element type for write-only CollectionBuilder collection types #7895

Merged
merged 12 commits into from
Mar 21, 2024
69 changes: 53 additions & 16 deletions proposals/csharp-12.0/collection-expressions.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,22 +103,25 @@ Collection literals are [target-typed](https://github.com/dotnet/csharplang/blob
A *collection expression conversion* allows a collection expression to be converted to a type.

An implicit *collection expression conversion* exists from a collection expression to the following types:
* A single dimensional *array type* `T[]`
* A single dimensional *array type* `T[]`, in which case the *element type* is `T`
* A *span type*:
* `System.Span<T>`
* `System.ReadOnlySpan<T>`
* A *type* with a *[create method](#create-methods)* with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) determined from a `GetEnumerator` instance method or enumerable interface, not from an extension method
* `System.ReadOnlySpan<T>`
in which cases the *element type* is `T`
Comment on lines +109 to +110
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `System.ReadOnlySpan<T>`
in which cases the *element type* is `T`
* `System.ReadOnlySpan<T>`, in which cases the *element type* is `T`

* A *type* with an appropriate *[create method](#create-methods)* and a corresponding *element type* resulting from that determination
* A *struct* or *class type* that implements `System.Collections.IEnumerable` where:
* The *type* has an *[applicable](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11642-applicable-function-member)* constructor that can be invoked with no arguments, and the constructor is accessible at the location of the collection expression.
* If the collection expression has any elements, the *type* has an *[applicable](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11642-applicable-function-member)* instance or extension method `Add` that can be invoked with a single argument of the [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement), and the method is accessible at the location of the collection expression.
* The *type* has an *[applicable](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11642-applicable-function-member)* constructor that can be invoked with no arguments, and the constructor is accessible at the location of the collection expression, and
* If the collection expression has any elements, the *type* has an *[applicable](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11642-applicable-function-member)* instance or extension method `Add` that can be invoked with a single argument of the [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement), and the method is accessible at the location of the collection expression,
in which case the *element type* is the *iteration type* of the *type*.
* An *interface type*:
* `System.Collections.Generic.IEnumerable<T>`
* `System.Collections.Generic.IReadOnlyCollection<T>`
* `System.Collections.Generic.IReadOnlyList<T>`
* `System.Collections.Generic.ICollection<T>`
* `System.Collections.Generic.IList<T>`
* `System.Collections.Generic.IList<T>`
in which cases the *element type* is `T`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
in which cases the *element type* is `T`
in which, for all cases, the *element type* is `T`


The implicit conversion exists if the type has an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `U` where for each *element* `Eᵢ` in the collection expression:
The implicit conversion exists if the type has an *element type* `U` where for each *element* `Eᵢ` in the collection expression:
* If `Eᵢ` is an *expression element*, there is an implicit conversion from `Eᵢ` to `U`.
* If `Eᵢ` is an *spread element* `Sᵢ`, there is an implicit conversion from the *iteration type* of `Sᵢ` to `U`.

Expand Down Expand Up @@ -158,21 +161,33 @@ namespace System.Runtime.CompilerServices
The attribute can be applied to a `class`, `struct`, `ref struct`, or `interface`.
The attribute is not inherited although the attribute can be applied to a base `class` or an `abstract class`.

The collection type must have an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement).
The *builder type* must be a non-generic `class` or `struct`.

For the *create method*:
First, the set of applicable *create methods* `CM` is determined.
It consists of methods that meet the following requirements:

* The *builder type* must be a non-generic `class` or `struct`.
* The method must have name specified in the `[CollectionBuilder(...)]` attribute.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

total nit: but for ultra pedantry, we probably want to say that the method is not an explicit-impl. but maybe that falls out since an explicit impl method would not be accessible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That lapse seems to predate this PR. I also noticed that the current speclet doesn't mention an substitution for the create method. Those should probably be addressed separately.

Copy link
Contributor

@AlekseyTs AlekseyTs Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, we do not care if the method explicitly implements anything as long as it is accessible. And yes, such methods exist in metadata. What we, perhaps, should care about that is whether the name in the attribute is a valid identifier.

jcouv marked this conversation as resolved.
Show resolved Hide resolved
* The method must be defined on the *builder type* directly.
* The method must be `static`.
* The method must be accessible where the collection expression is used.
* The *arity* of the method must match the *arity* of the collection type.
* The method must have a single parameter of type `System.ReadOnlySpan<E>`, passed by value, and there is an [*identity conversion*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/conversions.md#1022-identity-conversion) from `E` to the [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) of the *collection type*.
* The method must have a single parameter of type `System.ReadOnlySpan<E>`, passed by value.
* There is an [*identity conversion*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/conversions.md#1022-identity-conversion), [*implicit reference conversion*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/conversions.md#1028-implicit-reference-conversions), or [*boxing conversion*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/conversions.md#1029-boxing-conversions) from the method return type to the *collection type*.
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved

An error is reported if the `[CollectionBuilder]` attribute does not refer to an invocable method with the expected signature.
Methods declared on base types or interfaces are ignored and not part of the `CM` set.

Method overloads on the *builder type* with distinct signatures are ignored. Methods declared on base types or interfaces are ignored.
If the `CM` set is empty, then the *collection type* doesn't have *element type* and doesn't have *create method*. None of the following steps apply.
jcouv marked this conversation as resolved.
Show resolved Hide resolved

Second, an attempt is made to determine the [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) of the *collection type* from a `GetEnumerator` instance method or enumerable interface, not from an extension method.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps say "unambiguous enumerable interface"? but nbd here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional. See note in OP.


If an *iteration type* can be determined, then the *element type* of the *collection type* is the *iteration type*. If only one method among those in the `CM` set has an [*identity conversion*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/conversions.md#1022-identity-conversion) from `E` to the *element type* of the *collection type*, that is the *create method* for the *collection type*. Otherwise, the *collection type* doesn't have *create method*. None of the following steps apply.
jcouv marked this conversation as resolved.
Show resolved Hide resolved

Third (ie. if an *iteration type* cannot be determined), an attempt is made to infer the *element type*.
If the `CM` set contains more than one method, the inference fails and the *collection type* doesn't have an *element type* and doesn't have a *create method*.

Otherwise, type `E1` is determined from the only method `M` in the `CM` set by substituting the type parameters of the *collection type* for the type parameters of `M` in `E`. If any generic constraints are violated for `E1`, the *collection type* doesn't have *element type* and doesn't have *create method*. Otherwise, `E1` is the *element type* and `M` is the *create method* for the *collection type*.
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by substituting the type parameters of the collection type for the type parameters of M in E

This looks backwards and doesn't match the wording that was suggested. E is a type in M's signature. it cannot refer to "type parameters of the collection type", there is nothing to substitute there in the suggested way. #Closed


An error is reported if the `[CollectionBuilder]` attribute does not refer to an invokable method with the expected signature.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same as saying "An error is reported if the type decorated with [CollectionBuilder] does not have a create method"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have misunderstood the question. The type decorated with [CollectionBuilder] is the collection type. The [CollectionBuilder] attribute points to a builder type and a create method.
So we're not expecting a create method in the type decorated with [CollectionBuilder].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, this line attempts to describe existing rules (which were not fully spec'ed). This line doesn't propose a change in behavior.


For a *collection expression* with a target type <code>C&lt;S<sub>0</sub>, S<sub>1</sub>, &mldr;&gt;</code> where the *type declaration* <code>C&lt;T<sub>0</sub>, T<sub>1</sub>, &mldr;&gt;</code> has an associated *builder method* <code>B.M&lt;U<sub>0</sub>, U<sub>1</sub>, &mldr;&gt;()</code>, the *generic type arguments* from the target type are applied in order &mdash; and from outermost containing type to innermost &mdash; to the *builder method*.

Expand Down Expand Up @@ -377,7 +392,7 @@ The existing rules for the [*first phase*](https://github.com/dotnet/csharpstand
>
> An *input type inference* is made *from* an expression `E` *to* a type `T` in the following way:
>
> * If `E` is a *collection expression* with elements `Eᵢ`, and `T` is a type with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Tₑ` or `T` is a *nullable value type* `T0?` and `T0` has an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Tₑ`, then for each `Eᵢ`:
> * If `E` is a *collection expression* with elements `Eᵢ`, and `T` is a type with an *element type* `Tₑ` or `T` is a *nullable value type* `T0?` and `T0` has an *element type* `Tₑ`, then for each `Eᵢ`:
> * If `Eᵢ` is an *expression element*, then an *input type inference* is made *from* `Eᵢ` *to* `Tₑ`.
> * If `Eᵢ` is an *spread element* with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Sᵢ`, then a [*lower-bound inference*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#116310-lower-bound-inferences) is made *from* `Sᵢ` *to* `Tₑ`.
> * *[existing rules from first phase]* ...
Expand All @@ -386,7 +401,7 @@ The existing rules for the [*first phase*](https://github.com/dotnet/csharpstand
>
> An *output type inference* is made *from* an expression `E` *to* a type `T` in the following way:
>
> * If `E` is a *collection expression* with elements `Eᵢ`, and `T` is a type with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Tₑ` or `T` is a *nullable value type* `T0?` and `T0` has an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Tₑ`, then for each `Eᵢ`:
> * If `E` is a *collection expression* with elements `Eᵢ`, and `T` is a type with an *element type* `Tₑ` or `T` is a *nullable value type* `T0?` and `T0` has an *element type* `Tₑ`, then for each `Eᵢ`:
> * If `Eᵢ` is an *expression element*, then an *output type inference* is made *from* `Eᵢ` *to* `Tₑ`.
> * If `Eᵢ` is an *spread element*, no inference is made from `Eᵢ`.
> * *[existing rules from output type inferences]* ...
Expand Down Expand Up @@ -437,7 +452,7 @@ In the updated rules:
>
> * **`E` is a *collection expression* and one of the following holds:**
> * **`T₁` is `System.ReadOnlySpan<E₁>`, and `T₂` is `System.Span<E₂>`, and an implicit conversion exists from `E₁` to `E₂`**
> * **`T₁` is `System.ReadOnlySpan<E₁>` or `System.Span<E₁>`, and `T₂` is an *array_or_array_interface* with *iteration type* `E₂`, and an implicit conversion exists from `E₁` to `E₂`**
> * **`T₁` is `System.ReadOnlySpan<E₁>` or `System.Span<E₁>`, and `T₂` is an *array_or_array_interface* with *element type* `E₂`, and an implicit conversion exists from `E₁` to `E₂`**
> * **`T₁` is not a *span_type*, and `T₂` is not a *span_type*, and an implicit conversion exists from `T₁` to `T₂`**
> * **`E` is not a *collection expression* and one of the following holds:**
> * `E` exactly matches `T₁` and `E` does not exactly match `T₂`
Expand Down Expand Up @@ -830,6 +845,28 @@ However, given the breadth and consistency brought by the new literal syntax, we
## Unresolved questions
[unresolved]: #unresolved-questions

* Should we allow inferring the *element type* when the *iteration type* is "ambiguous" (by some definition)?
For example:
```csharp
Collection x = [1L, 2L];

// error CS1640: foreach statement cannot operate on variables of type 'Collection' because it implements multiple instantiations of 'IEnumerable<T>'; try casting to a specific interface instantiation
foreach (var x in new Collection) { }

static class Builder
{
public Collection Create(ReadOnlySpan<long> items) => throw null;
}

[CollectionBuilder(...)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is an error reported for this attribute by the design in this PR?

class Collection : IEnumerable<int>, IEnumerable<string>
{
IEnumerator<int> IEnumerable<int>.GetEnumerator() => throw null;
IEnumerator<string> IEnumerable<string>.GetEnumerator() => throw null;
IEnumerator IEnumerable.GetEnumerator() => throw null;
}
```

* Should it be legal to create and immediately index into a collection literal? Note: this requires an answer to the unresolved question below of whether collection literals have a *natural type*.
* Stack allocations for huge collections might blow the stack. Should the compiler have a heuristic for placing this data on the heap? Should the language be unspecified to allow for this flexibility? We should follow the spec for [`params Span<T>`](https://github.com/dotnet/csharplang/issues/1757).
* Do we need to target-type `spread_element`? Consider, for example:
Expand Down
28 changes: 15 additions & 13 deletions proposals/params-collections.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,30 @@ A *parameter_collection* consists of an optional set of *attributes*, a `params`
a *type*, and an *identifier*. A parameter collection declares a single parameter of the given type with the given name.
The *type* of a parameter collection shall be one of the following valid target types for a collection expression
(see https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#conversions):
- A single dimensional *array type* `T[]`
- A single dimensional *array type* `T[]`, in which case the *element type* is `T`
- A *span type*
- `System.Span<T>`
- `System.ReadOnlySpan<T>`
- A *type* with a *[create method](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#create-methods)*,
which is at least as accessible as the declaring member, and with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/draft-v9/standard/statements.md#1395-the-foreach-statement)
determined from a `GetEnumerator` instance method or enumerable interface, not from an extension method.
- `System.ReadOnlySpan<T>`
in which cases the *element type* is `T`
Comment on lines +53 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `System.ReadOnlySpan<T>`
in which cases the *element type* is `T`
- `System.ReadOnlySpan<T>`, in which cases the *element type* is `T`

- A *type* with an appropriate *[create method](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#create-methods)*,
which is at least as accessible as the declaring member, and with a corresponding *element type* resulting from that determination
- A *struct* or *class type* that implements `System.Collections.IEnumerable` where:
- The *type* has a constructor that can be invoked with no arguments, and the constructor is at least as accessible as the declaring member.
- The *type* has a constructor that can be invoked with no arguments, and the constructor is at least as accessible as the declaring member, and
- The *type* has an instance (not an extension) method `Add` that can be invoked with a single argument of
the [*iteration type*](https://github.com/dotnet/csharpstandard/blob/draft-v9/standard/statements.md#1395-the-foreach-statement),
and the method is at least as accessible as the declaring member.
and the method is at least as accessible as the declaring member,
in which case the *element type* is the *iteration type*
- An *interface type*
- `System.Collections.Generic.IEnumerable<T>`,
- `System.Collections.Generic.IReadOnlyCollection<T>`,
- `System.Collections.Generic.IReadOnlyList<T>`,
- `System.Collections.Generic.ICollection<T>`,
- `System.Collections.Generic.IList<T>`
- `System.Collections.Generic.IList<T>`
in which cases the *element type* is `T`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems unfortunate that this is restated in params-collections, instead of being able to reference collection-exprs (but that's outside the scope of this pr).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
in which cases the *element type* is `T`
in which, for all cases, the *element type* is `T`


In a method invocation, a parameter collection permits either a single argument of the given parameter type to be specified, or
it permits zero or more arguments of the collection [iteration type](https://github.com/dotnet/csharpstandard/blob/draft-v9/standard/statements.md#1395-the-foreach-statement)
to be specified. Parameter collections are described further in *[Parameter collections](#parameter-collections)*.
it permits zero or more arguments of the collection's *element type* to be specified.
jcouv marked this conversation as resolved.
Show resolved Hide resolved
Parameter collections are described further in *[Parameter collections](#parameter-collections)*.

A *parameter_collection* may occur after an optional parameter, but cannot have a default value – the omission of arguments for a *parameter_collection*
would instead result in the creation of an empty collection.
Expand All @@ -87,7 +89,7 @@ A parameter collection permits arguments to be specified in one of two ways in a
- The argument given for a parameter collection can be a single expression that is implicitly convertible to the parameter collection type.
In this case, the parameter collection acts precisely like a value parameter.
- Alternatively, the invocation can specify zero or more arguments for the parameter collection, where each argument is an expression
that is implicitly convertible to the parameter collection [iteration type](https://github.com/dotnet/csharpstandard/blob/draft-v9/standard/statements.md#1395-the-foreach-statement).
that is implicitly convertible to the parameter collection's *element type*.
In this case, the invocation creates an instance of the parameter collection type according to the rules specified in
[Collection expressions](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md)
as though the arguments were used as expression elements in a collection expression in the same order,
Expand Down Expand Up @@ -117,7 +119,7 @@ The [Applicable function member](https://github.com/dotnet/csharpstandard/blob/d
If a function member that includes a parameter collection is not applicable in its normal form, the function member might instead be applicable in its ***expanded form***:

- The expanded form is constructed by replacing the parameter collection in the function member declaration with
zero or more value parameters of the parameter collection [iteration type](https://github.com/dotnet/csharpstandard/blob/draft-v9/standard/statements.md#1395-the-foreach-statement)
zero or more value parameters of the parameter collection's *element type*
such that the number of arguments in the argument list `A` matches the total number of parameters.
If `A` has fewer arguments than the number of fixed parameters in the function member declaration,
the expanded form of the function member cannot be constructed and is thus not applicable.
Expand Down Expand Up @@ -158,7 +160,7 @@ In case the parameter type sequences `{P₁, P₂, ..., Pᵥ}` and `{Q₁, Q₂,
- **params collection of `Mᵢ` is `System.ReadOnlySpan<Eᵢ>`, and params collection of `Mₑ` is `System.Span<Eₑ>`, and an implicit conversion exists from `Eᵢ` to `Eₑ`**
- **params collection of `Mᵢ` is `System.ReadOnlySpan<Eᵢ>` or `System.Span<Eᵢ>`, and params collection of `Mₑ` is
an *[array_or_array_interface__type](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#overload-resolution)*
with *[iteration type](https://github.com/dotnet/csharpstandard/blob/draft-v9/standard/statements.md#1395-the-foreach-statement)* `Eₑ`, and an implicit conversion exists from `Eᵢ` to `Eₑ`**
with *element type* `Eₑ`, and an implicit conversion exists from `Eᵢ` to `Eₑ`**
- **both params collections are not *span_type*s, and an implicit conversion exists from params collection of `Mᵢ` to params collection of `Mₑ`**
- Otherwise, no function member is better.

Expand Down