Reducing "exception is never thrown in body of corresponding try statement" Error to a Warning

[Vitaly Davidovich]

Hi Mike,

On Sun, Oct 20, 2019 at 6:36 PM Mike Duigou <openjdk at duigou.org> wrote:

> Hello all;
>
> As part of my Oracle CodeOne talk "Exceptions 2020" I made two proposals
> for improving Java exceptions. One of the proposals was to remove the
> compile time error produced when and exception is caught that is not
> thrown by the enclosed statements. Specifically I am proposing the
> removal of the following section from JLS 11.2.3
>
>      "It is a compile-time error if a catch clause can catch checked
> exception class E1
>       and it is not the case that the try block corresponding to the
> catch clause can
>       throw a checked exception class that is a subclass or superclass of
> E1, unless E1
>       is Exception or a superclass of Exception."
>
> The advantage of removal of this rule is that removal allows API authors
> to better document the actual exceptions thrown by constructors and
> methods. Vestigial or inaccurate throws specifications can be eliminated
> without "breaking" calling code. This makes it easier to evolve the API
> by narrowing (including to empty), the exception specification of method
> without impacting existing code which uses the API.

Altering the checked exception throws clause is a contract change of the
method/ctor.  It’s not too dissimilar from changing the return value
(afterall, exceptions are just a side channel for return values).

I understand that people, in general, have a harder time designing checked
exception APIs “properly”, and thus this type of churn is more likely there
than normal method return types.  But, given it’s a contract change
nonetheless, I don’t see how weakening the check to a warning results in
better/robust/bug-free/etc code.  It sure makes the author’s life easier,
but that’s likely not where the focus should be.

>
>
> To allow application authors to clean up dead code an equivalent warning
> would still be provided and probably should be encouraged by the JLS
> specification. The warning would alert them that the catch clause will
> not be executed.

A good amount of projects I’ve worked on build with -Werror.  As such, a
hard error vs warning doesn’t matter - it’s a disruption either way, but as
mentioned above, a worthwhile one (IMO).

Given the JVM itself doesn’t care/know about checked exceptions, such
changes break source compatibility but allow binaries (bytecode) to
continue running.  That seems like the right compatibility knob, at least
given today’s Java.

>
>
> As an API author I have been frustrated many times by being unable to
> reduce or remove a method's exception specification without causing
> compatibility issues for my users and I have also been frustrated when
> libraries did choose to alter their method exception specifications and
> "broke" my code which caught the no longer thrown exceptions. Does this
> seem like a feature/change worth pursuing?
>
> Cheers,
>
> Mike
>
> The changes to javac needed to accomplish this change are quite simple:
>
> diff --git
> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
> index 0c1fead9a4..f7793d0605 100644
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
> @@ -1271,7 +1277,7 @@ public class Flow {
>               } else if (!chk.isUnchecked(pos, exc) &&
>                       !isExceptionOrThrowable(exc) &&
>                       !chk.intersects(exc, thrownInTry)) {
> -                log.error(pos, Errors.ExceptNeverThrownInTry(exc));
> +                log.warning(pos, Warnings.ExceptNeverThrownInTry(exc));
>               } else {
>                   List<Type> catchableThrownTypes =
> chk.intersect(List.of(exc), thrownInTry);
>                   // 'catchableThrownTypes' cannnot possibly be empty -
> if 'exc' was an
> diff --git
> a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
>
>
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> index 1f29df0704..e29b5583e4 100644
> ---
>
> a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> +++
>
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> @@ -533,10 +533,6 @@ compiler.err.error.reading.file=\
>   compiler.err.except.already.caught=\
>       exception {0} has already been caught
>
> -# 0: type
> -compiler.err.except.never.thrown.in.try=\
> -    exception {0} is never thrown in body of corresponding try
> statement
> -
>   # 0: symbol
>   compiler.err.final.parameter.may.not.be.assigned=\
>       final parameter {0} may not be assigned
> @@ -2019,6 +2015,14 @@ compiler.warn.unchecked.generic.array.creation=\
>   compiler.warn.unchecked.varargs.non.reifiable.type=\
>       Possible heap pollution from parameterized vararg type {0}
>
> +# 0: type
> +compiler.warn.except.never.thrown.in.try=\
> +    exception {0} is never thrown in body of corresponding try
> statement
> +
>   # 0: symbol
>   compiler.warn.varargs.unsafe.use.varargs.param=\
>       Varargs method could cause heap pollution from non-reifiable
> varargs parameter {0}
>
-- 
Sent from my phone