Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update first-class Span speclet with LDM decisions #8241

Merged
merged 4 commits into from
Jun 28, 2024
Merged

Update first-class Span speclet with LDM decisions #8241

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04ac4ef
Update first-class Span speclet with LDM decisions
jjonescz Jun 25, 2024
022e399
Improve wording
jjonescz Jun 26, 2024
e27ead7
Remove answered open question
jjonescz Jun 26, 2024
699e0b8
Remove well known type restriction
jjonescz Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 15 additions & 50 deletions proposals/first-class-span-types.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ The changes in this proposal will be tied to `LangVersion >= 13`.
### Implicit Span Conversions

We add a new type of implicit conversion to the list in [§10.2.1](https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/conversions.md#1021-general), an
_implicit span conversion_. This conversion is a conversion from expression and is defined as follows:
_implicit span conversion_. This conversion is a conversion from type and is defined as follows:

------

Expand All @@ -50,6 +50,9 @@ An implicit span conversion permits `array_types`, `System.Span<T>`, `System.Rea

------

Any Span/ReadOnlySpan types are considered applicable for the conversion if they match by their fully-qualified name.
If they do not match the types that are selected by the compiler's well-known type resolution logic, a compile-time error is reported.
Copy link
Member

Choose a reason for hiding this comment

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

This is not what we decided? We won't check that, and we can't really specify it either, since "the compiler's well-known type resolution logic" isn't a thing in the spec.

Copy link
Member Author

@jjonescz jjonescz Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, okay, thanks. I thought that was part of the option we discussed, but I might have misunderstood.

Copy link
Member Author

@jjonescz jjonescz Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Then the question is how will we emit - do we need to find the operators on the Span types manually? It would be much easier to just use the well-known member machinery; and hence report an error if the Span type doesn't match the well-known type. Maybe that doesn't have to be specified, it can be an implementation detail like well-known types are. I could replace this sentence with something like:

Compiler implementation can report a compile-time error if the Span type from the conversion
does not match the "well-known" Span type that would be selected in other places by implementation-specific logic.

But I guess it might not be so difficult to find the required helpers.


We also add _implicit span conversion_ to the list of standard implicit conversions
([§10.4.2](https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/conversions.md#1042-standard-implicit-conversions)). This allows overload resolution to consider
them when performing argument resolution, as in the previously-linked API proposal.
Expand All @@ -61,11 +64,21 @@ The explicit span conversions are the following:
There is no standard explicit span conversion unlike other *standard explicit conversions* ([§10.4.3][standard-explicit-conversions])
which always exist given the opposite standard implicit conversion.

#### User defined conversions

User-defined conversions are not considered when converting between
- any single-dimensional `array_type` and `System.Span<T>`/`System.ReadOnlySpan<T>`,
- any combination of `System.Span<T>`/`System.ReadOnlySpan<T>`,
- `string` and `System.ReadOnlySpan<char>`.

The implicit span conversions are exempted from the rule
that it is not possible to define a user-defined operator between types for which a non-user-defined conversion exists
([§10.5.2 Permitted user-defined conversions][permitted-udcs]).
This is needed so BCL can keep defining the existing Span conversion operators even when they switch to C# 13
(to avoid binary breaking changes and also because these operators are used in codegen of the new standard span conversion).

#### Extension receiver

We also add _implicit span conversion_ to the list of acceptable implicit conversions on the first parameter of an extension method when determining applicability
([12.8.9.3](https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/expressions.md#12893-extension-method-invocations)) (change in bold):

Expand Down Expand Up @@ -226,33 +239,6 @@ static class C
> [the new `OverloadResolutionPriorityAttribute`][overload-resolution-priority].
> See also [an open question](#unrestricted-betterness-rule) below.

#### Better conversion target

*Better conversion target* ([§12.6.4.7][better-conversion-target]) is updated to consider implicit span conversions.

> Given two types `T₁` and `T₂`, `T₁` is a *better conversion target* than `T₂` if one of the following holds:
>
> - An implicit conversion from `T₁` to `T₂` exists and no implicit conversion from `T₂` to `T₁` exists
> **(the implicit span conversion is considered in this bullet point, even though it's a conversion from expression)**
> - [...]

This change is needed to avoid ambiguities in existing code that would arise
because we are now ignoring user-defined Span conversions which were considered in the "better conversion target" rule,
whereas "implicit Span conversion" would not be considered as it is a "conversion from expression", not a "conversion from type".

```cs
using System;
C.M(null); // used to print 1, would be ambiguous without this rule
static class C
{
public static void M(object[] x) => Console.Write(1);
public static void M(ReadOnlySpan<object> x) => Console.Write(2);
}
```

This rule is not needed if the span conversion is a conversion from type.
See [an open question][conversion-from-type] below.

### Type inference

We update the type inferences section of the specification as follows (changes in **bold**).
Expand Down Expand Up @@ -457,6 +443,7 @@ However, that would mean users could get different behavior after updating the t

Should we break existing code like the following (real code found in runtime)?
LDM recently allowed breaks related to new Span overloads (https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-06-17.md#params-span-breaks).
Currently, this speclet has a mitigation for this break in [the extension receiver section](#extension-receiver).

```cs
using System;
Expand All @@ -468,28 +455,6 @@ var toRemove = new int[] { 2, 3 };
list.RemoveAll(toRemove.Contains); // error CS1113: Extension method 'MemoryExtensions.Contains<int>(Span<int>, int)' defined on value type 'Span<int>' cannot be used to create delegates
```

### Conversion from type vs. from expression
[conversion-from-type]: #conversion-from-type-vs-from-expression

We now think it would be better if the span conversion would be a conversion from type:

- Nothing about span conversions cares what form the expression takes; it can be a local variable, a `new[]`, a collection expression, a field, a `stackalloc`, etc.
- We have to go through everywhere in the spec that doesn't accept conversions from expression and check if they need updating to accept span conversions.
Like [better conversion target](#better-conversion-target) above.

Note that it is not possible to define a user-defined operator between types for which a non-user-defined conversion exists ([§10.5.2 Permitted user-defined conversions][permitted-udcs]).
Hence, we would need to make an exception, so BCL can keep defining the existing Span conversion operators even when they switch to C# 13
(to avoid binary breaking changes and also because we use these operators in codegen of the new standard span conversion).

In Roslyn, type conversions do not have access to Compilation which is needed to access well-known type Span.
We see a couple of ways of solving this concern:

1. We make the new conversions only applicable to the case where Span comes from the corelib, which would significantly simplify this space for us.
This would mean the rules don't exist downlevel; on the one hand, they already partly have issues downlevel
(covariance won't exist downlevel because the helper API doesn't exist).
On the other hand, that could affect partners abilities to take them up.
2. We couple type conversions to a Compilation or look at other ways of providing the well-known type to it. This will take a bit of investigation.

## Alternatives

Keep things as they are.
Expand Down