REASSERT Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed
Joe Darcy
joe.darcy at oracle.com
Tue Apr 23 05:51:04 UTC 2013
Hi David,
On 04/22/2013 10:25 PM, David Holmes wrote:
> Hi Joe,
>
> On 23/04/2013 9:05 AM, Joseph Darcy wrote:
>> Hello,
>>
>> Just reasserting the request for a review of the latest version of this
>> patch:
>>
>> http://cr.openjdk.java.net/~darcy/8012044.2
>>
>> I believe this version does an appropriate job of propagating exception
>> information when there is misuse of the methods on Throwable.
>
> I still find the use of addSuppressed in initCause to be questionable.
> Given:
>
> catch(SomeException s) {
> sharedException.initCause(s); // oops already has a cause
> throw sharedException;
> }
>
> then the ISE isn't suppressing 's', but replacing/suppressing
> sharedException in my view, as it is what would have been thrown had
> the ISE not occurred.
>
> I understand the desire to not lose sight of the fact that 's' was
> thrown, but this is really no different to a finally block losing the
> original exception info. (And fixing that was rejected when the
> suppression mechanism was added.)
Project Coin discussions did note try-catch-finally and
try-with-resources were inconsistent on this point. While the
try-with-resources behavior is regarded as preferable, we thought it
would be too large a change to redefine the long-standing semantics of
try-catch-finally.
>
> Anyway this isn't a "block" (not that such a thing exists), just a
> comment. The change isn't harmful and may be useful.
>
> Cheers,
> David
>
Yes, I would describe the intention of this change as provding
programmers more information to debug when the methods are Throwable and
used improperly.
Thanks,
-Joe
More information about the core-libs-dev
mailing list