javac warnings for multi-catch

[Gernot Neppert]

Interesting observation!
One might argue that the compiler warning points to a flaw in the
original code: even though it 'works' as intended,
it's not really 'clean code'. The author obviously did not trust the
official API of task.run().
If he had, catching RuntimeException and Error would have sufficed.
Rather, he wanted to catch 'sneaky' checked Exceptions, too.
So he used 'catch Throwable' to cover for that, even though there are
no Throwable-derivatives other than 'Error' and 'Exception'.
To sum it up, I'd suggest the following code as a better, more concise
way of expressing the intent:

try {
           task.run();
     } catch (Exception x) {
                  thrown = x; throw x;
     } catch (Error x) {
                  thrown = x; throw x;
     } finally {
                 afterExecute(task, thrown);
     }

Does the Java-7 compiler issue a warning for this, too?


2011/4/26 David Holmes <David.Holmes at oracle.com>:
> I have a query regarding the warnings that javac now issues that are
> related to the work on multi-catch that does more precise analysis of
> the exceptions that can be thrown. Here's an example:
>
> ../../../src/share/classes/java/util/concurrent/ThreadPoolExecutor.java:1115:
> warning: unreachable catch clause
>                     } catch (Throwable x) {
>                       ^
>   thrown types RuntimeException,Error have already been caught
>
> and here's the code:
>
>                     try {
>                         task.run();
>                     } catch (RuntimeException x) {
>                         thrown = x; throw x;
>                     } catch (Error x) {
>                         thrown = x; throw x;
>                     } catch (Throwable x) {
>                         thrown = x; throw new Error(x);
>                     } finally {
>                         afterExecute(task, thrown);
>                     }
>
> This pattern is not uncommon. The final catch of Throwable is there to
> close the gap that exists because of the API's (Class.newInstance) that
> allow you to throw checked-exceptions from methods that don't declare
> them. So the catch clause is not in fact unreachable.
>
> It seems to me that these warnings will encourage people to remove the
> "offending" catch clause and actually end up with code that is not
> robust because of these historical flaws in the API's (not to mention
> native calls).
>
> Perhaps it would be better to restrict the warnings to the cases where a
> previous clause will catch the exception expected by the later clause,
> rather than incorrectly assuming which exceptions can be thrown from the
> methods in the try-block.
>
> David
>
>