148 lines
5.1 KiB
Markdown
148 lines
5.1 KiB
Markdown
# C# Language Design Notes for April 22, 2019
|
|
|
|
## Agenda
|
|
|
|
1. Inferred nullable state from a finally block
|
|
2. Implied constraint for a type parameter of a partial?
|
|
3. Target-typed switch expression
|
|
4. DefaultCancellationAttribute and overriding/hiding/interface implementation
|
|
|
|
## Discussion
|
|
|
|
### Nullable Reference Types
|
|
|
|
#### Inferred nullable state from a finally block
|
|
|
|
See https://github.com/dotnet/roslyn/issues/34018
|
|
|
|
> I don't think our current inference design interacts with finally blocks well:
|
|
|
|
```C#
|
|
C? c = null;
|
|
try
|
|
{
|
|
c = SomeNonNullComputation;
|
|
}
|
|
finally
|
|
{
|
|
if (c != null) c.Cleanup();
|
|
}
|
|
c.Operation(); // undeserved warning
|
|
```
|
|
|
|
> We infer from c != null that c might be null. That inference leaks out to the enclosing construct. The result is a warning when c is used after the try-finally statement. This will be a common pain point.
|
|
|
|
> I don't think indirect inferences from inside a finally block should leak out to the enclosing context. An inference from an actual assignment in the finally block should indeed leak out, though.
|
|
|
|
|
|
This problem is partially caused by the limitations of the flow analysis being non-monotonic
|
|
and our simple path-independent analysis that uses the conservative result of a union
|
|
of the end of the try and the end of the finally, which doesn't distinguish between
|
|
different paths through the try-finally.
|
|
|
|
Proposal:
|
|
Modify the analysis so that inferred branches do not modify the state outside of the
|
|
finally. Thus, the state from the inferred `else` will not propagate out of the `finally`.
|
|
|
|
**Conclusion**
|
|
|
|
We're a little worried about the additional computational complexity of path-dependent
|
|
flow analysis, and if `finally` is the only place it would improve precision, it seems
|
|
to be of limited value. The proposal will fix a rather obvious flaw in the existing
|
|
analysis, is relatively simple, and will probably not make other scenarios significantly
|
|
worse. Accepted.
|
|
|
|
### Implied constraint for a type parameter of a partial?
|
|
|
|
See https://github.com/dotnet/csharplang/issues/2450
|
|
|
|
> What is the implied type parameter constraint in
|
|
|
|
```C#
|
|
#nullable disable
|
|
partial class C<T> { }
|
|
#nullable enable
|
|
partial class C<T> { }
|
|
```
|
|
|
|
> If there were only the first declaration, the constraint would be "oblivious object". If there
|
|
were only the second declaration, the constraint would be "nullable object".
|
|
|
|
> Is this an error?
|
|
|
|
We've been moving code to CoreFX recently and we've hit cases where we can't necessarily move all
|
|
of the partials to use nullable at the same time, so an error could make things much more
|
|
difficult.
|
|
|
|
Proposal: We already have rules about merging two type declarations with differing nullability
|
|
for type inference. The rule about invariants seems to fit well for this situation. The merged
|
|
result will be the same as the one produced for type inference.
|
|
|
|
Q: Will a warning be produced if constraints don't match? Where will it be produced?
|
|
|
|
Q: Will an error be produced if the conflict is two different annotations in enabled code?
|
|
|
|
Q: Would the proposal apply only to partial types? Or to partial methods as well?
|
|
|
|
**Conclusion**
|
|
|
|
We like the proposal to use the invariant matching from type inference and merging. A mismatch
|
|
between two non-oblivious candidates produces an error. No warnings are produced.
|
|
|
|
For partial methods, the constraints must match and we produce the same warnings/errors as we would with mismatched parameter types.
|
|
For the result, we use the implementation signature inside the implementation, and the
|
|
declaration signature for the callers.
|
|
|
|
### Target-typed switch expression
|
|
|
|
It seems like we would like this to work:
|
|
|
|
```C#
|
|
byte M(bool b) => b switch { false => 0, true => 1 };
|
|
```
|
|
|
|
Proposal:
|
|
|
|
1. If there is a common type, that is the natural type of the switch expression.
|
|
2. There is also a new implicit conversion from expression, to type `T` if every
|
|
expression type in the arms can convert to `T`.
|
|
|
|
|
|
Unfortunately, we cannot do the same thing for the conditional expression:
|
|
|
|
```C#
|
|
void M(byte b) { }
|
|
void M(long l) { }
|
|
|
|
void Test(bool b) => M(b ? 1 : 0);
|
|
// The above calls M(long l). If we use the above proposal for ?:
|
|
// then it will call M(byte b) because byte
|
|
// is a "better" overload than long
|
|
```
|
|
|
|
However, we could do a modification: the target typing exists only when the
|
|
conditional expression does not have a natural type.
|
|
|
|
**Conclusion**
|
|
|
|
Proposal accepted for the switch expression; do it before shipping C# 8.0. We also like the
|
|
modification to the conditional expression. We would like to do it in C# 8.0 if possible, but if
|
|
not we will not tie the switch expression changes to the conditional expression changes.
|
|
|
|
### DefaultCancellationAttribute and overriding/hiding/interface implementation
|
|
|
|
Proposal: produce a warning if the attribute is placed anywhere aside from a parameter with type
|
|
CancellationToken on an async iterator method. That means that abstract methods would have a
|
|
warning.
|
|
|
|
**Conclusion**
|
|
|
|
Accepted.
|
|
|
|
Also,
|
|
|
|
- We will not warn on applying the attribute to multiple CancellationToken parameters
|
|
inside an async iterator.
|
|
|
|
- We *will* warn if there is at least one CancellationToken parameter
|
|
to an async iterator and none of the parameters have the attribute applied. |