Skip to content

Possible Bug: A Way Is Needed To Handle Nullable Extension Types In Switch Statements #55104

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

Closed
mcmah309 opened this issue Mar 5, 2024 · 8 comments

Comments

@mcmah309
Copy link

mcmah309 commented Mar 5, 2024

There needs to be a way to handle nullable extension types in switch statements.

One may expect something like this to succeed.

extension type const Option<T>._(T? v) {}

extension type const Some<T>._(T v) implements Option<T> {
  const Some(T v) : this._(v);
}

extension type const None._(Null _) implements Option<Never> {
  const None() : this._(null);
}


void main() {
  Option<int> y = Some(1);
  final int x;
  switch (y) {
    case Some(:final v):
      x = v;
    case None():
      return;
  }
  int z = x; // issue
}

But it gives

The final variable 'x' can't be read because it's potentially unassigned at this point.
Ensure that it is assigned on necessary execution paths.

I believe this is because the after compilation it looks like this

void main() {
  int? y = 1;
  final int x;
  switch (y) {
    case int()?:
      x = y; // this doesn't give a compiler error or runtime error for some reason?
    case null:
      return;
  }
  int z = x; // issue
}

Rather than the intended

void main() {
  int? y = 1;
  final int x;
  switch (y) {
    case int():
      x = y;
    case null:
      return;
  }
  int z = x; // not issue
}

I would personally classify this as bug, since a case statement such as Some<int>(:final v) would never be intended to compile as int? from a developer standpoint, as far as I can think. it should be the extension types type (int), not the super type of the extension types type. But maybe there is a rational for this.

Alas, given with a regular nullable type you can do

void main() {
  int? y = 1;
  final int x;
  switch (y) {
    case int():
      x = y;
    case null:
      return;
  }
  int z = x; // no issue
}

And also with a class case

sealed class Option<T> {
}

class Some<T> extends Option<T> {
  final T v;
  Some(this.v);
}

class None<T> extends Option<T> {
  None();
}


void main() {
  Option<int> y = Some(1);
  final int x;
  switch (y) {
    case Some(:final v):
      x = v;
    case None():
      return;
  }
  int z = x; // no issue
}

If this in not a bug, there needs to be way to specify that you want the non-nullable case of an extension type. Something like

void main() {
  Option<int> w = Some(1);
  int p;
  switch (w) {
    case Some(:final v)!:
      p = v;
    case None():
      return;
  }
  int z = p;
}
@mcmah309 mcmah309 changed the title A Way Is Needed To Handle Nullable Extension Types In Switch Statements Possible Bug: A Way Is Needed To Handle Nullable Extension Types In Switch Statements Mar 5, 2024
@eernstg
Copy link
Member

eernstg commented Mar 5, 2024

I'll respond to this in a few steps (there are many examples).

For the first example, the error occurs not because Option<int> is a potentially nullable type, but because the switch statement isn't exhaustive. Hence, we can't know whether any of the cases will be executed, hence x might still be uninitialized, and then it's an error to use it. The immediate fix is to make the switch statement exhaustive, e.g., by adding a catch-all case:

    default:
      throw "Unreachable";

@eernstg
Copy link
Member

eernstg commented Mar 5, 2024

I believe this is because the after compilation it looks like this

void main() {
  int? y = 1;
  final int x;
  switch (y) {
    case int()?:
      x = y; // this doesn't give a compiler error or runtime error for some reason?
    case null:
      return;
  }
  int z = x; // issue
}

No, that's not quite true. Of course "after compilation" could mean many different things, but let's just focus on the fact that the compilation step will erase each extension type to its corresponding representation type. For instance, Some<int> is erased to int, not int?.

So we're introducing a final local variable named v whose type is int and whose value is the representation object of y, which is indeed the same object as y.

None is erased to Null, so case null: is an OK description of the translation of the second case.

The reason why there is no compile-time error at x = v is that the static type of v is int and so is the declared type of x, and there is no conflict with other (prior) locations where a value was assigned to x (it would be a compile-time error if it could be assigned a value multiple times because it's final).

@eernstg
Copy link
Member

eernstg commented Mar 5, 2024

Alas, given with a regular nullable type you can do

void main() {
  int? y = 1;
  final int x;
  switch (y) {
    case int():
      x = y;
    case null:
      return;
  }
  int z = x; // no issue
}

Right, in this case the flow analysis does actually recognize that the switch statement is exhaustive. Since the switch statement is known to be exhaustive, it is also known that x is definitely assigned after the switch statement, and hence there is no error at int z = x;.

We do get a diagnostic message anyway, because int? y = 1; will immediately cause the type of y to be promoted to int (rather than int?), because the initializing expression is non-nullable. If you want to avoid this promotion (and the warning that it causes) then you can use int? y = 1 as dynamic;.

However, the extension types aren't always-exhaustive, and hence we don't perform the exhaustiveness analysis on the switch statement on the extension types, and hence we don't discover that it is actually exhaustive.

We can force the exhaustiveness analysis to be performed by using a switch expression rather than a switch statement:

extension type const Option<T>._(T? v) {}

extension type const Some<T>._(T v) implements Option<T> {
  const Some(T v) : this._(v);
}

extension type const None._(Null _) implements Option<Never> {
  const None() : this._(null);
}

void main() {
  {
    Option<int> y = Some(1);
    final int x = switch (y) {
      Some(:final v) => v,
      None() => -1,
    };
    int z = x; // No issue.
  }
  {
    int? y = 1 as dynamic;
    final int x = switch (y) {
      int() => y,
      null => -1,
    };
    int z = x; // No issue.
  }
}

The exhaustiveness analysis will use the erasure of extension types (of the scrutinee as well as of any object patterns), and this means that the extension type version gets essentially exactly the same treatment as the non-extension type version, and they are then both recognized as exhaustive.

@eernstg
Copy link
Member

eernstg commented Mar 5, 2024

If this in not a bug

It may be surprising that the switch statement isn't recognized as being exhaustive because it isn't required to be exhaustive (so we don't even run that analysis). However, this behavior was chosen because it would be a massively breaking change to require all switch statements to be exhaustive, hence it's completely unlikely that the language will be changed to have that requirement. So I think we'll have to say that this is not a bug, and it won't change.

(... in spite of the fact that several members of the language team, including myself, were at times tempted by the idea that we could require all switch statements to be exhaustive ... ;-)

You might want to prefer switch expressions over switch statements when both are roughly equally convenient, in order to ensure that exhaustiveness is required and the exhaustiveness analysis is performed. Otherwise you might have to introduce a catch-all case (like the default case that I mentioned above).

You can choose to use a catch-all case that asserts the information that you consider to be guaranteed:

void main() {
  Option<int> y = Some(1);
  final int x;
  switch (y) {
    case Some(:final v):
      x = v;
    default:
      assert(y is None);
      return;
  }
  int z = x; // issue
}

So there are several different ways to deal with this lack of recognition of exhaustiveness. As always, YMMV.

there needs to be way to specify that you want the non-nullable case of an extension type

An extension type may or may not "have a non-nullable case". Of course, you can specify a nullable version of a given extension type E by writing E?, and that's a type which is nullable in the usual sense.

But if an extension type (like Option<int>) has a representation type which is nullable (like int?) then we can have a situation where an expression of that type has the value null, even though the static type is that extension type (Option<int>) which does not immediately reveal that it includes null.

In any case, you can always check for null using the pattern you mentioned (that would be a null assertion pattern that contains an object pattern):

void main() {
  Option<int> w = Some(1);
  int p;
  switch (w) {
    case Some(:final v)!:
      p = v;
    case None():
      return;
  }
  int z = p;
}

However, this, I'd assume, does not work in the way you intended: It means that we're performing the null-assert pattern (... !) on the scrutinee w as the very first step. When w is null, this will throw! ... so we're done, and the case None() is reported as unreachable.

You could use a null-check pattern (... ?) rather than the null-assert pattern, but that's redundant because Some(:final v) won't match null anyway.

All in all, I think it's all working as intended. The main point is that one needs to be aware of the special exceptions about switch statements and exhaustiveness, and there are several ways to handle that.

@mcmah309
Copy link
Author

mcmah309 commented Mar 5, 2024

Gotcha, thank you for the detailed response. Can we in the future make extension types exhaustive so exhaustive analysis of the erasure is done? Like

sealed extension type const Option<T>._(T? v) {}

final extension type const Some<T>._(T v) implements Option<T> {
  const Some(T v) : this._(v);
}

final extension type const None._(Null _) implements Option<Never> {
  const None() : this._(null);
}

I wouldn't mind using an switch expression, except in this case where I want to exit the containing function. So an alternative solution to this scenario, would be allow returning from a switch expression. A syntax like this

void funcToExit(Option<int> y){
    final int x = switch (y) {
        Some(:final v) => v,
        None() => {
            // possible other logic
            return; // exit containing function
            },
        };
}

@eernstg
Copy link
Member

eernstg commented Mar 5, 2024

Can we in the future make extension types exhaustive

I guess this would mean that we would include an extension type in the set of always-exhaustive types if and only if its representation type is always-exhaustive. However, this is the behavior that we have already:

sealed class A {}
class B1 extends A {}
class B2 extends A {}

extension type EA(A it) {}
extension type EB1(B1 it) implements EA {}
extension type EB2(B2 it) implements EA {}

String foo() {
  EA ea = EB1(B1());
  final String s;
  switch (ea) {
    case EB1(): s = 'EB1';
    case EB2(): s = 'EB2';
  }
  return s; // No error.
}

This is working as specified (search for 'always-exhaustive' in this section).

I don't think we can support a similar notion for extension types as such (independently of their representation types). We did have some discussions about using class modifiers (such as final or sealed) on extension types, but it was not accepted by the language team. One reason for not supporting sealed is that it is pretty much exclusively about exhaustiveness, and exhaustiveness is relevant only to switches and similar constructs, and they will perform run-time tests to determine whether or not any given scrutinee satisfies each of the patterns, and this will work on the actual object at run time, that is, on an object whose run-time type is some subtype of the representation type. So it all comes back to the representation type, no matter what we do.

allow returning from a switch expression

You might want to vote for this one: dart-lang/language#2025.

I think there was a proposal about block expressions somewhere (that is, expressions of the form { .../*statements*/... return e; }), but I can't find it right now.

@mcmah309
Copy link
Author

mcmah309 commented Mar 5, 2024

You might want to vote for this one: dart-lang/language#2025.

I think there was a proposal about block expressions somewhere (that is, expressions of the form { .../*statements*/... return e; }), but I can't find it right now.

Yeah I'm definitely a fan of that sort of control flow. I even added it as a feature to one of my packages. https://github.com/mcmah309/rust_core/tree/master/lib/src/result#early-return-key-notation

I am good with closing this if you are, given your explanation for what is happening and the rational for not supporting sealed type extensions.

void main() {
  Option<int> y = Some(1);
  final int x;
  switch (y) {
    case Some(:final v):
      x = v;
    default:
      assert(y is None);
      return;
  }
  int z = x;
}

Is a reasonable solution for this situation IMO.

@eernstg
Copy link
Member

eernstg commented Mar 5, 2024

Very good, thanks! I think it's fine to close the issue because it doesn't report on anything that should be fixed.

@mcmah309 mcmah309 closed this as completed Mar 5, 2024
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

2 participants