Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nnbd: can we make opt-out null safety == disabled null safety? #807

Closed
sigmundch opened this issue Jan 30, 2020 · 6 comments
Closed

nnbd: can we make opt-out null safety == disabled null safety? #807

sigmundch opened this issue Jan 30, 2020 · 6 comments

Comments

@sigmundch
Copy link
Member

I'm starting to be concerned of the additional complexity from having to track 3 states on our implementations: whether nnbd is enabled, opt-out, or opt-in.

We have a very detailed specification of opt-out and opt-in, but no specification what it means to mix nnbd disabled code with opt-in code or opt-out code. We could try to specify it, but I wonder whether it is worth it. Instead, it might be simpler if we can equate opt-out and disabled.

This means releasing upfront a few breaking changes from null safety (hopefully small). For example:

  • normalization: Without nullability, this means backends only break cases where we use structural subtyping and the types contain unnormalized FutureOr terms. Front-ends may also be affected on interesection types (to the extend that they are supported)
  • mutual subtyping of bounds in generic function type parameters: doing this together with normalization will hide cases where normalization was breaking (e.g. comparing two type bounds Object vs FutureOr<Object>), but may change the behavior of other subtype checks.

I'd love to hear your thoughts.

/cc @fishythefish @johnniwinther

@fishythefish
Copy link
Member

Based on https://github.com/dart-lang/language/pull/805/files#r373136879, we can update the semantics of generic function subtyping for everyone rather than track additional state. This is a breaking change, but I'd be surprised if anyone were actually hitting this corner case, and it shouldn't introduce any unsoundness.

Normalization is likewise slightly breaking - subtyping shouldn't be affected given the change to generic function subtyping, but eager normalization means that error messages, etc. may change if they previously contained a non-normalized type. The normalization document only specifies an NNBD version of normalization, but it should be straightforward to implement something equivalent for legacy - we can omit the cases for T? and T* and adjust the cases involving top/bottom types.

@lrhn
Copy link
Member

lrhn commented Jan 31, 2020

As I see it, and correct me if I'm wrong, enabling NNBD is a whole-program operation.
If you pass the --enable-experiment=non-nullable flag, then the entire program is in NNBD-released mode, which means that libraries are either opt-in (null safety) or opt-out (legacy) mode, where the legacy mode is close to the current behavior, but it lives in the NNBD type system.

Without the experiment flag, we use the current semantics and no NNBD types.

So, there should be no interaction between NNBD-on code and NNBD-off code, only between legacy and null-safe code when NNBD is on.

@johnniwinther
Copy link
Member

@lrhn: This distinction doesn't work in practice. When compiling the platform dill with support for NNBD, the NNBD types will be part of the generated AST. This means that all computation must take the existence of NNBD types into account and the only specified way of dealing with such types outside of opt-in semantics is the opt-out semantics of weak mode.

@leafpetersen
Copy link
Member

I think Siggi is concerned about the fact that using normalization and mutual subtyping in opted out code is technically a breaking change. <T extends FutureOr<dynamic>>() => null as void Function<T extends dynamic>() currently fails and will succeed in NNBD subtyping, so either we use different subtyping (and possibly different normalization) in opted in and opted out code, or we allow this small technical breaking change. I think the former is probably madness, and I'm not concerned about the latter, so SGTM.

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Feb 12, 2020
For dart-lang/language#807

Change-Id: Ife86fa35fda20347437391d742a4432518d7898a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135350
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@natebosch
Copy link
Member

Is there more to do here?

@sigmundch
Copy link
Member Author

I think this is pretty much done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants