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

dart2js: inline identical definitions for natives #7465

Closed
rakudrama opened this issue Dec 17, 2012 · 9 comments
Closed

dart2js: inline identical definitions for natives #7465

rakudrama opened this issue Dec 17, 2012 · 9 comments
Labels
P3 A lower priority bug or feature request web-dart2js

Comments

@rakudrama
Copy link
Member

Native methods from dart:html and friends can be aggressively inlined.

Given...

    @­JSName('jsfoo') int foo(int x) native;

... we can inline a Dart call "a.foo(b)" and generate, instead of "a.foo$1(b)", the inlined code "a.jsfoo(b)".

The foo$1 wrapper performs several functions. These are known in the compiler so can be generated or checked in the inlined code without implementing full inlining. They are (1) arity check (2) type annotation checks (in checked mode) (3) (rare) some conversions (wrapping callables in JS functions).

In checked mode we need to satisfy the checks that are inserted into foo$1, i.e. the precondition (b is int) needs to hold.

We can be more general that requiring "a" to be at one point in the inheritance hierarchy - this is especially important for SVG where many methods are inherited from secondary interfaces and explicitly implemented.

Considering the set of concrete types at "a":
If ...

  1. all types implement foo accepting on argument, or the type does not accept 'foo' with one argument and also does not implement noSuchMethod,
  2. all implementations of foo are by a native method named 'jsfoo',
  3. (checked mode) the arguments satisfy the intersection of type annotations constraints for the parameters,
  4. no parameter type of any of the implementations needs a conversion.

... then we can replace "a.foo$1(b)" with "a.jsfoo(b)".

These conditions can often be satisfied with no knowledge of the type of "a", e.g. the name "foo" is used only by native class methods and no classes implement noSuchMethod.
We arrange via method and field name generation that no other types use the names of native methods and fields, so there can be no accidental call to some other element called "jsfoo".

@anders-sandholm
Copy link
Contributor

Removed this from the M3 milestone.
Added this to the M4 milestone.

@kasperl
Copy link

kasperl commented Apr 22, 2013

Removed this from the M4 milestone.
Added this to the M5 milestone.

@kasperl
Copy link

kasperl commented May 23, 2013

Added TriageForM5 label.

@kasperl
Copy link

kasperl commented May 28, 2013

Is there more work to do for this?


Removed this from the M5 milestone.
Added this to the Later milestone.
Removed TriageForM5 label.

@rakudrama
Copy link
Member Author

We have done everything except the case where multiple classes have identical definitions for the native class or field.

There are 17 SVG classes that extend StyledElement and implement SVGLocatable.
The SVGLocatable methods are implemented by each subclass, e.g. there are 17 classes that have identical definitions for getBBox(). It would fruitful to know all classes implementing SVGLocatable have identical definitions that inline the same way.

I believe this only affects SVG code, and only code written to use the interfaces, so the priority is low. I'd be happy to close this issue if another was opened specifically for the multi-implementation case.


Removed Priority-Medium label.
Added Priority-Low label.

@kasperl
Copy link

kasperl commented May 29, 2013

Updated the summary to reflect what's missing.


Changed the title to: "dart2js: inline identical definitions for natives".

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@rakudrama rakudrama added Type-Defect P3 A lower priority bug or feature request web-dart2js labels Aug 4, 2014
@rakudrama
Copy link
Member Author

The specific case of getBBox() seems to have been solved by having it on SVGGraphicElement.
The old html generator used to manually flatten multiple inheritance. Dart now has mixins.
I don't think this is an issue. If I see it in the field, I'll open a new issue.

@kevmoo kevmoo removed the triaged label Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request web-dart2js
Projects
None yet
Development

No branches or pull requests

4 participants