-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Forwarding constructors must not have optional parameters #15101
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
Comments
The spec actually doesn't mention optional parameters for the implicit declared constructors of mixin applications. They looks like an oversight in the spec since there is no need for the restriction. cc @gbracha. |
cc @crelier. |
I agree this is unfortunate. That said, the work around is easy, and the behavior is as specified. The error message should be better: at the least, it should say that BaseClass with Mixin does not have a constructor with no arguments because no constructor without optional arguments could be found in BaseClass, and suggest that adding such a constructor would help. Certainly we want to liberalize this eventually - we want to lift the entire host of restrictions on mixins, all of which are limiting or annoying in one way or another. The work around for now would be: class BaseClass { class Class extends BaseClass with Mixin { Or, if one cannot or does not want to tamper with the API of BaseClass, define an intermediate class (probably private) class _IntermediateClass extends BaseClass { class Class extends _IntermediateClass with Mixin { To understand the issue: The constructors for mixin applications are auto-generated. They are intended to initialize any fields in the mixin and forward information to the superclass. As such, a mixin application needs to have constructors corresponding to those of its superclass so it can forward the arguments etc. However, in the interests of simplicity, the spec says that only constructors that have no optional parameters are created in this way. The superclass of Class is an application of Mixin to BaseClass, and that class does not have a constructor at all, because no constructor without optional parameters exists in its superclass. cc @mhausner. |
This comment was originally written by @filiph Thanks! That workaround really is easy, but not immediately obvious (not to me, at Seeing that on the language level, the fix is not easy (and maybe not even Even if the warning/error just links to this bug's URL, it'll be leaps |
This comment was originally written by @paulevans Just experimenting with mixins for the first time, just bumped in to this issue. |
Improved error message suggesting workaround submitted as r37057. |
This comment was originally written by [email protected] i still don't understand this issue. here is a variant i ran into: class A { class B extends A with M { void main() { it says "error:... forwarding constructors must not have optional parameters class B extends A with M {". the following works: class A { class B extends A with M { void main() { no part of the explanation given in post #4 seems to explain what also, i feel like this restriction defeats much of the point of mixins: convenience. if option (b) is deemed so easy and mechanical, why can't the dart compiler do it for us, |
This comment was originally written by @mhausner A forwarding constructor must not have optional parameters because it can't cover the case where no actual parameter is given at the call site. I.e., it can't do "if I was invoked with no parameter, call super(), else call super(x)". It can only do what was explicitly written in "variant 2" above, which is to invoke the super constructor with a value. This may be ok if the default value is null, as it is in the example given in comment 8. But consider the case where a default value for the optional parameter is given in class A. This value must be evaluated in the context of class A, but would also have to be the default value for the optional parameter of B's super class (A with M). This can be particularly hairy if A and "A with M" are in different libraries. |
thanks for the great explanation. that is amazingly subtle. if (A with M) were not publicly exposed anywhere, it might be possible to say "just make super(x: x) refer to A's constructor, not that of (A with M)", but i'm guessing (A with M) is exposed through mirrors or something. actually, this forwarding problem seems to be more general. e.g. |
Just to be clear, our 3 tools disagree here significantly ATM. Here's an example: $ cat test.dart Produces on analyzer $ dartanalyzer test.dart Produces on dart2js: $ dart2js test.dart Produces on the VM: $ dart test.dart I would strongly prefer if we could make it work for dartanalyzer/dart2js/dartVM in a consistent way. @gilad: Should we support it or not? |
The only problem I can see with mixin implicit constructors and optional parameters is that it isn't specified, not that it can't work. Mixins on top of classes with constructors with optional parameters (... phew ...) do work in dart2js. I think the VM should make it work too. |
This comment was originally written by @mhausner lrn@: How does dart2js address the scenario I described in comment 9, especially with regards to default values and libraries? |
This comment was originally written by @lrhn I can't say how it's actually implemented, but I see no inherent problem in having the same compile-time constant value as default value in the implicit mixin-application constructor. Values are not private to libraries, only identifiers. That makes it a specification problem to state the details, but the behavior should be clear - calling without specifying that optional parameter means that it uses the same default value and passes it to the super-constructor, just as if you hadn't supplied it to the super constructor directly. |
As I indicated in comment #4, this and many other restrictions on mixins are undesirable and unnecessary, and should be removed over time. Barring that, I believe we should implement the spec as it stands. That is why we have a spec. The only relevant counter-argument is that if this already works in some implementations, reverting them to comply with the spec might be considered a breaking change. However, this argument holds for every compiler bug. It is a judgement call how to treat this. I for one don't believe this case rises to that level, but that is not my call to make. In particular, the claim that the restriction is unnecessary is irrelevant. As Matthias notes, once the restriction is removed, the behavior wrt to default values may be disappointing to some - but that is an result of our optional parameter rules, regardless of mixins. Basically, the default value in the subclass will always take precedence over the one in the superclass.
|
Issue #17610 has been merged into this issue. cc @johnniwinther. |
Issue #19226 has been merged into this issue. |
/sub |
FWIW, the ability to forward constructors with optional arguments through mixins in this way would be of great value to Flutter's framework. |
I just ran into this again. The inability to use mixins when you use constructors with named arguments is a serious limitation when you make heavy use of named arguments (as Flutter does). In this particular case, I wanted to extract some code to enable code re-use. However, I can't, because the classes involved all have constructors with named arguments. I can't change the base class, because it's a public API and we don't want to have multiple redundant constructors. I can't introduce an intermediate class because requiring that all consumers of this mixin introduce an intermediary is too ugly. Instead, I'm going to have to put the code in the base class and this makes that class harder to use (because it now has a broader API surface, some of which is of no use to most subclasses). We should just change the spec to forward optional and named arguments the same way it works with non-optional positional arguments. |
Is there any active work being done to address this issue? |
I'll see what I can do. |
Just ran into this again. To make finding this bug easier, I'm going to quote the error message from the analyzer that is the symptom of this issue:
|
FWIW, Flutter is currently working around this by having an undesired mixin class in our class hierarchy: |
Following the last comment from @Hixie, here's a link that works over time: |
Just personally ran into this. Not sure what |
In short, because the spec says so. The reason for not forwarding optional parameters is probably that it gets complicated to say how default values are computed. We plan to do so for Dart 2.0, so you get forwarders for all generative constructors, not just those without optional parameters. |
So, are you saying this will get fixed in Dart 2.0? |
That is the plan, yes (ob-caveat: unless something changes, as always). |
@bwilkerson FYI: As of 2.0.0-dev-8.0 this is still an issue, but it looks like the analysis errors around it have changed and were not helpful in diagnosing the problem. The analyzer says: Once I ran it, the VM gave me a better error, which via googling led me here. |
This issue was originally filed by @filiph
What steps will reproduce the problem?
What is the expected output? What do you see instead?
I get a runtime error:
forwarding constructors must not have optional parameters
I would expect this to work without problems, or at least to have better explanation (it is, for example, not clear if the culprit is the base class, the mixin, or the class being extended) and a compile time error.
But really, I find this quite limiting.
What version of the product are you using? On what operating system?
Dart Editor version 1.0.0_r30187 (DEV)
Dart SDK version 1.0.0.3_r30187
Mac OSX 10.8.5
Please provide any additional information below.
===== CODE =====
class Mixin extends Object {
int mixinMember;
}
class BaseClass {
int baseMember;
BaseClass({this.baseMember: -1});
}
class Class extends BaseClass with Mixin {
int member;
Class(this.member);
}
void main() {
var c = new Class(42);
}
===== /CODE =====
Attachment:
mixinoptparameters.dart (249 Bytes)
The text was updated successfully, but these errors were encountered: