Skip to content

[CFE] Provide a normal body for redirecting factory constructors #41915

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
alexmarkov opened this issue May 15, 2020 · 3 comments
Closed

[CFE] Provide a normal body for redirecting factory constructors #41915

alexmarkov opened this issue May 15, 2020 · 3 comments
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@alexmarkov
Copy link
Contributor

Currently redirecting factory constructors are encoded in a special way in the kernel AST:

    static field dynamic _redirecting# = <dynamic>[tes::A::foo];
    static factory icon() → tes::A*
      let dynamic #redirecting_factory = tes::B::• in invalid-expression;

This body contains a tear-off of a target constructor and invalid expression. In this form it cannot be handled as any other factory by back-ends so it is skipped or handled specially.

It would be useful for back-ends if CFE would actually provide a normal body for redirecting factory constructors: with a StaticCall to a target factory or a ConstructorInvocation of a target generative constructor.

This would help to avoid special casing redirecting factory constructors in certain places. We can also unskip them in the VM and support calling them from Dart C API and mirrors (#33041).

Related: #28424, #28421.

/cc @johnniwinther @mkustermann

@johnniwinther
Copy link
Member

I think this is blocked on a fix for #33495

@lrhn
Copy link
Member

lrhn commented Feb 5, 2021

There is a complication in that a redirecting constructor forwards the parameters it receives. If an optional parameter is not provided, it's also not passed to the target constructor of the redirect. This is important if the target parameter has a non-trivial default value.
Example:

class List<T> {
  factory List([int length]) = _ListImpl<T>;
}
class ListImpl<T> {
  factory _ListImpl([Object length = _sentinel]) { 
    if (length == null) throw ArgumentError.notNull("length");
    if (length == sentinel) return GrowableList();
    return FixedLengthList(length as int);
  }
}

That means we can't just replace the target of the redirecting factory constructor with a normal body like

 => _ListImpl(length);

There needs to be different behavior depending whether the parameter is passed or not.
(If the Kernel can detect whether a parameter is passed or not, then it can directly insert the target default value,

  => _ListImpl(?length ? length : _sentinel);

or if it can cheat with types, then it can do List([int length = _sentinel /* not an int */]) => _ListImpl(length);. That feels more risky.)

@eernstg
Copy link
Member

eernstg commented Feb 5, 2021

It is an error to specify a default value for a parameter of a redirecting factory constructor (so we don't need to worry about what to do with such default values), but a desugaring to a non-redirecting factory would work correctly if the desugared parameter declarations include the default values declared by the redirectee (assuming that this step is running in a topological order, such that the redirectee, if it was itself a redirecting factory, is already transformed to a non-redirecting factory and hence already has received default values in this manner).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.
Projects
None yet
Development

No branches or pull requests

4 participants