Bug 273787

Comment 1 Kent Johnson 2009-05-20 16:13:22 EDT

javac also reports 2 errors in this case, which eclipse does not :

class X {
	<U> void test(Comparable<? extends U> v) {
		if (v instanceof Integer) {
			Object obj = Long.valueOf(((Integer) v).longValue());
		}
	}
}


We do not seem to detect that Comparable<? extends Integer> is not a good match for Integer or its superinterface Comparable<Integer>

Comment 2 Kent Johnson 2009-05-21 14:48:48 EDT

Created attachment 136700 [details]
Proposed patch and testcase

Comment 3 Kent Johnson 2009-05-21 15:17:10 EDT

Philippe & Olivier please review for 3.5

Comment 4 Olivier Thomann 2009-05-21 15:46:52 EDT

+1. Looks consistent with javac.

Comment 5 Kent Johnson 2009-05-22 10:56:23 EDT

Created attachment 136808 [details]
Proposed patch and testcase

Missed a super case

Comment 6 Kent Johnson 2009-05-22 14:05:39 EDT

If we remove both isTypeVariable() checks (see the patch) & do isCompatibleWith() calls :


@ line 583
-			if (!isTypeVariable() && !otherType.isTypeVariable()) {
 				return !isCompatibleWith(otherType);
- 			}

@ line 826
 		if (lowerBound2 != null) {
-			if (lowerBound2.isTypeVariable() || isTypeVariable()) {
- 				return false;
- 			}
			return !lowerBound2.isCompatibleWith(this);


We report the same 2 errors as javac on GenericTypeTest.test0675(), which is from bug 95638

Comment 7 Philipe Mulet 2009-05-23 08:34:10 EDT

This looks inconsistent (with patch):

if (lowerBound2 != null) {
  if (isTypeVariable()) {
    // check compatibility when lowerBound2 is a type variable
    // https://bugs.eclipse.org/bugs/show_bug.cgi?id=273787
    return false;
  }

did you mean ?

if (lowerBound2 != null) {
  if (lowerBound2.isTypeVariable()) {
    // check compatibility when lowerBound2 is a type variable
    // https://bugs.eclipse.org/bugs/show_bug.cgi?id=273787
    return false;
  }

Comment 8 Philipe Mulet 2009-05-23 08:49:18 EDT

Also in #isProbablyDistinctTypeArguments(...) there are many other places where both checks for type variable are present on both types; so you'd have to fix them all to be consistent.

The spec for provably distinct types is well-known to be far from complete, and javac has holes in this area.

I would not change our present behavior unless we had a better guidance from the spec, or at least a feel for a consistent change across our checks. 

Comment 9 Philipe Mulet 2009-05-23 09:02:31 EDT

I do not see why 'T' and 'Integer' would be provably distinct (in first example). For instance, I could construct a X<Integer> and invoke #test(Integer)...
So in essence, I would think that our behavior is actually correct, and javac's buggy. I would not want then to alter our behavior to alter their bugs.

public class X<T extends Comparable<?>> {
	public void test(T v) {
		if (v instanceof Integer) {
			Object obj = Long.valueOf(((Integer) v).longValue());
		}
	}
	void test2(Comparable<?> v) {
		if (v instanceof Integer) {
			Object obj = Long.valueOf(((Integer) v).longValue());
		}
	}
	public static void main(String[] args) {
		X<Integer> xi = new X<Integer>();
		xi.test(Integer.valueOf(2));
	}
}

Comment 10 Tim Ellison 2009-06-26 08:48:30 EDT

So is the conclusion that there is no bug to fix, or are you still thinking about it?

Comment 11 Eclipse Genie 2020-04-21 17:56:59 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.