Skip to content

Fix Analyzer-CFE integration issue where a mixin extends an abstract class #33678

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
scheglov opened this issue Jun 28, 2018 · 6 comments
Closed
Assignees

Comments

@scheglov
Copy link
Contributor

No description provided.

@scheglov scheglov changed the title CFE does not support super-mixins yet. CFE does not support super mixins yet. Jun 28, 2018
@chloestefantsova chloestefantsova self-assigned this Jul 23, 2018
@chloestefantsova
Copy link
Contributor

@scheglov Could you please provide more details? I've written the following example program and it compiles under CFE and its behavior is what I would expect it to be.

class M {
  void foo() {
    print("M.foo");
  }
}

class N extends M {
  void foo() {
    super.foo();
  }
}

class B {
  void foo() {
    print("B.foo");
  }
}

class C extends B with N {}

main() {
  C c = new C();
  c.foo();
}

When run in the current master branch, it prints out the following:

$ out/ReleaseX64/dart /tmp/example.dart
B.foo

@scheglov
Copy link
Contributor Author

I'm sorry, it was too global statement.
What I should have said is that a test fails with CFE.
I don't fully aware about all details of super-mixin semantics.
@bwilkerson added this test in https://codereview.chromium.org/2500143002

@chloestefantsova chloestefantsova changed the title CFE does not support super mixins yet. CFE does not allow mixins that extend abstract classes Jul 24, 2018
@chloestefantsova
Copy link
Contributor

Thanks for the clarification, Konstantin! I think this is the part of the spec that the test is checking: link. I've renamed the issue accordingly.

Also, here's the discussed test for convenience:

abstract class A {
  void test();
}

class B extends A {
  void test() {
    super.test;
  }
}

It should compile without warnings or errors when the super-mixin feature is enabled. Currently CFE produces a compile-time error.

@chloestefantsova
Copy link
Contributor

I did some research and it seems that the discussed example works in CFE if B is made abstract and enableSuperMixins is set to true. The former is reported by @bwilkerson in #33951, and I have a fix in CL 66380 for the latter.

@chloestefantsova
Copy link
Contributor

Blocked on #33951.

@chloestefantsova chloestefantsova added the status-blocked Blocked from making progress by another (referenced) issue label Jul 24, 2018
dart-bot pushed a commit that referenced this issue Jul 24, 2018
Partially addresses #33678.

Bug: http://dartbug.com/33678
Change-Id: Id2a5db02899e113129dc6654d262623c48201415
Reviewed-on: https://dart-review.googlesource.com/66380
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Dmitry Stefantsov <[email protected]>
@chloestefantsova chloestefantsova removed the status-blocked Blocked from making progress by another (referenced) issue label Jul 25, 2018
@chloestefantsova chloestefantsova changed the title CFE does not allow mixins that extend abstract classes Fix Analyzer-CFE integration issue where a mixin extends an abstract class Jul 25, 2018
@chloestefantsova
Copy link
Contributor

As per Leaf's comment on #33951, I believe there's nothing more to be done here after CL 66380.

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