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

[DDC/Dart 2.0] Fix behavior of type errors on partial instantiation in DDK. #34296

Closed
sjindel-google opened this issue Aug 29, 2018 · 4 comments
Assignees
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dev-compiler
Milestone

Comments

@sjindel-google
Copy link
Contributor

It was decided in #31953 that the partial instantiation operator needs to check the bounds of type arguments at the point of partial instantiation, rather than waiting until the resulting closure is called. The VM is currently being updated with the correct behavior, but DDK also has the incorrect behavior. For example, the following test should pass:

// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
//
// This test checks that necessary type argument bounds checks are performed
// eagerly during partial instantiation, rather than being delayed until the
// partially instantiated closure is invoked.

import "package:expect/expect.dart";

class C<T> {
  void foo<S extends T>(S x) {}
}

void main() {
  C<Object> c = C<int>();
  void Function(String) fn;
  Expect.throwsTypeError(() {
    fn = c.foo;
  });
}

I've suggested P1 priority for this issue because fixing the semantics here is a breaking change.

@sjindel-google sjindel-google added 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) web-dev-compiler labels Aug 29, 2018
@jmesserly jmesserly added this to the Dart2.1 milestone Aug 29, 2018
@jmesserly
Copy link

DDC probably has the same behavior with either Analyzer or Kernel backend. If that's true we'd have to fix both.

(regarding priority: I don't think this would be P1 per the guidelines? https://github.com/dart-lang/sdk/wiki/How-the-issue-tracker-works ... as this is not "A single project is unusable or has many test failures". Implicit instantiation is fairly obscure, and the only problem with the current behavior is throwing an exception later than we should. But it seems like something we should fix by the 2.1 milestone for consistency.)

@jmesserly jmesserly removed the P1 A high priority bug; for example, a single project is unusable or has many test failures label Aug 29, 2018
@sjindel-google
Copy link
Contributor Author

Thanks for linking me to that page, I wasn't aware that we had general priority guidelines across teams.

Note that implicit instantiation is not as obscure as it may seem. For example, this external customer hit a performance issue with it in the VM in Dart 2.0: #34090.

@jmesserly
Copy link

Note that implicit instantiation is not as obscure as it may seem. For example, this external customer hit a performance issue with it in the VM in Dart 2.0: #34090.

oh, good to know. thank you! :)

@jmesserly jmesserly self-assigned this Aug 31, 2018
@jmesserly
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dev-compiler
Projects
None yet
Development

No branches or pull requests

3 participants