RFR: JDK-8207224: Javac compiles source code despite illegal use of unchecked conversions

[joe darcy]

Hi Vicente,

Since it sounds like there is nontrivial source compatibility impact 
with this change, please file a CSR for it.

Thanks,

-Joe


On 12/2/2018 6:24 PM, Vicente Romero wrote:
> Hi Liam,
>
> Thanks for testing the change. Yes the change you found is an expected 
> result of the fix. If the number of rejected examples is not big I 
> think that we could go on with the fix. Eclipse compiler should be 
> rejecting that code too.
>
> Vicente
>
> On 12/2/18 4:01 PM, Liam Miller-Cushon wrote:
>> Hi Vicente,
>>
>> I did some testing with your fix to get a sense of the source 
>> compatibility impact, and it looks minor: I only found a handful of 
>> examples that are rejected after the change.
>>
>> One of them is in icu4j. Is this change an expected result of the fix?
>>
>> icu/icu4j/main/classes/core/src/com/ibm/icu/impl/locale/XLikelySubtags.java:47: 
>> error: make() in <anonymous 
>> com.ibm.icu.impl.locale.XLikelySubtags$Maker$1> cannot override 
>> <V>make() in Maker
>>              public Map<Object,Object> make() {
>>                                        ^
>>    return type Map<Object,Object> is not compatible with V
>>    where V is a type-variable:
>>      V extends Object declared in method <V>make()
>> ...
>> https://github.com/unicode-org/icu/blob/master/icu4j/main/classes/core/src/com/ibm/icu/impl/locale/XLikelySubtags.java#L47
>>
>> Thanks,
>>
>> On Sat, Dec 1, 2018 at 11:47 AM Vicente Romero 
>> <vicente.romero at oracle.com <mailto:vicente.romero at oracle.com>> wrote:
>>
>>     Please review fix for [1] at [2]. Javac is accepting this program
>>     as valid:
>>
>>     class ReturnTypeSubstitutableTest {
>>         abstract class AbstractDemo<Request extends AbstractResult,
>>     Response extends AbstractResult> {
>>             protected abstract Response test(Request request);
>>         }
>>
>>         abstract interface AbstractResult {}
>>
>>         abstract interface SimpleResult extends AbstractResult {}
>>
>>         class Result1 implements SimpleResult {}
>>
>>         class OtherResult implements AbstractResult {}
>>
>>         public class SimpleDemo<Request extends AbstractResult,
>>     Response extends AbstractResult> extends AbstractDemo<Request,
>>     Response> {
>>             @Override
>>             protected SimpleResult test(AbstractResult request)
>>     {<----------- this method is accepted as a good override for
>>     AbstractDemo::test
>>                 return null;
>>             }
>>         }
>>     }
>>
>>     From Dan's evaluation at the bug entry [1]:
>>
>>     Per JLS 8.4.8.3, the first method must be
>>     return-type-substitutable for the second. This can be satisfied
>>     in 3 ways (JLS 8.4.5):
>>     - SimpleResult is a subtype of Response (no)
>>     - SimpleResult can be converted to a subtype of Response by
>>     unchecked conversion (no)
>>     - The methods have different signatures (yep) and SimpleResult =
>>     erasure(Response) (no)
>>
>>     No case applies, so an error should occur. This fix syncs javac
>>     with the spec.
>>
>>     Thanks,
>>     Vicente
>>
>>     [1] https://bugs.openjdk.java.net/browse/JDK-8207224
>>     [2] http://cr.openjdk.java.net/~vromero/8207224/webrev.00/
>>     <http://cr.openjdk.java.net/%7Evromero/8207224/webrev.00/>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20181203/f2241733/attachment.html>