-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Stop catching Throwable/IIOBE/NPE in the compiler, outside of stack overflow handling #25832
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
Changes from all commits
cc82a01
ee2a584
c0a2621
b5d523e
e0b60d0
b40ebeb
82ba6b4
f681edf
86d5ef0
6a78b72
fb22933
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,10 +85,7 @@ class CodeGen(val backendUtils: BackendUtils, val primitives: ScalaPrimitives, v | |
| registerGeneratedClass(mainClassNode, isArtifact = false) | ||
| registerGeneratedClass(mirrorClassNode, isArtifact = true) | ||
| catch | ||
| case ex: InterruptedException => throw ex | ||
| case ex: CompilationUnit.SuspendException => throw ex | ||
| case ex: Throwable => | ||
| if !ex.isInstanceOf[TypeError] then ex.printStackTrace() | ||
| case ex: TypeError => | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given that we were willing to print a stack trace for non-typeerrors, I think letting it bubble up to "compiler bug" is better
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: double check that if something bad happens in the backend we get the usual compiler "plz file a bug" message (says Seb)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, we do |
||
| report.error(s"Error while emitting ${unit.source}\n${ex.getMessage}", cd.sourcePos) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,6 @@ import annotation.tailrec | |
| import util.SimpleIdentityMap | ||
| import util.Stats | ||
| import java.util.WeakHashMap | ||
| import scala.util.control.NonFatal | ||
| import config.Config | ||
| import reporting.* | ||
| import collection.mutable | ||
|
|
@@ -2388,7 +2387,7 @@ object SymDenotations { | |
| } | ||
| } | ||
| catch { | ||
| case ex: Throwable => | ||
| case ex: Exception => | ||
| tp match | ||
| case tp: CachedType => btrCache.remove(tp) | ||
| case _ => | ||
|
|
@@ -2429,8 +2428,7 @@ object SymDenotations { | |
| names | ||
| } | ||
| catch { | ||
| case ex: Throwable => | ||
| handleRecursive("member names", i"of $this", ex) | ||
| case ex: Throwable => handleRecursive("member names", i"of $this", ex) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the formatting for the handleRecursives where it was easy to do so they're easier to look at in search results, since I'll deal with those in the next step |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -2618,7 +2616,7 @@ object SymDenotations { | |
| // since the older file might have been loaded from a jar earlier in the | ||
| // classpath. | ||
| def sameContainer(f: AbstractFile): Boolean = | ||
| try f.container == chosen.container catch case NonFatal(ex) => true | ||
| try f.container == chosen.container catch case ex: Exception => true | ||
| if !ambiguityWarningIssued then | ||
| for conflicting <- assocFiles.find(!sameContainer(_)) do | ||
| report.warning(em"""${ambiguousFilesMsg(conflicting)} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -393,15 +393,17 @@ class TypeApplications(val self: Type) extends AnyVal { | |
| case _ => false | ||
| } | ||
| } | ||
| if ((dealiased eq stripped) || followAlias) | ||
| try | ||
| val instantiated = dealiased.instantiate(args) | ||
| if (followAlias) instantiated.normalized else instantiated | ||
| catch | ||
| case ex: IndexOutOfBoundsException => | ||
| AppliedType(self, args) | ||
| case ex: Throwable => | ||
| handleRecursive("try to instantiate", i"$dealiased[$args%, %]", ex) | ||
| if (dealiased eq stripped) || followAlias then | ||
| val paramsWithoutArg = dealiased.typeParams.drop(args.length).map(_.paramRef) | ||
| val hasParamsWithoutArg = paramsWithoutArg.nonEmpty && dealiased.resType.existsPart(paramsWithoutArg.contains, forceLazy = false) | ||
| if hasParamsWithoutArg then | ||
| AppliedType(self, args) | ||
| else | ||
| try | ||
| val instantiated = dealiased.instantiate(args) | ||
| if (followAlias) instantiated.normalized else instantiated | ||
| catch | ||
| case ex: Throwable => handleRecursive("try to instantiate", i"$dealiased[$args%, %]", ex) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the only non-mechanical change, I had to add a specific guard for what the |
||
|
|
||
| else AppliedType(self, args) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the IO-related ones I followed the Javadoc, and in one case removed a catch because it was calling one of our own util methods that already catches and ignores internally