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

Breaking: dart2js checks setter return type #20032

Closed
DartBot opened this issue Jul 14, 2014 · 10 comments
Closed

Breaking: dart2js checks setter return type #20032

DartBot opened this issue Jul 14, 2014 · 10 comments
Assignees
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures web-dart2js
Milestone

Comments

@DartBot
Copy link

DartBot commented Jul 14, 2014

This issue was originally filed by dvar...@skawa.hu


What steps will reproduce the problem?
Unfortunately, attempts to create a minimalistic test-case for this issue has been a failure so far. I will keep trying to narrow down in what situations this problem occurs.
This issue occurs only if dart2js is invoked with -c option.

What is the expected output? What do you see instead?
Let's examine this line of dart code (where myElement is an instance of Element):
  myElement.scrollTop = 0;

I would expect this line of code to be compiled to JS as:
  t1.scrollTop = C.JSNumber_methods.toInt$0(C.JSInt_methods.roundToDouble$0(0))
where the following expression does evaluate as -> value:
  t1 instanceof HTMLElement -> true // the instance of myElement
  isNaN(C.JSNumber_methods.toInt$0(C.JSInt_methods.roundToDouble$0(0))) -> false
  
Instead the compiled output looks like this:
H.voidTypeCheck(t1.scrollTop = C.JSNumber_methods.toInt$0(C.JSInt_methods.roundToDouble$0(0)));

What version of the product are you using? On what operating system?
Windows
Dart versions tested and the error occurs: 1.5.1, 1.5.2, 1.5.3
The issue with SDK version 1.4.3 does not happen.

Please provide any additional information below.
Checking the return type of the assignment will raise and Error
"Uncaught Error: type 'JSInt' is not a subtype of type 'void'"

Checking the scrollTop setter of Element, one can see that the return type of it is indeed void. However, should type checking for setters occur?
In the minimalistic project I was trying to reproduce the error in seemed to have compiled a similar scrollTop setter properly.

@sethladd
Copy link
Contributor

Added Area-Dart2JS, Triaged labels.

@floitschG
Copy link
Contributor

cc @johnniwinther.

@johnniwinther
Copy link
Member

The bug can be reproduced be compiled this program in checked mode:

import 'dart:html';

main() {
  new Element.div().scrollTop = 0;
}


Added Accepted label.

@DartBot
Copy link
Author

DartBot commented Aug 26, 2014

This comment was originally written by dvar...@skawa.hu


I would really appreciate some update on this issue. This one prevents us from following updates and upgrading our Dart version from 1.4.3. At the time of writing this 1.5.8 is out.

@floitschG
Copy link
Contributor

Added this to the 1.7 milestone.
Removed Priority-Unassigned label.
Added Priority-High label.

@johnniwinther
Copy link
Member

The immediate problem (scrollTop) has been fixed in r38925 (https://codereview.chromium.org//443883002).

Do you experience it generally or for other properties?

@floitschG
Copy link
Contributor

Set owner to @karlklose.

@karlklose
Copy link
Contributor

As I mentioned in the codereview of r38925, dart2js implements the specified semantics:

  'Even though the return value of a setter is ignored, it must be checked against
the declared return type in checked mode since setters are evaluated like normal
functions (see Specification, 16.12).'


Added NeedsInfo label.

@DartBot
Copy link
Author

DartBot commented Sep 9, 2014

This comment was originally written by dvar...@skawa.hu


I have done some tests on our code with SDK 1.6.0.
I was unable to reproduce the issue that were experienced in versions 1.5.x.

We have not ran into any other properties that would cause problems (although scrollTop is the only setter being used in our code with a return value other than void).

If we can be of any further help tackling this issue, please let us know.

@floitschG
Copy link
Contributor

Added Fixed label.

@DartBot DartBot added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures web-dart2js labels Sep 9, 2014
@DartBot DartBot added this to the 1.7 milestone Sep 9, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures web-dart2js
Projects
None yet
Development

No branches or pull requests

5 participants