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

Redirecting factory tear-off type error. #53837

Open
lrhn opened this issue Oct 24, 2023 · 5 comments
Open

Redirecting factory tear-off type error. #53837

lrhn opened this issue Oct 24, 2023 · 5 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cfe-encodings Encoding related CFE issues. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js

Comments

@lrhn
Copy link
Member

lrhn commented Oct 24, 2023

If a redirecting factory constructor redirects to a constructor with a wider parameter type for an optional parameter, and which has a default value in that wider parameter type, then the constructor tear-off seems to get that default value as its own default value, and when it gets type-checked, that becomes an error.

It should likely not be type-checked.

Example:

class C {
  factory C([int x]) = D;
}
class D implements C {
  D([int? x = null]);
}
void main() {
  var f = C.new;
  f();
}

Here Dart2JS (dartpad.dev, master branch) prints:

Uncaught Error: TypeError: null: type 'JSNull' is not a subtype of type 'int'

VM has no issue.

This may be due to the tear-off lowering introduced by the front-end. The result of that lowering is not a valid Dart function declaration, and must therefore be treated specially by the compiler (by not checking the default value against the parameter type, which is safe because it's definitely forwarding to a constructor which will accept the value).


It is very likely also be an issue in the constructor-tear-off specification, where it says that the tear-off is equivalent to tearing off a static function with the same parameters and default values. That does not account for forwarding factory constructors not having default values of their own, and their target not necessarily having a default value mathcing the parameter type.
Even if the spec has an issue, the desired behavior is that of the VM, which means that tearing off a forwarding factory constrcuctor cannot be expressed as a pure desugaring to tearing off a static function.

That is, use the following instead:

A constructor of a non-generic class C with parameter list signature P has a corresponding function signature of C Function(P).
A constructor for a generic class C with type parameters TS and parameter list signature P has a corresponding function signature of C Function<TS>(P).
Tearing off a constructor creates a function value with the same function signature as the constructor's corresponding function signature.
Invoking the torn-off function, with type arguments if applicable, and an argument list, returns the same value as invoking the original constructor on the class type, instantiated with those type arguments if applicable, and with the same argument list.

(And we likely need to make sure that the implicit generic function instantiation coercion is also defined in terms of forwarding the argument list, not copying default values, since it applies to the constructor tear-off of generic classes, including forwarding factory tear-offs.)

We really should get out of the habit of defining things in terms of desugarings. It keeps having unintended side-effects.

@lrhn lrhn added web-dart2js type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. labels Oct 24, 2023
@johnniwinther johnniwinther added the cfe-encodings Encoding related CFE issues. label Oct 24, 2023
@eernstg
Copy link
Member

eernstg commented Oct 25, 2023

See also dart-lang/language#3427.

I think we might be better off by just dropping the forwarder and evaluating C.new directly to D.new, but keeping the static type unchanged as C Function([int x]).

@lrhn
Copy link
Member Author

lrhn commented Oct 26, 2023

I disagree with providing direct access to a potentially private constructor without limiting the possible arguments.

@eernstg
Copy link
Member

eernstg commented Oct 26, 2023

That is a very strong argument, and I'd like to have that encapsulation as well. However, we might need some new language concepts in order to be able to support this.

@lrhn
Copy link
Member Author

lrhn commented Oct 26, 2023

We may only need to allow the synthetic tear-off function is allowed to have a default value which is not a subtype of the parameter type. It does mean that we can't use the normal "binding arguments to formals" algorithm when calling it.

We could directly say what its behavior is, when called with an argument list. Which we need to do anyway, if it can't be a desugaring to Dart code.
That could be to invoke the target constructor with the same arguments where provided, and with the default value for parameters with no argument, default values that may be of a type that isn't a subtype of the parameter type of this constructor, but is allowed by the target constructor. The synthetic function wouldn't have to create any internal local variable to do it, and therefore the incompatibly typed parameter default value is not an issue. We could even say that there is an internal variable, but it's typed as the (transitive) target constructor's parameter type.
(If we allowed parameter variables with wider internal type than the external parameter type, dart-lang/language#1311, then there would be a possible desugaring..)

As long as it means that A.foo(args) and (A.foo)(args) behave the same, and var f = A.foo; doesn't leak the target's, potentially more permissive, parameter singature, any way to specify it is fine.
Except desugaring to Dart code, which cannot satisfy both.

@eernstg
Copy link
Member

eernstg commented Oct 26, 2023

we can't use the normal "binding arguments to formals" algorithm ...
We could directly say what its behavior is, when called with an argument list. Which we need to do anyway, if it can't be a desugaring to Dart code.
... any way to specify it is fine. Except desugaring to Dart code, which cannot satisfy both.

Exactly: We need to take the new (currently unsupported) semantics into account as a full-fledged part of the language (that is, it must be well-defined and well-understood).

We may then use this new semantics in just the situations where we must use it (that is, we may not provide access to the new semantics using any syntax that developers can write), and then in the future we may still choose to generalize the mechanism and provide access to it using some syntactic form.

To me it sounds like the core of a function forwarding semantics.

@sigmundch sigmundch added P2 A bug or feature request we're likely to work on area-front-end Use area-front-end for front end / CFE / kernel format related issues. and removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels Nov 3, 2023
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. area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cfe-encodings Encoding related CFE issues. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js
Projects
None yet
Development

No branches or pull requests

4 participants