JDK-8214729 : Javac compiles source code despite illegal use of unchecked conversions
  • Type: CSR
  • Component: tools
  • Sub-Component: javac
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 13
  • Submitted: 2018-12-03
  • Updated: 2018-12-18
  • Resolved: 2018-12-18
Related Reports
CSR :  
Description
Summary
-------

Javac should be sync with a section of JLS 11 ��8.4.5

Problem
-------

Javac is not in sync with JLS 11 ��8.4.5. See details below in the specification section.

Solution
--------

Synchronize javac with the spec. But in order to reduce possible compatibility issues, the new approach will only be executable for source >= 13

Specification
-------------

Per JLS 11 ��8.4.8.3, when analyzing if a method can override another one, the first method must be return-type-substitutable for the second. And according to JLS 8.4.5:

A method declaration d1 with return type R1 is return-type-substitutable for another method d2 with return type R2 iff any of the following is true, the following apply to the case in which R1 is a reference type:

* R1 , adapted to the type parameters of d2 (��8.4.4), is a subtype of R2.

* R1 can be converted to a subtype of R2 by unchecked conversion (��5.1.9)

* d1 does not have the same signature as d2 (��8.4.2), and R1 = | R2 |

Javac is implementing the last assertion incorrectly as it is testing for subtyping (R1 <: | R2 | ) instead of type equality which is what the spec is enforcing.

The actual fix to the compiler is rather simple:

    --- old/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java	2018-12-03 18:26:38.765597146 -0500
    +++ new/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java	2018-12-03 18:26:38.469597132 -0500
    @@ -185,7 +185,8 @@
             SWITCH_MULTIPLE_CASE_LABELS(JDK12, Fragments.FeatureMultipleCaseLabels, DiagKind.PLURAL),
             SWITCH_RULE(JDK12, Fragments.FeatureSwitchRules, DiagKind.PLURAL),
             SWITCH_EXPRESSION(JDK12, Fragments.FeatureSwitchExpressions, DiagKind.PLURAL),
    -        RAW_STRING_LITERALS(JDK12, Fragments.FeatureRawStringLiterals, DiagKind.PLURAL);
    +        RAW_STRING_LITERALS(JDK12, Fragments.FeatureRawStringLiterals, DiagKind.PLURAL),
    +        RETURN_TYPE_SUBSTITUTABLE_SAME_TYPE(JDK12);

             enum DiagKind {
                 NORMAL,
    --- old/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java	2018-12-03 18:26:39.689597191 -0500
    +++ new/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java	2018-12-03 18:26:39.393597177 -0500
    @@ -91,6 +91,7 @@
         final Names names;
         final boolean allowDefaultMethods;
         final boolean mapCapturesToBounds;
    +    final boolean returnTypeSustitutableSameType;
         final Check chk;
         final Enter enter;
         JCDiagnostic.Factory diags;
    @@ -114,6 +115,7 @@
             Source source = Source.instance(context);
             allowDefaultMethods = Feature.DEFAULT_METHODS.allowedInSource(source);
             mapCapturesToBounds = Feature.MAP_CAPTURES_TO_BOUNDS.allowedInSource(source);
    +        returnTypeSustitutableSameType = Feature.RETURN_TYPE_SUBSTITUTABLE_SAME_TYPE.allowedInSource(source);
             chk = Check.instance(context);
             enter = Enter.instance(context);
             capturedName = names.fromString("<captured wildcard>");
    @@ -4235,8 +4237,13 @@
                 return covariantReturnType(r1.getReturnType(), r2res, warner);
             if (isSubtypeUnchecked(r1.getReturnType(), r2res, warner))
                 return true;
    -        if (!isSubtype(r1.getReturnType(), erasure(r2res)))
    -            return false;
    +        if (returnTypeSustitutableSameType) {
    +            if (!isSameType(r1.getReturnType(), erasure(r2res)))
    +                return false;
    +        } else {
    +            if (!isSubtype(r1.getReturnType(), erasure(r2res)))
    +                return false;
    +        }
             warner.warn(LintCategory.UNCHECKED);
             return true;
         }

References
-------------

http://cr.openjdk.java.net/~vromero/8207224/webrev.01


Comments
Changing the solution section to be applied if the source is >= 13 rather than >= 12 since the CSR is now against JDK 13. Please monitor the reaction to this change after it goes back, webbugs and other bug reports, etc. and add a release note if warranted. Moving to Approved.
18-12-2018

[~darcy] I have sent another iteration of the webrev guarding the new code with a check on the source level
03-12-2018

On compiler-dev review thread (http://mail.openjdk.java.net/pipermail/compiler-dev/2018-December/012690.html), there is some discussion about this change breaking compilation. In the absence of crisp data about the frequency of this breakage and the closeness of JDK 12 rampdown one, I'd prefer this change be controlled by the -source/--release level (as mentioned on the review thread) or deferred until JDK 13. Moving to Provisional.
03-12-2018