Bug 330347

Summary: [1.4][1.5][compiler] The performance test FullSourceWorkspaceBuildTests#testFullBuildDefault() fails
Product: [Eclipse Project] JDT Reporter: Satyam Kandula <satyam.kandula>
Component: CoreAssignee: Srikanth Sankaran <srikanth_sankaran>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, jamesblackburn+eclipse, Olivier_Thomann
Version: 3.7   
Target Milestone: 3.7 M4   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Proposed fix
none
Patch with regression test none

Description Satyam Kandula CLA 2010-11-16 08:44:42 EST
This test FullSourceWorkspaceBuildTests#testFullBuildDefault() does seem to be failing since around 4th November. 8 unexpected errors are getting reported.
Comment 1 Olivier Thomann CLA 2010-11-16 16:49:42 EST
Could you please provide the corresponding errors ?
Comment 2 Olivier Thomann CLA 2010-11-17 08:46:35 EST
*** Bug 330428 has been marked as a duplicate of this bug. ***
Comment 3 Olivier Thomann CLA 2010-11-17 09:15:59 EST
The 8 errors are:
PrimitiveValueImpl.java:
	The method compareTo(Float) in the type Float is not applicable for the arguments (Object)
PrimitiveValueImpl.java:
	The method compareTo(Integer) in the type Integer is not applicable for the arguments (Object)
PrimitiveValueImpl.java:
	The method compareTo(Long) in the type Long is not applicable for the arguments (Object)
PrimitiveValueImpl.java:
	The method compareTo(Short) in the type Short is not applicable for the arguments (Object)
PrimitiveValueImpl.java:
	The method compareTo(Character) in the type Character is not applicable for the arguments (Object)
PrimitiveValueImpl.java:
	The method compareTo(Byte) in the type Byte is not applicable for the arguments (Object)
PrimitiveValueImpl.java:
	The method compareTo(Double) in the type Double is not applicable for the arguments (Object)
SortedMapFactory.java:
	The method compareTo(String) in the type String is not applicable for the arguments (Object)
Comment 4 Olivier Thomann CLA 2010-11-17 09:27:24 EST
In order to reproduce the failure, compile the following code using a 1.5 or above JDK library and in compliance 1.4:
public class X {
	Object fValue;

	public int compareTo(Object obj) {
			return ((Character)fValue).compareTo(obj);
	}
}

The compareTo(..) call is not resolved to the bridge method and in 1.5 libraries the java.lang.Character class doesn't have a compareTo(Object) method anymore. It has a compareTo(Character) method and a bridge method with Object.

Srikanth, could you please take a look at it ?
Comment 5 Olivier Thomann CLA 2010-11-17 09:55:10 EST
Srikanth, could this be related to the filtering of bridge methods in:
org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.createMethods(IBinaryMethod[], long, char[][][]) ?

I wonder if we should not remove the check on the originalComplianceLevel.

The reason is that if you compile in 1.4 mode using a 1.5 library, you want to preserve the bridge methods, but with the check we would only preserve it if the original compliance is 1.5 or above.

That might introduce other problems. I'll run the tests to check that.
Comment 6 Olivier Thomann CLA 2010-11-17 11:34:52 EST
Created attachment 183314 [details]
Proposed fix

This brings back bridge methods in 1.4 mode. All tests seem to be fine with this change. If I remember well, this was done to fix reconciler issues.
Srikanth, do you have concerns about this change that might bring back some of the reconciler issues ?
Comment 7 Srikanth Sankaran CLA 2010-11-18 00:20:26 EST
(In reply to comment #5)
> Srikanth, could this be related to the filtering of bridge methods in:
> org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.createMethods(IBinaryMethod[],
> long, char[][][]) ?

Yes it is.

> I wonder if we should not remove the check on the originalComplianceLevel.

Yes, we should. The change from complianceLevel to originalComplianceLevel
is a recent one that is causing this problem to act up, but any check against
compliance level is wrong here.   

The check against compliance check got introduced by the fix for
bug# 124943. Interestingly, bug# 124943 comment# 4 speaks only about
a check against source level, while the actual fix that went in also
checked against compliance level which is wrong. 

bug# 124943 comment# 6 is untenable - as it is not just a question of
JRE, interoperability of 1.4/1.5 (non-jre) code can easily lead to this
situation. 

(In reply to comment #6)

> Srikanth, do you have concerns about this change that might bring back some of
> the reconciler issues ?

No, I don't think so, there are regression tests for the reconciler problems
for one thing, for another any check against compliance level is inappropriate
here and so needs to go.
Comment 8 Srikanth Sankaran CLA 2010-11-18 00:26:56 EST
(In reply to comment #7)

[...]

> The check against compliance check got introduced by the fix for
> bug# 124943. Interestingly, bug# 124943 comment# 4 speaks only about
> a check against source level, while the actual fix that went in also
> checked against compliance level which is wrong. 

As a result, see that this test case below fails even after restoring
the check against complianceLevel from being originalComplianceLevel.

(In reply to comment #4)
> In order to reproduce the failure, compile the following code using a 1.5 or
> above JDK library and in compliance 1.4:
> public class X {
>     Object fValue;
> 
>     public int compareTo(Object obj) {
>             return ((Character)fValue).compareTo(obj);
>     }
> }
Comment 9 Srikanth Sankaran CLA 2010-11-18 01:11:15 EST
Created attachment 183363 [details]
Patch with regression test

Added unit test org.eclipse.jdt.core.tests.compiler.regression.BatchCompilerTest.testBridgeMethodRetention().

This test shows that checking against any compliance level (be it
the requesting project aka "original" compliance level or the requested
project) is wrong.
Comment 10 Srikanth Sankaran CLA 2010-11-18 01:39:33 EST
(In reply to comment #6)

> Srikanth, do you have concerns about this change that might bring back some of
> the reconciler issues ?

I can't of think of a problem with binary types, but we do have an issue
with source types, bridge methods and reconcile. Since it is unconnected
to the current subject I have raised bug 330537
Comment 11 Srikanth Sankaran CLA 2010-11-18 01:43:08 EST
Released in HEAD for 3.7 M4
Comment 12 Olivier Thomann CLA 2010-12-07 12:08:34 EST
Verified using I20101207-0250 (4.1 I-build)