csharplang/meetings/2017/LDM-2017-10-04.md
2017-10-05 20:15:39 -07:00

6.8 KiB

C# Language Design Review, Oct 4, 2017

Quote of the Day:

"You don't get to use this with your grandfather's Oldsmobile"

Agenda

We looked at nullable reference types with the reviewers, Anders Hejlsberg and Kevin Pilch.

  1. Overall philosophy
  2. Switches
  3. Libraries
  4. Dotted names
  5. Type narrowing
  6. The dammit operator
  7. Array covariance
  8. Null warnings
  9. Special methods
  10. Conclusion

Overall philosophy

Think of this feature as a linting tool, an analyzer. It will help you find many bugs, but it will not guarantee anything. It is important that it does not lead to too much inconvenience, and does not yell too much over existing code. Too much whining at programmers makes them turn the feature off. First appearances are everything.

We should not think of the feature as dialable, with multiple switches or settings. We should design our way to the best balance, and stick to it. One switch: On or off.

Don't worry too much about unannotated libraries. Push on the library owners to get annotations, and live with the lack of them until that happens. In many cases the feature will still be useful even on unannotated libraries, because the default of assuming non-null is often going to be correct.

Switches

Have just one on/off switch for the warnings. The annotations should be allowed regardless of whether the warnings are on or off.

New projects should have it on by default, existing projects probably off.

Libraries

We should push to get our libraries upgraded to have annotations. Since it's only about adding attributes, it's possible we can do something with reference assemblies, leaving the actual binaries untouched.

Even if some libraries aren't upgraded, or not at the same pace, it's not a disaster, and we shouldn't hold up the feature waiting for a sufficient amount of libraries to be ready. Instead, use the availability and (hopefully) popularity of the feature to drive libraries to annotate.

Dotted names

The flow analysis should track dotted names. We have experience from TypeScript, and customers there would definitely complain if we didn't. We also know that it is common in existing code bases to check a dotted name (e.g. a property) for null, then dereference it. In particular, of course, "dotted" names with an implicit this. are common.

It would be a disaster for existing code not to track the null state of dotted names.

If we do, what does it take to invalidate assumptions about a dotted name? In principle, any intervening call can cause a property to change. Even another thread could do that!

But assuming the worst on this is just going to lead to a lot of pain. We have to be lenient, and assume that dotted names remain the same unless something in the dotted chain is assigned to, or maybe passed by out or ref.

There's going to be a type of subtle bug that we won't catch as a result of this lenience. But the price of catching it is too high in most cases, in terms of the amount of perfectly safe existing code that it would flag.

Type narrowing

We are currently tracking nullness of variables separately from the type system. Even when a nullable variable is known not to be null at a given place in the source code, it's type is still nullable, and that's what we feed into e.g. type inference around it.

This allows us to handle things that are not necessarily nullable, such as type parameters. However, when we do know the type is nullable and not null, it feels like we're throwing away useful information not to narrow the type.

We should consider a hybrid, where we narrow the type to non-nullable when we can.

Similarly for use of the ! (dammit) operator. Yes it should silence warnings on conversions and dereferencing. But it should also narrow the type of the expression to non-nullable when possible.

The dammit operator

TypeScript also has this, and it is often a nuisance that it doesn't "stick" - it applies only locally to the expression it is used on. It might be nice with some sort of assertion that sticks throughout the scope of the variable. We should consider it.

It's a little suspicious to use the same operator for silencing warnings and narrowing the type, but probably better than having two different ones. Casts aren't good for suppressing warnings, because they also imply a runtime check (which we may not want, since the suppression of the warning may be because you want to allow a null).

Array covariance

We currently treat arrays as invariant with respect to element nullability.

arrayOfNullable = arrayOfNonNullable; // warning

But arrays are covariant, albeit unsafely. We should consider applying the same covariance to nullability, for consistency. I.e., we would allow the above code without warning. That is even though a null check won't be part of the runtime type check that arrays do on write. The alternative is worse.

Null warnings

we should warn on the majority of cases where a null value makes it into a nonnullable variable. However, there are cases where it simply gets too harsh on existing code.

Places where we should warn:

  • constructors that don't initialize fields of nonnullable reference type
  • converting a null literal to a nonnullable reference type
  • passing or assigning a nullable reference value to a non-nullable reference type
  • default(T) expressions when T is a nonnullable reference type

The last one could also yield a T? and no warning, but that would just lead to other warnings further down the line. Besides, default isn't used very much on reference types.

On the other hand we should not warn on array creation expressions, even though they create a whole array of forbidden null values:

var array = new string[10];

These are numerous in current code, and very often they are fine. Besides, the code one would have to write instead is quite unappetizing!

We could maybe find special cases, where we could warn, e.g. if a newly created array of nonnullable element type is read from without ever having been written to at all.

Special methods

Some methods have a special relationship with null: if string.IsNullOrEmpty returns false, then the argument was not null. If a TryGet method returns false, then the resulting out parameter may be null.

TypeScript has a notion of user-defined predicates, which are methods that claim to establish membership of a given type for a given value. We may try to think along similar lines, and consider whether methods can somehow convey extra knowledge about nullability.

Conclusion

This is going to be a great feature, and people will love it. Have a list of known holes, and make clear that there's no guarantee.

We're not going to get everything in the world. It's not possible! And even some of the possible stuff is too inconvenient.

There are only so many greenfield projects in this very established world. Don't be discouraged by low adoption in the beginning. These things take time.