Skip to content

Type promotion doesn't work on "is!" #80

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
munificent opened this issue Nov 3, 2018 · 4 comments
Closed

Type promotion doesn't work on "is!" #80

munificent opened this issue Nov 3, 2018 · 4 comments
Labels
request Requests to resolve a particular developer problem

Comments

@munificent
Copy link
Member

From: @jamesderlin (dart-lang/sdk#35019):

The following code is accepted without error:

class Base {}

class Derived extends Base {
  void foo() {}
}

void main() {
  Base d = Derived();
  if (d is Derived) {
    d.foo();
  }
}

However, if I invert the d is Derived check:

  if (d is! Derived) {
  } else {
    d.foo();
  }

then I get an error:

Error: The method 'foo' isn't defined for the class '#lib1::Base'.

(The same thing happens if I try !(d is Derived).)

This seems inconsistent. Couldn't we internally invert is! and the if-else clauses to achieve the same behavior as the first version?

(And if anyone is wondering why I don't just always use the first version, there are cases where one path has much more code than the other, and for readability I prefer to have the simpler block first, e.g.:

if (d is! Derived) {
  // assert(false); // Or run some other failure handler
} else {
  d.foo();
  // Followed by a lot more code.
}

Of course, in that particular case, it be even nicer if:

assert(d is Derived);
d.foo();

worked.)

@rrousselGit
Copy link

In the same fashion, the else is not required:

if (d is! Derived) {
  return; // or throw
}
// d should be inferred as Derived

@jtlapp
Copy link

jtlapp commented Sep 9, 2019

Here are some variations of the is! problem (originally posted to SO):

class Organism {}

class Animal extends Organism {
  void eat(Organism food) {}
  void kill() {}
}

class Predator1 extends Animal {
  void eat(Organism food) {
    assert(food is Animal, "Food must be an animal");
    food.kill(); // error: kill not defined (but maybe okay due to production behavior)
  }
}

class Predator2a extends Animal {
  void eat(Organism food) {
    if (food is! Animal) throw ArgumentError("Food must be an animal");
    food.kill(); // error: kill not defined
  }
}

class Predator2b extends Animal {
  void eat(Organism food) {
    if (!(food is Animal)) throw ArgumentError("Food must be an animal");
    food.kill(); // error: kill not defined
  }
}

class Predator3a extends Animal {
  void eat(Organism food) {
    if (food is! Animal) {
      throw ArgumentError("Food must be an animal");
    } else {
      food.kill(); // error: kill not defined
    }
  }
}

class Predator3b extends Animal {
  void eat(Organism food) {
    if (!(food is Animal)) {
      throw ArgumentError("Food must be an animal");
    } else {
      food.kill(); // error: kill not defined
    }
  }
}

class Predator4a extends Animal {
  void eat(Organism food) {
    if (food is! Animal) return;
    food.kill(); // error: kill not defined
  }
}

class Predator4b extends Animal {
  void eat(Organism food) {
    if (!(food is Animal)) return;
    food.kill(); // error: kill not defined
  }
}

For reference, the following compile fine:

class Predator5 extends Animal {
  void eat(Organism food) {
    if (food is Animal) {
      food.kill(); // compiles fine, but sheesh!
    } else {
      throw ArgumentError("Food must be an animal");
    }
  }
}

class Predator6 extends Animal {
  void eat(Organism food) {
    var prey = food as Animal; // compiles fine, only if i like this exception
    prey.kill(); // compiles fine
  }
}

@leafpetersen
Copy link
Member

I believe that 2a, 3a, and 4a are all planned to work in the NNBD release. 2b, 3b, and 4b are interesting. They may just work out of the box: if not, I think they could be made to work. cc @stereotype441

@leafpetersen
Copy link
Member

All of the examples here are now handled except the assert case, which I don't believe we want to support. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

4 participants