112 lines
7.2 KiB
Markdown
112 lines
7.2 KiB
Markdown
# C# Language Design Meeting for September 14th, 2020
|
|
|
|
## Agenda
|
|
|
|
1. [Partial method signature matching](#partial-method-signature-matching)
|
|
2. [Null-conditional handling of the nullable suppression operator](#null-conditional-handling-of-the-nullable-suppression-operator)
|
|
3. [Annotating IEnumerable.Cast](#annotating-ienumerable.cast)
|
|
4. [Nullability warnings in user-written record code](#nullability-warnings-in-user-written-record-code)
|
|
5. [Tuple deconstruction mixed assignment and declaration](#tuple-deconstruction-mixed-assignment-and-declaration)
|
|
|
|
## Quote of the Day
|
|
|
|
- "Now with warning waves it's a foam-covered baseball bat to hit people with"
|
|
|
|
## Discussion
|
|
|
|
### Partial method signature matching
|
|
|
|
https://github.com/dotnet/roslyn/issues/45519
|
|
|
|
There is a question about what amount of signature matching is required for method signatures, both as part of the expanded partial methods in C# 9
|
|
and for the new `nint` feature in C# 9. Currently, our rules around what has to match and what does not are confusing: tuple names must match,
|
|
`dynamic`/`object` do not have to match, we warn when there are unsafe nullability conversions, and other differences are allowed (including parameter
|
|
names). We could lean on a warning wave here and make our implementation more consistent with the following general rules:
|
|
|
|
1. If there are differences in the CLR signature, we error (as we cannot emit code at all!)
|
|
2. If there are differences in the syntactic signature, we warn (even for safe nullability changes).
|
|
|
|
While we like this proposal in general, we have a couple of concerns around compatibility. Tuple names erroring is existing behavior, and if we loosen
|
|
that to a general warning that would need to be gated behind language version, as you could write code with a newer compiler in C# 8 mode that does not
|
|
compile with the C# 8 compiler. This complicates implementation, and to make it simple we lean towards just leaving tuple name differences as an error.
|
|
We also want to make sure that nullability is able to differ by obliviousness: a common case for generators is that either the original signature or the
|
|
implementation will be unaware of nullable, and we don't want to break this scenario such that either both the user and generator must be nullable aware,
|
|
or the must both be nullable unaware.
|
|
|
|
We also considered an extension to these rules where we make the new rules always apply to the new enhanced partial methods, regardless of warning level.
|
|
However, we believe that this would result in a complicated user experience and would make the mental model harder to understand.
|
|
|
|
#### Conclusion
|
|
|
|
1. We will keep the existing error that tuple names must match.
|
|
2. We will keep the existing warnings about unsafe nullability differences.
|
|
3. We will add a new warning wave warning for all other syntactic differences between partial method implementations.
|
|
a. This includes differences like parameter names and `dynamic`/`object`
|
|
b. This includes nullability differences where both contexts are nullable enabled, even if the difference is supposedly "safe" (accepting `null`
|
|
where it is not accepted today).
|
|
c. If nullability differs by enabled state (one part is enabled, the other part is disabled), this will be allowed without warning.
|
|
|
|
### Null-conditional handling of the nullable suppression operator
|
|
|
|
https://github.com/dotnet/csharplang/issues/3393
|
|
|
|
This is a spec bug that shipped with C# 8, where the `!` operator does not behave as a user would expect. Members of the LDT believe that this is broken
|
|
on the same level as the `for` iterator variable behavior that was changed in C# 5, and we believe that we should take a similar breaking change to fix
|
|
the behavior here. We have made a grammatical proposal for adjusting how null-conditional statements are parsed, and there was general agreement that this
|
|
proposal is where we want to go. The only comment is that `null_conditional_operations_no_suppression` should be renamed to avoid confusion, as there can
|
|
be a null suppression inside the term, just not at the end. A better name would be `null_conditional_operations_no_final_suppression`.
|
|
|
|
#### Conclusion
|
|
|
|
Accepted, with the above rename. Will get a compiler developer assigned to implement this ASAP.
|
|
|
|
### Annotating IEnumerable.Cast
|
|
|
|
https://github.com/dotnet/runtime/issues/40518
|
|
|
|
In .NET 5, the `Cast<TResult>` method was annotated on the return to return `IEnumerable<TResult?>`, which means that regardless of whether the input
|
|
enumerable can contain `null` elements, the returned enumerable would be considered to contain `null`s. This resulted in some spurious warnings when
|
|
upgrading roslyn to use a newer version of .NET 5. However, the C# in general lacks the ability to properly annotate this method for a combination of
|
|
reasons:
|
|
|
|
1. There is no way to express that the nullability of one type parameter depends on the nullability of another type parameter.
|
|
2. Even if there was a way to express 1, `Cast` is an extension method on `IEnumerable`, not `IEnumerable<T>`, because C# does not have partial type
|
|
inference to make writing code in this scenario better.
|
|
|
|
Given this, we have a few options:
|
|
|
|
1. Leave the method as is, and possibly enhance the compiler/language to know about this particular method. This is analogous to the changes we're
|
|
considering with `Where`, but it feels like a bad solution as it's not generalizable.
|
|
2. Make the method return `TResult`, unannotated. The issue with this is that it effectively means the method might actually lie: there is no way to
|
|
ensure that the method actually returns a non-null result if a non-null `TResult` is provided as a type, given that nullability is erased in the
|
|
implementation. We're concerned that this could make the docs appear to lie, which we think would also give a bad experience.
|
|
3. Convert `Cast` back to being unannotated. This seems to be compromise that both sides can agree on: analyzers can flag use of the unannotated API
|
|
to help users, and spurious warnings get suppressed. It also matches the behavior of `IEnumerator.Current`, and means that the behavior of `foreach`
|
|
loops over such a list behave in a consistent manner.
|
|
|
|
#### Conclusion
|
|
|
|
The BCL will make `Cast` and a few related APIs an oblivious API.
|
|
|
|
### Nullability warnings in user-written record code
|
|
|
|
The question here is on whether we should warn users when manually-implemented methods and properties for well-known members in a `record` should warn
|
|
when nullability is different. For example, if their `Equals(R other)` does not accept `null`. There was no debate on this.
|
|
|
|
#### Conclusion
|
|
|
|
we'll check user-defined `EqualityContract`, `Equals`, `Deconstruct`, ... methods on records for nullability safety issues in their declaration. For
|
|
example, `EqualityContract` should not return Type?.
|
|
|
|
### Tuple deconstruction mixed assignment and declaration
|
|
|
|
https://github.com/dotnet/csharplang/issues/125
|
|
|
|
We've discussed this feature in the past (https://github.com/dotnet/csharplang/blob/master/meetings/2016/LDM-2016-11-30.md#mixed-deconstruction), and
|
|
we liked it then but didn't think it would fit into C# 7. It's been in Any Time since, and now we have a community PR. We have no concerns with
|
|
moving forward with the feature.
|
|
|
|
#### Conclusion
|
|
|
|
Let's try and get this into C# 10.
|