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

Error creating super mixin on class with no default 0 arg ctor #35011

Closed
MichaelRFairhurst opened this issue Oct 31, 2018 · 14 comments
Closed

Error creating super mixin on class with no default 0 arg ctor #35011

MichaelRFairhurst opened this issue Oct 31, 2018 · 14 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Milestone

Comments

@MichaelRFairhurst
Copy link
Contributor

Repro:

class NoDefaultZeroArgCtor {                                                     
  NoDefaultZeroArgCtor(int x);                                                   
                                                                                 
  int foo() => 122;                                                              
}                                                                                
                                                                                 
mixin FooMixin on NoDefaultZeroArgCtor {                                         
  int foo() => super.foo();                                                      
}                                                                                
                                                                                 
class UsesFoo extends NoDefaultZeroArgCtor with FooMixin {                       
  UsesFoo(int x) : super(x);                                                     
}                                                                                
                                                                                 
void main() {                                                                    
  print(new UsesFoo(0).foo());                                                   
} 

via dart:

lib/mixin.dart:1: Warning: Interpreting this as package URI, 'package:test/mixin.dart'.
lib/mixin.dart:1: Error: The superclass, 'NoDefaultZeroArgCtor', has no unnamed constructor that takes no arguments.

via dartanalyzer:

Analyzing lib/mixin.dart...
No issues found!

The latter seems desirable to me, though I can only hazard a guess at if there's a good reason why the former is the required/correct behavior.

@MichaelRFairhurst MichaelRFairhurst added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. front-end-kernel type-question A question about expected behavior or functionality labels Oct 31, 2018
@stereotype441 stereotype441 added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. front-end-kernel type-question A question about expected behavior or functionality labels Oct 31, 2018
@stereotype441
Copy link
Member

Classifying as "area-language" for now since we're not sure whether analyzer or front end behavior is correct. @leafpetersen, @eernstg, @lrhn, can you let us know which behavior is correct, so we can reassign the bug to analyzer or front end as appropriate?

@stereotype441 stereotype441 added this to the Dart2.1 milestone Oct 31, 2018
@stereotype441 stereotype441 added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Oct 31, 2018
@lrhn
Copy link
Member

lrhn commented Nov 1, 2018

The analyzer is correct, and so is the code.

Mixin declarations are not class declarations, and their super-class constraints are not super-classes. Mixin declarations have no constructors, and they do not need to call a super-constructor, so should make no difference which constructors their super-constraint classes have.

I get the same error from both dart2js and the VM, so it's most likely a front-end issue.
(It may be that both back-ends have implemented the same error with the same error message, but it's less likely than it being a front-end error).

If I change the code to:

class X {}
class NoDefaultZeroArgCtor implements X {                                                     
  NoDefaultZeroArgCtor(int x);                                                   
...
}                                                                                
                                                                                 
mixin FooMixin on NoDefaultZeroArgCtor, X {    
...

then the program runs. It seems that the front-end is introducing an abstract interface in the case where there are more than one super-constraint, and that interface does have an unnamed nullary constructor, so their hack works. It should work in the one-constraint case too.

@lrhn lrhn added area-front-end Use area-front-end for front end / CFE / kernel format related issues. and removed area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Nov 1, 2018
@stereotype441
Copy link
Member

@kmillikin

@kmillikin
Copy link

kmillikin commented Nov 1, 2018

The corresponding code with old-style super mixins would have had:

class FooMixin extends NoDefaultZeroArgCtor {                                         
  int foo() => super.foo();                                                      
}

which would have signaled the same error; and there was no way to repair it by adding a constructor to FooMixin because then it couldn't be used as a mixin.

So this is a problem affecting new code using mixin declarations, but not for porting existing code using old-style super mixins because it wouldn't have worked in the first place.

I'm testing a fix.

@kwalrath
Copy link
Contributor

I'm running into this bug with the 2.1.0 candidate.

/cc @JekCharlsonYu @dgrove

@kwalrath kwalrath reopened this Nov 15, 2018
@kwalrath
Copy link
Contributor

See https://travis-ci.org/dart-lang/site-www/builds/455227237. I've got a smaller repro, if you want it.

@kwalrath
Copy link
Contributor

Here's the smaller repro (passes analysis but fails to run unless you swap in the commented-out lines):

mixin Musical {
  bool canPlayPiano = false;
  bool canCompose = false;
  bool canConduct = false;

  void entertainMe() {
    if (canPlayPiano) {
      print('Playing piano');
    } else if (canConduct) {
      print('Waving hands');
    } else {
      print('Humming to self');
    }
  }
}

abstract class Performer {
  Performer(String name);
  String name;
}

class Musician extends Performer with Musical {
//  Musician.withName(String name) : super(name);
//  Musician() : super('Anonymous');
  Musician(String name) : super(name);
}


mixin MusicalPerformer on Musician {
  bool canDance = true;

  @override
  void entertainMe() =>
      canDance ? dance() : super.entertainMe();

  void dance() => print('Dancing');
}

class SingerDancer extends Musician with MusicalPerformer {
//  SingerDancer(String name) : super.withName(name);
  SingerDancer(String name) : super(name);
}


void main() {
//  var musician = Musician.withName('Kathy');
  var musician = Musician('Kathy'); // Musician.withName('Kathy');
  musician.canPlayPiano = true;
  musician.entertainMe(); // Playing piano

  var singerDancer = SingerDancer('Todd');
  singerDancer.entertainMe(); // Dancing
}

@kwalrath
Copy link
Contributor

I suppose @MichaelRFairhurst's repro is better/shorter. :)

kwalrath added a commit to dart-lang/site-www that referenced this issue Nov 15, 2018
kwalrath added a commit to dart-lang/site-www that referenced this issue Nov 15, 2018
* 2.1-ify the source code
* Add int-to-double words; add/update int & double code
* Say that dart:async import isn't always required
* Update mixin description for 2.1
  * Workaround for 2.1.0 mixin bug: dart-lang/sdk#35011
@kmillikin
Copy link

This is fixed but the fix somehow missed the final 2.1 dev release. The fix landed before the last push to dev, so I assumed that it had made it in and didn't request a merge to dev. I'm sorry about that.

So this is now an (extremely) annoying bug in the new 2.1 feature that will be fixed in the next release.

@athomas
Copy link
Member

athomas commented Dec 13, 2018

@kmillikin if this is supposed to go into the 2.1.1 stable release, please file a merge-to-stable request.

@lukepighetti
Copy link

lukepighetti commented Feb 18, 2019

Not sure if this is helpful, but here is an example that fails on 2.1.0

class Cook = Person with AwesomeCookingSkills;

class Person {
  final String name;

  Person(this.name);
}

mixin AwesomeCookingSkills on Person {
  String get bestDish => "$name makes the best chocolate.";
}

main() {
  final cook = Cook("Johnny Appleseed");
  /// Error: The superclass, 'Person', has no unnamed constructor that takes no arguments.
  
  print(cook.bestDish);
}

And here is a Gist if it's useful for tracking this https://dartpad.dartlang.org/b4f766621b01ffa5270e5de93ef592b5

@lukepighetti
Copy link

Did this miss 2.1.1 or is it just not in the changelog? https://github.com/dart-lang/sdk/blob/master/CHANGELOG.md#211---2019-02-18

@kmillikin
Copy link

It should be in 2.1.1.

@athomas
Copy link
Member

athomas commented Feb 20, 2019

ad2db25 is in 2.1.1:

$ git tag --contains ad2db25
2.1.1

@athomas athomas closed this as completed Feb 20, 2019
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. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

7 participants