Maurizio Cimadamore said the following on 04/27/11 18:10:
On 27/04/11 09:05, Maurizio Cimadamore wrote:
On 27/04/11 03:42, David Holmes wrote:
I sent email to coin-dev on this yesterday but it doesn't seem to
have turned up - at least I haven't seen it.
In some cases at least these catch clauses are in fact reachable -
e.g. ThreadPoolExecutor. The analysis assumes that checked-exceptions
can only be thrown by methods that declare them, but we all know that
is not the case: Class.newInstance and native methods can throw them
too (are there other cases?). So sometimes we want the "unreachable"
catch as a "safety-net" for these cases.
Given checked-exception checks can be circumvented it seems to me
that the analysis should not be issuing warnings for these cases
because it will encourage people to "fix the code" - as you have just
asked to be done! - when in fact the code could be less robust if it
is changed.
I see your concerns - unfortunately there is no way to statically
distinguish 'good' cases from 'bad' cases - i.e. a call to
Class.newInstance could be hidden behind a method called inside the
try-block. So, it seems like the only sensible option would be to
special case 'catch(Throwable)'.
Btw - it seems like both Constructor and Method throws
InvocationTargetException in order to wrap checked exceptions from user
code. Why Class.newInstance does not use ITE ? That'd be for
compatibility, right?
Right - Class.newInstance predates the full reflection API and the
powers that be at the time refused to change it for "compatibility
concerns" even though it violated the language semantics. :( It took
some effort to even get the problem clearly documented in the method.
Because of that and the problem of native code, any framework code
wanting to ensure that it dealt with all exceptions, has been forced to
use the kind of pattern we are discussing - and similar ones for
rethrowing exceptions (by wrapping any unexpected checked-exception in
an Error).
For some reason "they" never even considered deprecating
Class.newInstance so that people would at least be steered to use the
reflection APIs. Though given deprecated APIs never actually disappear
it makes little practical difference.
David
Maurizio
Maurizio
Cheers,
David
Joe Darcy said the following on 04/27/11 11:53:
Hello.
FYI, javac in JDK 7 does a more precise analysis than before on what
catch blocks can actually be reached and warnings are issued for
unreachable catch blocks. I've extracted those warnings from a
recent full JDK build in case anyone wants to update the code in
question.
-Joe
../../../../../../src/share/classes/com/sun/corba/se/impl/protocol/BootstrapServerRequestDispatcher.java:124:
warning: unreachable catch clause
} catch (java.lang.Exception ex) {
^
thrown type RuntimeException has already been caught
../../../../../../src/share/classes/com/sun/corba/se/impl/protocol/BootstrapServerRequestDispatcher.java:124:
warning: unreachable catch clause
} catch (java.lang.Exception ex) {
^
thrown type RuntimeException has already been caught
../../../src/share/classes/java/util/concurrent/ThreadPoolExecutor.java:1115:
warning: unreachable catch clause
} catch (Throwable x) {
^
thrown types RuntimeException,Error have already been caught
../../../src/share/classes/java/net/DatagramSocket.java:183:
warning: unreachable catch clause
} catch(IOException e) {
^
thrown type SocketException has already been caught
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
2 warnings
../../../src/share/classes/java/security/cert/X509CertSelector.java:2217:
warning: unreachable catch clause
} catch (CertificateException e3) {
^
thrown types
CertificateNotYetValidException,CertificateExpiredException have
already been caught
../../../src/share/classes/javax/management/modelmbean/RequiredModelMBean.java:1220:
warning: unreachable catch clause
} catch (Exception e) {
^
thrown types
InvocationTargetException,IllegalAccessException,RuntimeException
have already been caught
1 warning
../../../../src/share/classes/sun/security/rsa/RSASignature.java:205:
warning: unreachable catch clause
} catch (GeneralSecurityException e) {
^
thrown type BadPaddingException has already been caught
