lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dawid Weiss (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-7800) Remove code that potentially rethrows checked exceptions from methods that don't declare them ("sneaky throw" hack)
Date Fri, 28 Apr 2017 10:33:04 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-7800?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15988610#comment-15988610
] 

Dawid Weiss commented on LUCENE-7800:
-------------------------------------

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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message