Bug 225737 - [compiler][1.5] Generics - JDT cannot compile hudson-core when javac can
Summary: [compiler][1.5] Generics - JDT cannot compile hudson-core when javac can
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 RC2   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-04 09:28 EDT by Sébastien Vauclair CLA
Modified: 2008-05-23 06:34 EDT (History)
3 users (show)

See Also:
maxime_daniel: review+
Olivier_Thomann: review+


Attachments
Minimal project to reproduce bug (2.60 KB, application/x-zip-compressed)
2008-04-04 09:28 EDT, Sébastien Vauclair CLA
no flags Details
Proposed patch (4.23 KB, patch)
2008-05-16 15:49 EDT, Philipe Mulet CLA
no flags Details | Diff
Patch for 3.3.x (5.16 KB, patch)
2008-05-16 18:52 EDT, Philipe Mulet CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sébastien Vauclair CLA 2008-04-04 09:28:03 EDT
Created attachment 94841 [details]
Minimal project to reproduce bug

Build ID: M20080221-1800

Steps To Reproduce:
1. Open attached JDT project (minimal set of classes extracted from the Hudson project, http://hudson.dev.java.net/)
2. Compile with javac then run class JdtBugTest: no error
3. Run class JdtBugTest from Eclipse:
Exception in thread "main" java.lang.Error: Unresolved compilation problem: 
The method filter(List<Descriptor<T>>, Class<? extends AbstractProject<?,?>>) in the type BuildStepDescriptor is not applicable for the arguments (PublisherList, Class<capture#1-of ? extends Object>)

More information:
Happens 3.4M6 and 3.3.2
Comment 1 Philipe Mulet CLA 2008-05-16 12:50:02 EDT
Reproduced.
Comment 2 Philipe Mulet CLA 2008-05-16 12:57:54 EDT
I would agree this is our bug.
Comment 3 Philipe Mulet CLA 2008-05-16 13:11:11 EDT
Tentatively tagging for 3.4RC2, since Hudson libs are quite popular.
Comment 4 Sébastien Vauclair CLA 2008-05-16 13:36:27 EDT
Thanks for looking after this one!
Note that Hudson seems to have been fixed in the meantime, according to: 
https://hudson.dev.java.net/servlets/BrowseList?listName=dev&from=1082374&to=1082374&count=7&by=thread&paged=false
Comment 5 Philipe Mulet CLA 2008-05-16 15:22:50 EDT
Problem does not come from inference, but from #getClass() invocation.
Comment 6 Philipe Mulet CLA 2008-05-16 15:23:13 EDT
Reduced testcase:

public class X extends AbstractProject<X, Object> {
	public void testBug() {
		filter(getClass());
	}
	public static void filter(Class<? extends AbstractProject<?, ?>> type) {
	}
}
abstract class AbstractProject<P extends AbstractProject<P, R>, R> {
}
Comment 7 Philipe Mulet CLA 2008-05-16 15:31:02 EDT
Re: comment 4
Thanks for the pointer, indeed this is really strange. An implicit message send fails, where explicit ones do succeed...

e.g.
public class X extends AbstractProject<X, Object> {
	public void testBug() {
		filter(getClass());//1
		filter(this.getClass());//2
		filter(X.class);//3
	}
	public static void filter(Class<? extends AbstractProject<?, ?>> type) {
	}
}
abstract class AbstractProject<P extends AbstractProject<P, R>, R> {
}

only (1) does fail
Comment 8 Philipe Mulet CLA 2008-05-16 15:43:28 EDT
Problem found. Indeed implicit msg send to #getClass() did not get properly substituted for its return type: Class<?> into Class<? extends |receiverType|>.

Added GenericTypeTest#test1333-1334.
Comment 9 Philipe Mulet CLA 2008-05-16 15:49:22 EDT
Created attachment 100718 [details]
Proposed patch

Reviewers - there was a code path (through Scope#getImplicitMethod(...) which did not process #getClass(). The processing code was already written elsewhere (see Scope#getMethod(...) line 2118 for instance), but not invoked.
Comment 10 Olivier Thomann CLA 2008-05-16 16:05:54 EDT
+1 for 3.4RC2. Now the code is identical to the getMethod(...) path.
Comment 11 Philipe Mulet CLA 2008-05-16 18:52:32 EDT
Created attachment 100749 [details]
Patch for 3.3.x
Comment 12 Philipe Mulet CLA 2008-05-16 19:28:58 EDT
Released in 3.3.x maintenance stream.
Comment 13 Philipe Mulet CLA 2008-05-19 03:33:04 EDT
Given OTT is off today, asking Maxime for review.
Comment 14 Maxime Daniel CLA 2008-05-19 04:15:20 EDT
Per the same token as Olivier (symmetry of code paths), I +1 the proposed change for 3.4 RC2. 

Given the fact that this code has to be invoked very often, I'd suggest that in 3.5 we consider reordering the terms of the boolean expression though, in both places, to make the most of the shortcut. I've not established full-blown statistics for this, but I would expect the two first terms to be true very often, the third to be false most of the time, and the fourth to be expansive to compute (which is a good reason to keep it where it is). Which probes for a comparison of the performances of the current ordering with (at least) one ordering that would bring the third term (or even a faster quick check of part of it) in first position. Adding Kent to the discussion for this specific point.
Comment 15 Philipe Mulet CLA 2008-05-19 08:53:55 EDT
Re: comment 14

The first 2 tests are identity tests, where the 3rd one is a char[] comparison.
So slower, also message sends to as receiver type Object are not super frequent.

if (receiverType.id != T_JavaLangObject
   && argumentTypes == Binding.NO_PARAMETERS
   && CharOperation.equals(selector, GETCLASS)
   && methodBinding.returnType.isParameterizedType()/*1.5*/) {
Comment 16 Philipe Mulet CLA 2008-05-23 04:08:08 EDT
Forgot to mark it as fixed.
Comment 17 Eric Jodet CLA 2008-05-23 06:34:57 EDT
verified for 3.4RC2 using build I20080523-0100