Bug 189933 - [compiler][1.5] extraneous ambiguous constructor error on generics
Summary: [compiler][1.5] extraneous ambiguous constructor error on generics
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.3 RC4   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 191029 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-05-30 10:38 EDT by Maxime Daniel CLA
Modified: 2007-06-06 19:21 EDT (History)
2 users (show)

See Also:
philippe_mulet: review+
jerome_lanneluc: review+
maxime_daniel: review+
Olivier_Thomann: review+


Attachments
Discussion basis (a prototype that works, plus split test cases) (17.83 KB, patch)
2007-06-01 08:07 EDT, Maxime Daniel CLA
no flags Details | Diff
Proposed patch (1.91 KB, patch)
2007-06-04 15:17 EDT, Kent Johnson CLA
no flags Details | Diff
Updated patch (4.89 KB, patch)
2007-06-05 10:30 EDT, Kent Johnson CLA
no flags Details | Diff
Correct updated patch (4.99 KB, patch)
2007-06-05 11:13 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 Maxime Daniel CLA 2007-05-30 10:38:25 EDT
I20070525-1350

The following test case raises two errors with JDT, no errors with javac 1.5.0_12 or 6_02:

public class X<T> {
    public void bar(K<T, Object> p) {
	   new Y(p); // err1
	   new Y((J<T, Object>) p); // err2
       new Y((I<T, Object>) p);
    }
}
class Y<T, U> {
  Y(I<? extends T, ? extends U> p) {
  }
  Y(J<T, ? extends U> p) {
  }
}
interface I<T, U> {  
}
interface J<T, U> extends I<T, U> {
}
interface K<T, U> extends I<T, U>, J<T, U> {
}

Build I20070523-0010 (aka a bit later than RC1, with was 20070517-1700) shows no error, but three warnings only (as javac does when using -Xlint:unchecked).

Note that if J does not extend I, both JDT and javac complain on line err1.
Comment 1 Maxime Daniel CLA 2007-05-30 10:54:01 EDT
Reverting the change of bug 18761 fixes the problem.
Comment 2 Maxime Daniel CLA 2007-05-30 10:56:22 EDT
Released inactive test cases AmbiguousMethodTest#60 and 61 in HEAD.
Comment 3 Olivier Thomann CLA 2007-05-30 11:56:27 EDT
(In reply to comment #1)
> Reverting the change of bug 18761 fixes the problem.
This is not the right bug number. Did you mean bug 188960 ?
Comment 4 Maxime Daniel CLA 2007-05-31 00:50:46 EDT
No. Bug 188741 (one digit wrong, apologies).
Comment 5 Maxime Daniel CLA 2007-05-31 10:16:52 EDT
Current thinking is that Scope#2591 should acknowledge the fact that one and two methods may get rawified *by the same token* (which is the case here, since this is Y getting rawified that rawifies both constructors), and that in this case there is no point in unrawifying two alone (nor in unrawifying anyone, indeed). I will dig a bit deeper along this line.
Comment 6 Maxime Daniel CLA 2007-06-01 08:07:16 EDT
Created attachment 69682 [details]
Discussion basis (a prototype that works, plus split test cases)

This is meant to get the discussion starting.
The suggested patch memorizes methods substitutions in the stack above isAcceptableMethod for the test cases that we want to fix.
A few caveats:
- marking the methods themselves, which has design problems anyway, proved to be too strong, as the methods returned by computeCompatibleMethod are cached;
- considering only changes due to the call to computeCompatibleMethod at Scope#3330 proved insufficient, breaking at least one of the AmbiguousMethodTest-s;
- relying upon wasInferred to capture changes due to the call to computeCompatibleMethod at Scope#1123 proved insufficient as well;
- this patch only prototypes a working solution; I have not spent much time proof-reading it yet;
- tests are on their way at the time of writing.
Comment 7 Maxime Daniel CLA 2007-06-01 09:46:21 EDT
Tests pass.
Comment 8 Kent Johnson CLA 2007-06-04 15:17:00 EDT
Created attachment 70019 [details]
Proposed patch

Was too aggressive with previous change for bug 188741

This is a simple tweak that recognizes methods from raw types
Comment 9 Kent Johnson CLA 2007-06-04 15:29:22 EDT
Risk:
The change is simple and has a low risk of causing new problems.

The fix for bug 188741 did not handle the case when the second method is from a raw type already, in which case we should not switch back to the original method.

Benefits:
This is a regression of sorts from 3.3RC2. Its a testcase from actual code, so the fix is necessary for some users.
Comment 10 Maxime Daniel CLA 2007-06-05 04:20:46 EDT
Much better way to make the difference between 'raw' and 'rawified'. Also checked that the suggested patch passed the code that raised the bug.
Hence +1.

Additional remark: since toCheck is used only once, you may consider not declaring at all and using its value instead in the call to needsUncheckedConversion.
Comment 11 Olivier Thomann CLA 2007-06-05 09:46:40 EDT
*** Bug 191029 has been marked as a duplicate of this bug. ***
Comment 12 Olivier Thomann CLA 2007-06-05 10:05:30 EDT
+1.
Test cases from bug 191029 should be added as regression tests.
Comment 13 Kent Johnson CLA 2007-06-05 10:21:00 EDT
Bug 191029 is really a duplicate of the original bug 188960

The testcase for that bug is AmbiguousMethodTest test059

Added the additional test as test059a
Comment 14 Kent Johnson CLA 2007-06-05 10:30:28 EDT
Created attachment 70140 [details]
Updated patch
Comment 15 Kent Johnson CLA 2007-06-05 11:13:46 EDT
Created attachment 70148 [details]
Correct updated patch
Comment 16 Philipe Mulet CLA 2007-06-05 13:26:14 EDT
+1 for 3.3rc4
Comment 17 Jerome Lanneluc CLA 2007-06-05 13:30:50 EDT
+1 for 3.3 RC4
Comment 18 Kent Johnson CLA 2007-06-05 13:31:28 EDT
Released into HEAD for 3.3RC4
Comment 19 Olivier Thomann CLA 2007-06-06 19:21:29 EDT
Verified for 3.3RC4 using I20070606-0010.