Bug 330347 - [1.4][1.5][compiler] The performance test FullSourceWorkspaceBuildTests#testFullBuildDefault() fails
Summary: [1.4][1.5][compiler] The performance test FullSourceWorkspaceBuildTests#testF...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.7 M4   Edit
Assignee: Srikanth Sankaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 330428 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-11-16 08:44 EST by Satyam Kandula CLA
Modified: 2010-12-07 12:08 EST (History)
3 users (show)

See Also:


Attachments
Proposed fix (1.09 KB, patch)
2010-11-17 11:34 EST, Olivier Thomann CLA
no flags Details | Diff
Patch with regression test (3.81 KB, patch)
2010-11-18 01:11 EST, Srikanth Sankaran CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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)