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

Incremental compilation gives "JsProperty setter %s and getter %s cannot have inconsistent types." when in fact they are consistent #9518

Closed
nfekete opened this issue Apr 30, 2017 · 3 comments
Milestone

Comments

@nfekete
Copy link

nfekete commented Apr 30, 2017

GWT version: 2.8.1
Browser (with version): doesn't matter
Operating System: doesn't matter


Description

I have a type hierarchy with an abstract class defining a @JsProperty through a getter-setter pair. The setter is implemented in a base class, the getter is left as abstract for subclasses to implement. At seemingly random times (which means I couldn't figure out the precise mechanism yet), while using the incremental compiler, it complains that the getter and setter methods have inconsistent types. There's nothing changed that affects these getters and setters, and their type is consistent, the setter accepting and the getter returning String, yet, the incremental compiler sometimes complains about the types being different.

Indeed, when debugging the location where the error is triggered, in com.google.gwt.dev.jjs.impl.JsInteropRestrictionChecker.checkJsPropertyConsistency(JMember, JsMember), the types used by the setter and getter are compared with reference equality, probably under the assumption that there will be no two instances representing the same type.

There are two constructors for JClassType, placing a breakpoint on them to break whenever a JClassType instance gets created to represent java.lang.String I can confirm there will be many JClassType instances created for the same type in a single incremental compilation, so the assumption that reference equality will suffice seems to be wrong.

Shouldn't the JClassType implement a proper semantic equals method and use that instead for comparison? Maybe the whole JNode hierarchy?

Steps to reproduce

I can't provide a reliable way to reproduce it yet.

Known workarounds

Touching both classes that define the getter/setter pairs involved in this error will satisfy the compiler for the next incremental compilation step.

Links to further discussions
@jnehlmeier jnehlmeier added this to the 2.8.2 milestone May 2, 2017
@rluble
Copy link
Contributor

rluble commented May 4, 2017

the types used by the setter and getter are compared with reference equality, probably under the assumption that there will be no two instances representing the same type.

Correct. It is an invariant in the (non incremental) compiler that instances of JDeclaredType are unique AFTER unification. In the incremental mode this invariant is only valid for types that are part of the compilation units participating in the incremental compilation plus those immediately reachable.

The second assumption is that types that are external do not influence the incremental compiler. This second assumption might be false for JsInteropRestrictionChecker which does some analyses that might need to look at these types.

My guess is that happens in very limited cases such as looking at the type of paramters in methods from classes that are not part for the current incremental compilation.

The fix there would be to change

newMember.getter.getType() !=  setterParams.get(0).getType()

to compare the types by name:

!newMember.getter.getType().getName().equals(setterParams.get(0).getType().getName())

Looking at the code in JsInteropRestrictionChecker that seems to be the only place to looks at a type that is not in the current compilation.

@nfekete
Copy link
Author

nfekete commented May 4, 2017

That would probably solve the issue, as I was able to work around it running the compiler in debug mode from Eclipse, and putting a hacky conditional breakpoint with side-effects right at that line, looking like this:

if (java.util.Objects.equals(newMember.getter.getType().getName(), setterParams.get(0).getType().getName())) {
    ((JMethod)newMember.getter).setType(setterParams.get(0).getType());
}
return false;

@rluble
Copy link
Contributor

rluble commented May 4, 2017

Code review at https://gwt-review.googlesource.com/#/c/18260/

hubot pushed a commit that referenced this issue May 4, 2017
…ker.

In incremental compilation (reference only) types might have more than
one JDeclaredType instance representing the same type.
JsInteropRestrictionChecker needs to look in some cases at reference
only types (e.g. when determining whether setter and getter have
the same property type) and in this case comparing by reference equality
is not correct.

Bug: #9518
Bug-Link: #9518
Change-Id: I3778b15791ae1b5d80d28cb1e641de5cdd41ab50
@rluble rluble closed this as completed May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants