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

Forwarding stubs do not handle default parameter values correctly #31027

Closed
stereotype441 opened this issue Oct 6, 2017 · 5 comments
Closed
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. customer-dart-2 front-end-kernel P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@stereotype441
Copy link
Member

Consider the following code (which I'll be submitting as a test case shortly):

class B {
  Object _x;
  void f([num x = 10]) {
    _x = x;
  }
  void g({num x = 20}) {
    _x = x;
  }
  void check(Object expectedValue) {
    if (_x != expectedValue) {
      throw 'Expected _x == $expectedValue; got $_x';
    }
  }
}
abstract class I<T> {
  void f([T x]);
  void g({T x});
}
class C extends B implements I<num> {}
main() {
  C c = new C();
  c.f();
  c.check(10);
  c.g();
  c.check(20);
}

This test fails with the current front end, because when the front end generates forwarding stubs for the class C, it doesn't preserve the default values from B::f and B::g. The front end is adding these methods to the kernel representation of class C:

forwarding-stub method f([generic-covariant-impl core::num x]) → void
  return super.{self::B::f}(x);
forwarding-stub method g({generic-covariant-impl core::num x}) → void
  return super.{self::B::g}(x: x);

(Note the absence of default values for the x parameters). We could fix the bug by copying over the default values from class B, but that would lead to problems with modular compilation, since we don't want to have to preserve default parameter values in summaries. It would be much nicer if we could fix this bug my having a special representation of forwarding stubs in kernel, and let the back end take care of doing the right thing with default parameter values.

@kmillikin I understand you're thinking of making a similar change to how redirecting constructors are represented, for similar reasons. What do you think?

@stereotype441 stereotype441 added area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-kernel labels Oct 6, 2017
whesse pushed a commit that referenced this issue Oct 6, 2017
…y don't)

This test demonstrates issue #31027.

Change-Id: I2d9e8464b0304e1d41ddc775e8b5ffd37b6e8655
Reviewed-on: https://dart-review.googlesource.com/11141
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
@kmillikin kmillikin self-assigned this Nov 28, 2017
@kmillikin kmillikin added this to Incoming in Dart Front End Jan 3, 2018
@sjindel-google sjindel-google modified the milestones: 2.0-alpha2, 2.0 Jan 16, 2018
@sjindel-google sjindel-google added the P2 A bug or feature request we're likely to work on label Jan 16, 2018
@sjindel-google sjindel-google moved this from Incoming Untriaged to Triaged in Dart Front End Jan 16, 2018
@dgrove
Copy link
Contributor

dgrove commented Jun 15, 2018

@stereotype441 Is this required for Dart2Stable?

@stereotype441
Copy link
Member Author

@dgrove I don't really know the status of this bug. @sjindel-google, is it possible that this bug has been fixed?

@mit-mit
Copy link
Member

mit-mit commented Jul 9, 2018

We think we should fix this; @stefantsov to take a look. Tagging P1 for now

@mit-mit mit-mit assigned chloestefantsova and unassigned kmillikin Jul 9, 2018
@mit-mit mit-mit added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Jul 9, 2018
@chloestefantsova
Copy link
Contributor

I'm addressing this in CL 64540 by cloning the default values from the procedure the forwarder was generated for.

@stereotype441
Copy link
Member Author

Note that CL 64540 doesn't address the modular compilation issue described in the original bug description--that issue is now being tracked in #33820.

@kmillikin kmillikin moved this from Triaged to Done in Dart Front End Aug 1, 2018
@kmillikin kmillikin added area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-kernel and removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels Sep 19, 2018
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. customer-dart-2 front-end-kernel P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
Development

No branches or pull requests

6 participants