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

It is OK to declare a const constructor in a class with a mixin #33644

Closed
scheglov opened this issue Jun 27, 2018 · 5 comments
Closed

It is OK to declare a const constructor in a class with a mixin #33644

scheglov opened this issue Jun 27, 2018 · 5 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.

Comments

@scheglov
Copy link
Contributor

This code should not give any errors, according to the current state of the language specification.

class M {
}
class A extends Object with M {
  const A();
}

See 12.1 Mixin Application:
image
M has no fields, so Object with M has a constant default constructor.

@scheglov scheglov added the area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). label Jun 27, 2018
@chloestefantsova chloestefantsova added area-analyzer-cfe and removed area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). labels Jul 23, 2018
@chloestefantsova
Copy link
Contributor

I believe this issue and #33645 have the same cause: fasta doesn't allow mixin applications to have const constructors.

@chloestefantsova chloestefantsova self-assigned this Jul 23, 2018
@chloestefantsova
Copy link
Contributor

/cc @askeksa-google

@chloestefantsova
Copy link
Contributor

I did some research (that is, talked to @eernstg :)), and it seems that it's not ok to declare a const constructor in a mixin application, but a const constructor is implicitly delcared in case there's one in the superclass of the mixin application, and the mixin doesn't have fields.

So I think the example program in the original post of this issue should result in a compile-time error. The following example program is supposed to demonstrate the effect of implicitly declared const and not-const constructors in a mixin application.

class M {}

class S {
  const S.foo();
}

class A extends S with M {
  // The following is implicitly declared:
  //
  //     const A.foo() : super.foo();
}

class N {
  int bar;
}

class B extends S with N {
  // The following is implicitly declared:
  //
  //     B.foo() : super.foo();
}

main() {
  A a = const A.foo();  // Ok.
  B b = const B.foo();  // Compile-time error.
}

@chloestefantsova
Copy link
Contributor

Ok, I had another conversation, and it seems that what I've written above is misleading. The implicitly declared constructors are declared within the mixin application class (S with M and S with N in my example above), not within the class that extends it (A and B in the example above). The class that extends a mixin application is a regular class that may have const constructors in case there are const constructors in its superclass (that is, the mixin application class).

So, the example program in the original post should compile without errors, as rightfully suggested. And the compiler should produce a compile-time error for my example program, because there are no constructors foo declared either in A or B.

Sorry for the inconvenience!

@eernstg
Copy link
Member

eernstg commented Aug 6, 2018

Agreed! (so this example has no error; and the constructors are not implicitly created in A and B, but in their immediate superclasses).

@bwilkerson bwilkerson removed this from In progress in Analyzer/FE Integration Aug 28, 2018
@bwilkerson bwilkerson added area-front-end Use area-front-end for front end / CFE / kernel format related issues. and removed area-analyzer-cfe labels Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.
Projects
None yet
Development

No branches or pull requests

4 participants