Bug 174445 - [1.5][compiler] missing unchecked conversion warning upon parametrized method
Summary: [1.5][compiler] missing unchecked conversion warning upon parametrized method
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 M7   Edit
Assignee: Maxime Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-02-16 10:04 EST by Rémi Forax CLA
Modified: 2007-04-27 10:05 EDT (History)
1 user (show)

See Also:


Attachments
Tentative fix (9.11 KB, patch)
2007-03-12 03:16 EDT, Maxime Daniel CLA
no flags Details | Diff
Test case modifications (1.65 KB, patch)
2007-03-12 03:17 EDT, Maxime Daniel CLA
no flags Details | Diff
Tentative fix + test case (12.26 KB, patch)
2007-03-16 03:24 EDT, Maxime Daniel CLA
no flags Details | Diff
Alternative patch (3.93 KB, patch)
2007-03-16 14:16 EDT, Kent Johnson CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rémi Forax CLA 2007-02-16 10:04:05 EST
Build ID: I20070209-1006

Steps To Reproduce:
1. compile the code, it should not compile but it does.
2. run it, a CCE is thrown

public class EclipseBug2 {
    enum MyEnum {
      value;
    }
    
    enum AnotherValue {
      value2;
    }
    
    static abstract class A<T> {
      abstract <U extends T> U empty();
    }
    
    static class B extends A<Enum<?>> {
      @Override
      Enum<?> empty() {
        return MyEnum.value;
      }  
    }
    
    public static void main(String[] args) {
      A<Enum<?>> a=new B();
      AnotherValue value2 = a.empty();
    }
}

More information:
B.empty doesn't overrides A.empty but eclipse
seems doesn't care about that ??.

section 8.4.2 of the JLS clearly states that
if B.empty overrides A.empty, they must
have the same number of parameter types.
Comment 1 Olivier Thomann CLA 2007-02-16 10:10:11 EST
Note that javac 1.5.0_11, 1.6.0_01 and 1.7b06 also accept it.
Comment 2 Rémi Forax CLA 2007-02-16 15:36:08 EST
Oups, i have forgiven to test with javac.

Do you have reported this bug in SUN database or
should i do it ?

Rémi
Comment 3 Philipe Mulet CLA 2007-02-19 10:28:16 EST
Please do so, you found the problem, and deserve credits for it.
Comment 4 Rémi Forax CLA 2007-02-19 10:56:05 EST
(In reply to comment #3)
> Please do so, you found the problem, and deserve credits for it.
> 

Ok, i will post the SUN bug id here when it will be available.

About the credits, are you taking about respect or due ?
SUN already send me a workstation and
i don't think i can have more credits :)

I have just wanted to avoid duplicates.

Rémi
Comment 5 Rémi Forax CLA 2007-02-20 05:41:45 EST
the SUN bug id is 6526545.
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6526545

Rémi

Comment 6 Maxime Daniel CLA 2007-02-27 12:02:27 EST
Reading JLS 3 § 8.4.2, it seems to me that the signature of B#empty() is a subsignature of the signature of A#empty() (because the erasure erases the type parameters). If I am right, then the code must compile.
Nevertheless, javac reports an unchecked conversion that should draw the user attention onto the incumbent CCE, and we don't. I will further stress an early patch I have that enables us to do that (warn about the unchecked conversion) and post progress here.
Please speak up if I have missed something.
Comment 7 Rémi Forax CLA 2007-03-07 10:08:58 EST
(In reply to comment #6)
> Reading JLS 3 § 8.4.2, it seems to me that the signature of B#empty() is a
> subsignature of the signature of A#empty() (because the erasure erases the type
> parameters). 

A#empty() is override equivalent to B#empty()
if A#empty() == erasure(B#empty()) or
   B#empty() == erasure(A#empty())

because A#empty() and B#empty() are both generics,
they can not be override equivalent.

> If I am right, then the code must compile.
> Nevertheless, javac reports an unchecked conversion that should draw the user
> attention onto the incumbent CCE, and we don't. I will further stress an early
> patch I have that enables us to do that (warn about the unchecked conversion)
> and post progress here.

an error must be reported here, not an unchecked conversion.

> Please speak up if I have missed something.
> 

Rémi
Comment 8 Maxime Daniel CLA 2007-03-07 10:34:36 EST
(In reply to comment #7)
> (In reply to comment #6)
> > Reading JLS 3 § 8.4.2, it seems to me that the signature of B#empty() is a
> > subsignature of the signature of A#empty() (because the erasure erases the type
> > parameters). 
> 
> A#empty() is override equivalent to B#empty()
> if A#empty() == erasure(B#empty()) or
>    B#empty() == erasure(A#empty())
Let us note the 'has same signature as' relationship as '<=>'.
erasure(A#empty) <=> B#empty iff:
a) they have the same name (which is true);
and
b) they have the same argument types, which is true if all of the following is
   true:
   b1) they have the same number of formal parameters (true: 0);
   b2) they have the same number of type parameters (true: 0);
   b3) bound check condition, which we don't care since we have no type 
       parameter left.
Hence B#empty <=> erasure(A#empty) and comment #6 should hold?
Comment 9 Rémi Forax CLA 2007-03-07 10:54:09 EST
ok, forget my last comment.

> Let us note the 'has same signature as' relationship as '<=>'.
> erasure(A#empty) <=> B#empty iff:
> a) they have the same name (which is true);
> and
> b) they have the same argument types, which is true if all of the following is
>    true:
>    b1) they have the same number of formal parameters (true: 0);
>    b2) they have the same number of type parameters (true: 0);

oups, A#empty() has one type parameter (T) and
B#empty() zero.

>    b3) bound check condition, which we don't care since we have no type 
>        parameter left.
> Hence B#empty <=> erasure(A#empty) and comment #6 should hold?
> 

so B#empty <=> erasure(A#empty)

Rémi
Comment 10 Maxime Daniel CLA 2007-03-09 03:59:15 EST
Changing the title to reflect the problem analysis results.
Comment 11 Rémi Forax CLA 2007-03-09 06:16:57 EST
(In reply to comment #10)
> Changing the title to reflect the problem analysis results.
> 

Daniel, i agree with your analysis if the code is this one :
public class OverrideGenerics {
    static abstract class A<T> {
      abstract <U extends T> U f();
    }
    static class B extends A<Number> {
      @Override
      Number f() {
        return 3;
      }  
    }
}

but in the submitted code, the return type of B#empty is
not the raw type Enum but Enum<?>, so for me

erasure(bridge(A#empty)) => Enum empty()
and
B#empty => Enum<?>

so there is no overriding here.
Comment 12 Maxime Daniel CLA 2007-03-09 06:50:20 EST
If I am right, the return type is not considered as far as establishing the subsignature property is concerned (JLS 3 § 8.4.2).
Comment 13 Maxime Daniel CLA 2007-03-12 03:16:36 EDT
Created attachment 60532 [details]
Tentative fix

This fix adds a parameter to method substitution so as to decide whether closest matches should use upper bound or erasure types when the type parameters numbers do not match. This may be a rather crude approach to the issue, but it yields results and should kick the discussion off.
Comment 14 Maxime Daniel CLA 2007-03-12 03:17:02 EDT
Created attachment 60533 [details]
Test case modifications
Comment 15 Maxime Daniel CLA 2007-03-15 10:18:32 EDT
Kent, any thoughts?
Comment 16 Maxime Daniel CLA 2007-03-16 03:24:10 EDT
Created attachment 61057 [details]
Tentative fix + test case

Patch refresh on HEAD.
Comment 17 Kent Johnson CLA 2007-03-16 14:16:01 EDT
Created attachment 61146 [details]
Alternative patch

Not sure why the previous test was separated onto 2 lines...
Comment 18 Maxime Daniel CLA 2007-03-19 05:03:16 EDT
Not sure either, since the test for the bug that change fixed (83218) passes with your patch, which is far more elegant than mine. Will release it for M7 after running full tests.
Comment 19 Maxime Daniel CLA 2007-03-29 04:11:27 EDT
Released for 3.3 M7.
Comment 20 Olivier Thomann CLA 2007-04-27 10:05:50 EDT
Verified for 3.3M7 using I20070427-0010