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

[Vicente Romero]

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/
>

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