Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

try/catch catches NonLocalReturnException #2807

Closed
scabug opened this issue Dec 16, 2009 · 12 comments
Closed

try/catch catches NonLocalReturnException #2807

scabug opened this issue Dec 16, 2009 · 12 comments

Comments

@scabug
Copy link

scabug commented Dec 16, 2009

Currently if I write some code like this:

for(i <- 0 to 20){
   try{
      if(isGood(i)) return i
   }catch{
      case e => {
         System.out.println("Exception occurred. Continuing")
      }
   }
}

Then the catch statement ends up catching the value returned by the return statement.

This is deeply confusing...

Would there be a possible workaround for this? E.g. could we declare that, by default, a pattern for an exception does not match NonLocalReturnException?

It would at least be nice to have some prominent documentation warning users about this problem.

@scabug
Copy link
Author

scabug commented Dec 16, 2009

Imported From: https://issues.scala-lang.org/browse/SI-2807?orig=1
Reporter: Rob Ennals (robennals)

@scabug
Copy link
Author

scabug commented Dec 16, 2009

@paulp said:
You can find lots of messages from me over the last year pointing out that this is a problem. Special casing that one specific exception is a non-starter. More plausible is special casing scala.util.control.ControlException, from which that and others are derived. However that's not a very good solution either.

I want to make case e => or case _ => ... compile time errors. If you really want to catch Throwable (which you basically never should) you would have to say case e: Throwable. However before I would push for that I really need to get scala.util.control.Exception into a more finished state.

By the way I put a warning in to complain about these a while ago.

% scala -Ywarn-catches

scala> // your code here

<console>:14: warning: catch clause swallows everything: not advised.
               case e => {
               ^

@scabug
Copy link
Author

scabug commented Jan 5, 2010

@hubertp said:
It is really hard to come up with something simple that would solve the problem. Possibly we could detect for return in one of those special cases but again this is not trivial. For now assigning to community. Any ideas are welcome.

@scabug
Copy link
Author

scabug commented Jan 6, 2010

Rob Ennals (robennals) said:
Perhaps the best solution would be to leave the behaviour as it is now, but document it well, include a link to the documentation in the error message for the exception, and maybe also have the compiler warn if it sees a return inside a wildcarded try/catch?

-Rob

@scabug
Copy link
Author

scabug commented Feb 28, 2010

@ijuma said:
Even though the original report mentions a catch-all exception handler, NonLocalReturnException inherits from RuntimeException so it's even worse.

I'd suggest changing:

class NonLocalReturnException[T](val key: AnyRef, val value: T) extends RuntimeException with ControlException

to

class NonLocalReturnException[T](val key: AnyRef, val value: T) extends ControlException

That would be an improvement over the current situation.

@scabug
Copy link
Author

scabug commented Mar 5, 2010

@paulp said:
In r21058 I did a big cleanup. Pretty much every control flow object now inherits from ControlThrowable, and not from Error, Exception, or god forbid, RuntimeException. My preferred next step is to make "case _ => ..." a compile time error in catch blocks, and/or to automatically generate a case which rethrows control exceptions in front of user written catch blocks unless some affirmative action is taken to inhibit it.

Martin said he's take it under advisement but naturally he resists because it introduces non-orthogonality between regular pattern matches and catches.

@scabug
Copy link
Author

scabug commented Jun 23, 2012

@retronym said:
Baby steps: This now emits a warning.

try x catch { case _ => }

scala/scala@cfc0e18

@scabug
Copy link
Author

scabug commented Jun 29, 2012

@dragos said:
The warning is a bit too eager, see my comment on the pull request.

@scabug
Copy link
Author

scabug commented Jul 2, 2012

@adriaanm said:
scala/scala#798

@scabug
Copy link
Author

scabug commented Jul 2, 2012

@retronym said:
This ticket has grander plans than a simple warning, so I'm going to leave it open.

@scabug
Copy link
Author

scabug commented Jul 2, 2012

@adriaanm said:
oh, sorry about that

@SethTisue
Copy link
Member

something in this area could be revived on https://contributors.scala-lang.org

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants