Add LDM notes for Aug 28, 2019
This commit is contained in:
parent
2118a41673
commit
fffefd2455
101
meetings/2019/LDM-2019-09-04.md
Normal file
101
meetings/2019/LDM-2019-09-04.md
Normal file
|
@ -0,0 +1,101 @@
|
|||
|
||||
# C# Language Design Meeting for September 4, 2019
|
||||
|
||||
## Agenda
|
||||
|
||||
1. `[AllowNull]` on properties
|
||||
|
||||
## Discussion
|
||||
|
||||
[https://github.com/dotnet/roslyn/issues/37313](https://github.com/dotnet/roslyn/issues/37313)
|
||||
|
||||
### AllowNull and flow analysis
|
||||
|
||||
Some questions have come up concerning `AllowNull` and property analysis, like in the following example:
|
||||
|
||||
```C#
|
||||
class Program
|
||||
{
|
||||
private string _f = string.Empty;
|
||||
|
||||
[AllowNull]
|
||||
public string P
|
||||
{
|
||||
get => _f;
|
||||
set => _f = value ?? string.Empty;
|
||||
}
|
||||
|
||||
static void Main(string[] args)
|
||||
{
|
||||
var p = new Program();
|
||||
p.P = null; // No warning, as expected
|
||||
Console.Write(p.P.Length); // unexpected warning
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The tricky part here is that we are tracking flow state for properties, meaning if `null` is stored in a
|
||||
property, we treat that property as having a null state. Although there is an attribute, the flow state
|
||||
of the variable wins, meaning that we think there is a `null` inside `P`, even though the stated type
|
||||
is `string`. Similarly, if you invert the attribute and use `[NotNull]` on a `string?`, the flow state
|
||||
will win again.
|
||||
|
||||
There a couple ideas to address the problem, including
|
||||
|
||||
1. Don't flow-track non-nullable fields/properties.
|
||||
2. Have `[AllowNull]` suppress both the warning, and suppress setting the flow state to `null`
|
||||
3. Have the `[NotNull]` attribute suppress the flow state (which it currently doesn't), and require
|
||||
the property be written with a nullable type (`string?`) and have `[NotNull]` on the getter.
|
||||
4. Stop tracking when any nullability attributes are present on properties.
|
||||
|
||||
|
||||
2 and 3 are somewhat related and we could do both, in theory. The main problem is that it greatly
|
||||
complicates the user's understanding of when flow tracking is enabled, and there are also potentially
|
||||
downstream affects for type inference if we allow the rule for (2) to apply to parameters.
|
||||
|
||||
For (1) it seems plausible, since we would still have a warning on assigning a null to a non-nullable
|
||||
member. This would remove the flow-based subsequent warnings, but the user would still be warned at
|
||||
the point where the problem happens. However, it doesn't seem to solve the problem for generics, e.g.
|
||||
|
||||
```C#
|
||||
class C<T> where T : new()
|
||||
{
|
||||
private T _f = new T();
|
||||
|
||||
[AllowNull]
|
||||
public T P
|
||||
{
|
||||
get => _f ?? throw new Exception();
|
||||
set
|
||||
{
|
||||
if (value is null)
|
||||
{
|
||||
throw new Exception();
|
||||
}
|
||||
return _f;
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Here the generic property `P` cannot be marked nullable because it is unconstrained.
|
||||
|
||||
However, 2 & 3 also have a problem around `MaybeNull`. If a property is annotated `MaybeNull`, then
|
||||
the attribute would override the state, meaning that a null check is useless. If you check for null
|
||||
it would not matter, because when you read the property again, the attribute would override the state
|
||||
and the result would still be maybe null.
|
||||
|
||||
An idea to address this is a combination of (4) and (3), where we have special attribute behavior
|
||||
for properties, and in that case `NotNull` has precedence over nullable state, but nullable state
|
||||
has precedence over `MaybeNull`. In addition, `AllowNull` modifies the state transition to use
|
||||
the declared state if the input is null, while it otherwise uses the non-null state.
|
||||
|
||||
**Conclusion**
|
||||
|
||||
The proposal starting point is:
|
||||
|
||||
NotNull wins over tracked state, which wins over MaybeNull.
|
||||
AllowNull transforms an incoming maybe null state to the declared state.
|
||||
|
||||
There's an action item to go investigate how this will play into the rest nullability, and an open
|
||||
question of whether to treat fields like properties, or like local variables.
|
|
@ -39,10 +39,6 @@
|
|||
- Triage new proposed [9.0 features](https://github.com/dotnet/csharplang/issues?q=is%3Aopen+is%3Aissue+no%3Amilestone+label%3A%22Proposal+champion%22)
|
||||
- Finish triaging away [8.X milestone](https://github.com/dotnet/csharplang/issues?q=is%3Aopen+is%3Aissue+label%3A%22Proposal+champion%22+milestone%3A%228.X+candidate%22)
|
||||
|
||||
## Sep 4, 2019
|
||||
|
||||
- https://github.com/dotnet/roslyn/issues/37313 (Rikki, Phillip)
|
||||
|
||||
## Aug 26, 2019
|
||||
|
||||
- Triage language features
|
||||
|
@ -60,6 +56,12 @@
|
|||
|
||||
Overview of meetings and agendas for 2019
|
||||
|
||||
## Sep 4, 2019
|
||||
|
||||
[C# Language Design Notes for Sep 04, 2019](LDM-2019-09-04.md)
|
||||
|
||||
1. AllowNull on properties https://github.com/dotnet/roslyn/issues/37313
|
||||
|
||||
## Aug 28, 2019
|
||||
|
||||
[C# Language Design Notes for Aug 28, 2019](LDM-2019-08-28.md)
|
||||
|
|
Loading…
Reference in a new issue