[
https://issues.apache.org/jira/browse/LUCENE-7800?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15988610#comment-15988610
]
Dawid Weiss edited comment on LUCENE-7800 at 4/28/17 10:32 AM:
---------------------------------------------------------------
Moving the discussion here from LUCENE-7796. Uwe, I don't think this code is
actually any cleaner than the existing sneaky throws:
{code}
@FunctionalInterface
public interface Unchecked<R,T extends Throwable> {
R call() throws T;
@SuppressWarnings("unchecked")
static <R> R exec(Unchecked<R,?> lambda) {
return ((Unchecked<R,RuntimeException>) lambda).call();
}
}
{code}
Sure, it's cleaner to call, but the fact that you can cast {{Unchecked<R,?>}}
to {{Unchecked<R, RuntimeException>}} is a fault of the type system/ erasure --
to me it's still the same argument as for sneaky throw: it catches you by
surprise with a potential of getting a checked exception from java code that
invokes it and has no type system-safe way of reacting to it. Sure, it's
permitted by the JVM, but it's still an abuse of the holes in Java's type
system definition.
Let me put it this way: do you think there are benefits of *not* wrapping the
(checked) exception thrown by such code? I can think of one -- you'd get one
fewer "caused by" in the stack trace, should you dump it somewhere. But other
than that nothing comes to my mind. Hell, it's even worse -- hacks like this
one completely break the type system's utility. Look at this code that uses the
above:
{code}
try {
Unchecked.exec(() -> {
throw new IOException();
});
} catch (IOException e) {
// bam, doesn't compile.
}
{code}
This catches the IOException propagated directly from exec... but it won't
compile because the type system doesn't "see" any checked exception thrown from
{{exec}}. See my point?
was (Author: dweiss):
Moving the discussion here from LUCENE-7796. Uwe, I don't think this code is
actually any cleaner than the existing sneaky throws:
{code}
@FunctionalInterface
public interface Unchecked<R,T extends Throwable> {
R call() throws T;
@SuppressWarnings("unchecked")
static <R> R exec(Unchecked<R,?> lambda) {
return ((Unchecked<R,RuntimeException>) lambda).call();
}
}
{code}
Sure, it's cleaner to call, but the fact that you can cast {{Unchecked<R,?>}}
to {{Unchecked<R, RuntimeException>}} is a fault of the type system/ erasure --
to me it's still the same argument as for sneaky throw: it catches you by
surprise with a potential of getting a checked exception from java code that
invokes it and has no type system-safe way of reacting to it. Sure, it's
permitted by the JVM, but it's still an abuse of the holes in Java's type
system definition.
Let me put it this way: do you think there are benefits of *not* wrapping the
(checked) exception thrown by such code? I can think of one -- you'd get one
fewer "caused by" in the stack trace, should you dump it somewhere. But other
than that nothing comes to my mind. Hell, it's even worse -- hacks like this
one completely break the type system's utility. Look at this code that uses the
above:
{code}
try {
Unchecked.exec(() -> {
throw new IOException();
});
} catch (IOException e) {
// bam, doesn't compile.
}
{code}
This catches the IOException propagated directly from exec... but it won't
compiler because the type system doesn't "see" any checked exception thrown
from {{exec}}. See my point?
> Remove code that potentially rethrows checked exceptions from methods that
> don't declare them ("sneaky throw" hack)
> -------------------------------------------------------------------------------------------------------------------
>
> Key: LUCENE-7800
> URL: https://issues.apache.org/jira/browse/LUCENE-7800
> Project: Lucene - Core
> Issue Type: Task
> Reporter: Dawid Weiss
> Priority: Minor
> Fix For: 6.x, master (7.0)
>
> Attachments: LUCENE-7800.patch
>
>
> For a long time I considered the "sneaky" throw hack to be a nice way of
> coding around some of Java's limitations (especially with invoking methods
> via reflection or method handles), but with time I started to see how it can
> be potentially dangerous and is nearly always confusing. If you have a Java
> method and its signature doesn't indicate the possibility of a checked
> exception you, as a programmer, simply don't expect it to happen. Never. So,
> for example, you could write:
> {code}
> try {
> luceneApi();
> } catch (RuntimeException | Error e) {
> // Handle unchecked exceptions here.
> }
> {code}
> and consider the code above to be absolutely bullet-proof in terms of
> handling exceptions. Unfortunately with sneaky throws anywhere in the
> "luceneApi" this is no longer the case -- you can receive a checked exception
> that will simply fall through and hit some code frame above.
> So I suggest we remove sneaky throw from the core entirely. It only exists in
> two places -- private methods inside Snowball programs invoked via method
> handles (these don't even declare checked exceptions so I assume they can't
> occur) and AttributeFactory -- here there is a real possibility somebody
> could declare an attribute class's constructor that does throw an unchecked
> exception. In that case I think it is more reasonable to wrap it in a
> RuntimeException than rethrow it as original.
> Alternatively, we can modify the signature of {{createAttributeInstance}} and
> {{getStaticImplementation}} to declare some kind of checked exception (either
> a wrapper or even a Throwable), but I see little reason for it and it'd
> change the API.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
