61 lines
3.8 KiB
Markdown
61 lines
3.8 KiB
Markdown
|
# C# Language Design Meeting for November 4th, 2020
|
||
|
|
|
||
|
|
## Agenda
|
||
|
|
|
||
|
|
1. [Nullable parameter defaults](#nullable-parameter-defaults)
|
||
|
|
2. [Argument state after call for AllowNull parameters](#argument-state-after-call-for-allownull-parameters)
|
||
|
|
|
||
|
|
## Quote of the Day
|
||
|
|
|
||
|
|
No particularly amusing quotes were said during this meeting, sorry.
|
||
|
|
|
||
|
|
## Discussion
|
||
|
|
|
||
|
|
### Nullable parameter defaults
|
||
|
|
|
||
|
|
https://github.com/dotnet/csharplang/pull/4101
|
||
|
|
|
||
|
|
We started today by examining some declaration cases around nullable that we special cased in our our initial implementation, but
|
||
|
|
felt that we should re-examine in light of `T?`. In particular, today we do not warn when you create a method signature that assigns
|
||
|
|
`default` to `T` as a default value. This means that it's possible for generic substitution to cause bad method signatures to be
|
||
|
|
created, where a `null` is assigned to a non-nullable reference type. The proposal, then, is to start warning about these cases, in
|
||
|
|
both C# 8 and 9. In C# 8, the workaround is to use `AllowNull` on that parameter, and C# 9 would allow `T?` for that parameter. There
|
||
|
|
was no pushback to this proposal.
|
||
|
|
|
||
|
|
As part of this, we also considered the other locations in this example. For example, we could issue a warning at the callsite of such
|
||
|
|
a method. The proposal would be to expand the warnings on nullable mismatches in parameters to implicit parameters as well. This could
|
||
|
|
end up causing double warning, if both the method and the callsite get a warning here, but it might be able to help users who are using
|
||
|
|
otherwise unannotated methods, or libraries compiled with an older version of the compiler that did not warn here. There is some
|
||
|
|
concern, though, that putting a warning at the callsite is the wrong location. It was the method author that created this invalid
|
||
|
|
signature, and we'd be punishing users with additional warnings. Presumably, if the author allows `null`, they're appropriately
|
||
|
|
handling it, even if the code is still oblivious or in an older version of C#.
|
||
|
|
|
||
|
|
#### Conclusion
|
||
|
|
|
||
|
|
The original proposal is approved. We'll continue looking at the callsite proposal as an orthogonal feature and come back when we have
|
||
|
|
a complete proposal to review.
|
||
|
|
|
||
|
|
### Argument state after call for AllowNull parameters
|
||
|
|
|
||
|
|
https://github.com/dotnet/csharplang/discussions/4102
|
||
|
|
https://github.com/dotnet/roslyn/issues/48605
|
||
|
|
|
||
|
|
Next, we looked at fallout over a previous decision to update the state of variables passed as parameters to a method. This allowed us
|
||
|
|
to bring the behavior of `void M([NotNull] string s)` and `void M(string s)` in line, which caused issues for the BCL (as it meant
|
||
|
|
that any change to add `NotNull` to a parameter would be a good change to make, and they were not interested in updating thousands of
|
||
|
|
methods to do this). However, it caused an unfortunate side effect: `void M([AllowNull] string s)` would have no warnings, and would
|
||
|
|
silently update the parameter state to not null, even though there was absolutely no way `M` could have affected the input argument as
|
||
|
|
it was not passed by ref. We considered 2 arguments for this:
|
||
|
|
|
||
|
|
1. Perhaps the method isn't annotated correctly? The real-world example here is `JsonConvert.WriteJson`, and there is an argument to be
|
||
|
|
made that in C# 9, this parameter would just be declared as `T?`, solving the issue. However, it does feel somewhat obvious that this
|
||
|
|
method shouldn't update the state of the parameter.
|
||
|
|
2. We loosen the "effective" resulting type of the parameter based on the precondition, if the parameter is by-value. `[AllowNull]` would
|
||
|
|
loosen the effective resulting type to `T?`, which would not make any changes to the current state of the argument. We might also do the
|
||
|
|
inverse for `DisallowNull`.
|
||
|
|
|
||
|
|
#### Conclusion
|
||
|
|
|
||
|
|
We ran out of time today, but are interested in approach 2 above. We'll come back with a complete proposal for a future LDM and examine it
|
||
|
|
again.
|