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

Boolean.booleanValue() and other calls that are devirtualized should also be devirtualized in JSNI #9356

Closed
Legioth opened this Issue Jun 2, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@Legioth

Legioth commented Jun 2, 2016

    private static native boolean toPrimitive(Boolean value)
    /*-{
      return value.@Boolean::booleanValue()();
    }-*/;

compiles to value_0.booleanValue(), which doesn't work since value_0 is a native JS boolean that doesn't have a member named booleanValue.

Tested with the 2.8.0-beta1 and 2.8.0-SNAPSHOT.

@rluble

This comment has been minimized.

Contributor

rluble commented Jun 2, 2016

Yes. That needs to be banned as we do for all JSOs.

@Legioth Legioth changed the title from Boolean.valueOf() is not eliminated from JSNI to Boolean.booleanValue() is not eliminated from JSNI Jun 2, 2016

@jnehlmeier jnehlmeier added this to the 2.8 milestone Jun 2, 2016

@jnehlmeier

This comment has been minimized.

Member

jnehlmeier commented Jun 2, 2016

Its the same as with Double, see #9242

@rluble

This comment has been minimized.

Contributor

rluble commented Jun 6, 2016

@rluble rluble added the ReviewPending label Jun 6, 2016

@rluble rluble changed the title from Boolean.booleanValue() is not eliminated from JSNI to Boolean.booleanValue() and other calls that are devirtualized should not be allowed from JSNI Jun 6, 2016

hpehl pushed a commit to hpehl/gwt that referenced this issue Jun 8, 2016

Fix for illegal method calls in JSNI.
Methods that are devirtualized or require trampoline dispatch are not
supported in JSNI.

Bug: gwtproject#9356
Bug-Link: http://github.com/gwtproject/gwt/issues/9356
Change-Id: I3b4ccee4ef158be3b87ce16cddfbade2cfd65446
@rluble

This comment has been minimized.

Contributor

rluble commented Jun 9, 2016

A couple of patches related to this bug are up for review

Improvements to the jsni restriction error reporting - https://gwt-review.googlesource.com/#/c/15120
Only disallow taking a method reference to devirtualized instance methods but allow direct calls to them - https://gwt-review.googlesource.com/#/c/15121

@Legioth

This comment has been minimized.

Legioth commented Jun 10, 2016

Only disallow taking a method reference to devirtualized instance methods but allow direct calls to them - https://gwt-review.googlesource.com/#/c/15121

Does this mean that it would work in the way I suggested in #9242, i.e. that value.@Boolean::booleanValue() is rejected but value@Boolean::booleanValue()() works?

@rluble

This comment has been minimized.

Contributor

rluble commented Jun 10, 2016

Yes. I had to enhance the Devirtualizer to undestand the JS AST; almost all of GWT's Java AST passes it do not deal at all with the JS AST in JSNI, but only use some summary information to know which methods, fields, classliterals are referenced there.

But it turned out to be a quite straight forward to add to the Devritualizer.

Thanks for the suggestion.

@dankurka

This comment has been minimized.

Member

dankurka commented Jun 12, 2016

@rluble rluble closed this Jun 14, 2016

hpehl pushed a commit to hpehl/gwt that referenced this issue Jun 16, 2016

Fix JSNIRestrictionChecker error messages.
Bug: gwtproject#9356
Bug-Link: http://github.com/gwtproject/gwt/issues/9356
Change-Id: Ifbb582be9612e323a1a2e6e72a06bdcf813a9f1d

hpehl pushed a commit to hpehl/gwt that referenced this issue Jun 16, 2016

Allow calls to devirtualized methods from JSNI.
Only disallow taking references to devirtualized methods but
allow invocations.

Bug: gwtproject#9356
Bug-Link: http://github.com/gwtproject/gwt/issues/9356
Change-Id: I98d3248aedc705439db737b2d13e5bbe28469b5f

@rluble rluble changed the title from Boolean.booleanValue() and other calls that are devirtualized should not be allowed from JSNI to Boolean.booleanValue() and other calls that are devirtualized should also be devirtualized in JSNI Jun 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment