Skip to content

"is" doesn't work in classes #43167

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
bernaferrari opened this issue Aug 24, 2020 · 13 comments
Closed

"is" doesn't work in classes #43167

bernaferrari opened this issue Aug 24, 2020 · 13 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@bernaferrari
Copy link
Contributor

I'm having a very weird error. I couldn't find any other bug report, but it seems big. Basically, cast is broken when you declare the variable in a class.
Reproducible issue:
Tested on DartPad and locally. Possible fix: declare as dynamic. But it is a bad fix.

• Flutter version 1.22.0-2.0.pre.36 at /usr/local/flutter
• Framework revision d30e36ba8c (3 days ago), 2020-08-21 22:36:03 -0400
• Engine revision d495da20d0
• Dart version 2.10.0 (build 2.10.0-49.0.dev)
class A {
  String getMessage() => 'A';
}

class B {
  String getMessage() => 'B';
}

class P {
  int test = 12;

  String getMessage() => 'P';
}

class AB extends P with A, B {}

class TestClass {
  final A a = AB();

  test() {
    if (a is AB) {
      print(a.test); // Fails because it thinks a is A, and not AB.
    }
  }
}
@devoncarew
Copy link
Member

I believe that this is correct - AB is not a subclass of A, it just gets the methods defined in A.

@leafpetersen @lrhn to confirm -

@bernaferrari
Copy link
Contributor Author

Why then this works fine?

void main() {
  A a = AB();
  if (a is AB) {
    print(a.test);
  }
}

@lrhn
Copy link
Member

lrhn commented Aug 24, 2020

AB implements all the interfaces A, B, P, AB and Object (as well as the anonymous mixin application classes P&A and (P&A)&B).

The reason promotion doesn't work is that we don't promote instance variables, only local variables.

So yes, promotion is broken when you declare the variable in a class.

@lrhn lrhn closed this as completed Aug 25, 2020
@lrhn lrhn added the closed-as-intended Closed as the reported issue is expected behavior label Aug 25, 2020
@bernaferrari
Copy link
Contributor Author

I don't understand.. so you don't plan on fixing this? Is there any issue to track this?

@leafpetersen
Copy link
Member

Promotion on instance variables can't be done soundly. This is definitely a pain point, but we haven't found a good solution yet. There are (at least) two open issue tracking requests to try to find sound patterns to allow promotion, with some discussion of the issues.

dart-lang/language#104
dart-lang/language#1167

@Hixie
Copy link
Contributor

Hixie commented Aug 27, 2020

@bernaferrari the problem is that you can't tell if "a" will always return the same type -- consider for example:

  class C extends TestClass {
    bool _flag = false;
    @override
    A get a {
      _flag = !_flag;
      if (flag)
        return AB();
      return A();
    }
  }

...if you call C().test(), it'll crash, because even though the first time you check a you get an AB, the second time, you don't, and so it doesn't have a test().

@bernaferrari
Copy link
Contributor Author

bernaferrari commented Aug 28, 2020

What I was thinking @Hixie was something like Kotlin smart cast, where it allows some impressive and advanced stuff while keeping it relatively simple.
Your sample is good, but if I had if (a is AB), and a were final, that edge case you suggested might never happen?

@leafpetersen
Copy link
Member

@bernaferrari even final isn't enough for soundness unfortunately. See the first issue I linked above for further explanation of why not. I just filed an issue for discussion of a runtime checked version of promoting instance checks here.

@Hixie
Copy link
Contributor

Hixie commented Aug 28, 2020

@bernaferrari as far as I can tell, everything listed under "smart casts" on that page is stuff Dart already does.

@bernaferrari
Copy link
Contributor Author

I just tried to reproduce a similar code sample and it works fine on Kotlin with both interface and open class:

open class A {
    open fun getMessage(): String = "A"
}

interface B {
    fun getMessage(): String = "B"
}

interface C {
    fun test(): String = "C"
}

class AB : A(), B, C {
    override fun getMessage(): String = "AB"
}

class Main {
    private val a: A = AB()

    fun main(): String {
        if (a is AB) {
            return a.test()
        }

        return ""
    }
}

Main().main()

Dart can detect a is AB but Dart doesn't allow to call a.test() which only exists in AB and not in A. Kotlin allows. The easy fix is to redeclare the variable in Dart, like this, but then you start having to deal with unnecessary boilerplate:

  final A a = AB();

  test() {
    if (a is AB) {
      print(a.test()); // Fails because it thinks a is A, and not AB.

      final AB ab = a;
      print(ab.test()); // Works
    }
  }

@leafpetersen
Copy link
Member

I just tried to reproduce a similar code sample and it works fine on Kotlin with both interface and open class:

@bernaferrari That example is not the same code, because properties in Kotlin are sealed by default. If you change your code to add open to the declaration of a, then you get the following error:

Smart cast to 'AB' is impossible, because 'a' is a property that has open or custom getter

Adding non-virtual members to Dart is one of the directions that we could go in to help address this. Note though that this would prevent you from using the class as an interface, which is one of the features of Dart that many people speak positively of.

@bernaferrari
Copy link
Contributor Author

bernaferrari commented Aug 28, 2020

ohhhh D: now I can see the hole 😭. I don't know how to contribute to the discussion. So, I guess.. Thanks! haha
I spent a few months with Typescript, so I'm still "re-learning" Dart limits (and missing custom types and type intersection everywhere) but also enjoying some of the flexibility, like class as an interface, which is kind of weird (still unsure when to use mixin+on vs abstract class+with) but also veeery nice.

@leafpetersen
Copy link
Member

ohhhh D: now I can see the hole 😭. I don't know how to contribute to the discussion. So, I guess.. Thanks! haha
I spent a few months with Typescript, so I'm still "re-learning" Dart limits (and missing custom types and type intersection everywhere) but also enjoying some of the flexibility, like class as an interface, which is kind of weird (still unsure when to use mixin+on vs abstract class+with) but also veeery nice.

No problem, and good to hear feedback. Note that there's some active discussion in the language repository (links I shared above, and others recently filed) on this general topic if you're still interested in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

5 participants