Skip to content

Specify treatment of missing type arguments in redirecting factory constructors #32049

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
eernstg opened this issue Feb 5, 2018 · 7 comments
Closed
Assignees
Labels
area-specification (deprecated) Deprecated: use area-language and a language- label. status-blocked Blocked from making progress by another (referenced) issue

Comments

@eernstg
Copy link
Member

eernstg commented Feb 5, 2018

Currently we do not specify explicitly how to deal with the case where a redirecting factory constructor omits actual type arguments from the redirectee:

class A<S extends num, T extends num> {
  // Redirectee accepts 3 type arguments, `S` and `T` are in scope, no relation expected.
  factory A("argumentList") = B;
}

class B<X extends num, Y extends num, Z extends num> implements A<X, Y> {
  B("argumentList") {
    print("Constructing a B<$X, $Y, $Z>");
  }
}

void main() {
  // Inference applies to `B` when invoked explicitly, will select 3 type arguments
  // to `B` such that the resulting instance has a suitable type for assignment to `x`.
  A<int, int> x = new B("actuals");
  // This case is similar, but not currently specified.
  new A<int, int>("actuals");
}

This issue is intended to ensure that the specification treats the above class A in the same way as the following variant:

class A<S extends num, T extends num> {
  factory A("argumentList") { return new B("arguments"); }
}

for which we already have a well-established inference process. Note that this approach was discussed in #30855 first, this issue just exists in order to focus on the specification task which is not the main topic of that issue.

This may be added to dartLangSpec.tex when we specify instantiate-to-bound in general, or it may be added in some feature spec before that (but we don't currently have any feature specs where it fits naturally).

@eernstg eernstg added the area-specification (deprecated) Deprecated: use area-language and a language- label. label Feb 5, 2018
@eernstg eernstg self-assigned this Feb 5, 2018
@eernstg
Copy link
Member Author

eernstg commented Feb 6, 2018

No milestone now: This will not block Dart 2.

@eernstg
Copy link
Member Author

eernstg commented Feb 12, 2018

Note that we already have the following in dartLangSpec.tex on redirecting factory constructors (where $k$ is the redirecting factory constructor and $k'$ is its redirectee):

Calling a redirecting factory constructor $k$ causes the constructor
$k'$ denoted by $type$ (respectively, $type.identifier$) to be called with
the actual arguments passed to $k$, and returns the result of $k'$ as the result of $k$.
The resulting constructor call is governed by the same rules as an instance creation
expression using \NEW{} (\ref{instanceCreation}).

This implies that the static checks applied to the invocation of the redirecting factory constructor $k$ correspond to the static checks that would be applied to the corresponding invocation of the redirectee $k'$, and missing type arguments for $k$ are treated like missing type arguments for $k'$, etc.

This makes it a natural generalization (when the language specification is updated to cover inference) that inference of type arguments for $k$ should amount to inference on the corresponding invocation of $k'$, which is exactly what this issue requests.

So the current language specification already says what it should say in order to "generalize naturally" to do what this issue requests. Hence, this issue exists simply as a reminder to double-check that the update to include inference does indeed satisfy this expectation.

@eernstg
Copy link
Member Author

eernstg commented Feb 12, 2018

Marking this issue as blocked, awaiting specification of inference (global: #27499, local: #27500).

@eernstg eernstg added the status-blocked Blocked from making progress by another (referenced) issue label Feb 12, 2018
dart-bot pushed a commit that referenced this issue Jul 10, 2018
The current specification seems to allow (or even mandate) that a
redirecting constructor (factory or generative) must be considered as a
syntactic shorthand for an invocation of the redirection target, with
no checks applied statically or dynamically according to the
declarations in the redirected constructor. So the following would be
OK:

  class A {
    A(num n) { print(n); }
    A.foo(int i): this(i);
  }

  main() => new A.foo(3.0);

This CL changes dartLangSpec.tex to mandate all the checks (static
and dynamic) for the declaration of the redirecting constructor as
well as each one of the redirection targets.

Note that the analyzer already rejects the above program, which
lessens the disruption and the implementation burden, but compilers
would presumably need to have the dynamic checks implemented.

Underlying issues: #31590,
#32049,
#32511.

Change-Id: Icc15da6b817e4e678cdfc8829a1e06458756eb4b
Reviewed-on: https://dart-review.googlesource.com/28140
Reviewed-by: Leaf Petersen <[email protected]>
Reviewed-by: Lasse R.H. Nielsen <[email protected]>
@eernstg
Copy link
Member Author

eernstg commented Jul 13, 2018

Note the following example: #32988 (comment). It shows how the current treatment allows for a heap invariant violation (c refers to an instance of C<dynamic>, but it has type annotation C<int>), but it also shows that the type arguments in the redirection of the constructor C.infer falls back on dynamic (instantiate to bound?) and never checks that the type of the returned object is appropriate.

So that's a case where the situation addressed in this issue plays a major role: If that type argument had been chosen by inference then the code could have been in good shape (using C<T1>), otherwise it may be an error that the type argument is omitted at all, or we might choose instantiate-to-bound and then raise an error because C<dynamic> won't work.

@lrhn
Copy link
Member

lrhn commented Jul 13, 2018

I think we should specify that we perform inference on the redirectee, if necessary. In almost all cases, it will be a simple copying over of the type arguments. In a few cases, there will be more complicated situations, but no more complicated than if we had just written => Redirectee(arguments) instead of = Redirectee.

If we do not infer, then it must be a compile-time error if a constructor does not guarantee that the created object is a subtype of the constructed class's type.

lrhn pushed a commit to dart-lang/language that referenced this issue Aug 22, 2018
The current specification seems to allow (or even mandate) that a
redirecting constructor (factory or generative) must be considered as a
syntactic shorthand for an invocation of the redirection target, with
no checks applied statically or dynamically according to the
declarations in the redirected constructor. So the following would be
OK:

  class A {
    A(num n) { print(n); }
    A.foo(int i): this(i);
  }

  main() => new A.foo(3.0);

This CL changes dartLangSpec.tex to mandate all the checks (static
and dynamic) for the declaration of the redirecting constructor as
well as each one of the redirection targets.

Note that the analyzer already rejects the above program, which
lessens the disruption and the implementation burden, but compilers
would presumably need to have the dynamic checks implemented.

Underlying issues: dart-lang/sdk#31590,
dart-lang/sdk#32049,
dart-lang/sdk#32511.

Change-Id: Icc15da6b817e4e678cdfc8829a1e06458756eb4b
Reviewed-on: https://dart-review.googlesource.com/28140
Reviewed-by: Leaf Petersen <[email protected]>
Reviewed-by: Lasse R.H. Nielsen <[email protected]>
@eernstg
Copy link
Member Author

eernstg commented Aug 22, 2018

If we do not infer, then it must be a compile-time error if a constructor does
not guarantee that the created object is a subtype of the constructed class's type.

With 9282a34 that is indeed a compile-time error (and the spec currently always assumes that inference has already taken place).

@eernstg
Copy link
Member Author

eernstg commented Sep 28, 2018

Closing: We've decided that we will add a specification of inference (of any kind) in a later edition of the language specification, and until then the specification will express everything under the assumption that inference has taken place already. This means that this issue can be closed: Inference for this case will be specified elsewhere (than dartLangSpec.tex), and if there is no result from inference then we will use instantiate to bound, which is handled in instantiate-to-bound.md, which is being integrated into the language specification in a separate effort.

@eernstg eernstg closed this as completed Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-specification (deprecated) Deprecated: use area-language and a language- label. status-blocked Blocked from making progress by another (referenced) issue
Projects
None yet
Development

No branches or pull requests

2 participants