diff --git a/compiler/src/dotty/tools/dotc/Driver.scala b/compiler/src/dotty/tools/dotc/Driver.scala index a07ea754bf90..4e94c3e80660 100644 --- a/compiler/src/dotty/tools/dotc/Driver.scala +++ b/compiler/src/dotty/tools/dotc/Driver.scala @@ -12,6 +12,8 @@ import core.Decorators.* import util.chaining.* import fromtasty.{TASTYCompiler, TastyFileUtil} +import scala.annotation.nowarn + /** Run the Dotty compiler. * * Extending this class lets you customize many aspect of the compilation @@ -26,6 +28,7 @@ class Driver { protected def emptyReporter: Reporter = new StoreReporter(null) + @nowarn("msg=Catching StackOverflowError can lead to unexpected behavior") // yes, but we immediately exit protected def doCompile(compiler: Compiler, files: List[AbstractFile])(using Context): Reporter = if files.nonEmpty then var runOrNull = ctx.run diff --git a/compiler/src/dotty/tools/dotc/ast/Positioned.scala b/compiler/src/dotty/tools/dotc/ast/Positioned.scala index adfe369fae2b..5c2fbed5f3e8 100644 --- a/compiler/src/dotty/tools/dotc/ast/Positioned.scala +++ b/compiler/src/dotty/tools/dotc/ast/Positioned.scala @@ -165,7 +165,7 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Src * - Parent spans contain child spans * - If item is a non-empty tree, it has a position */ - def checkPos(nonOverlapping: Boolean)(using Context): Unit = try { + def checkPos(nonOverlapping: Boolean)(using Context): Unit = printOnAssertionError(i"error while checking $this") { import untpd.* val last = LastPosRef() def check(p: Any): Unit = p match { @@ -231,11 +231,6 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Src } } } - catch { - case ex: AssertionError => - println(i"error while checking $this") - throw ex - } } object Positioned { diff --git a/compiler/src/dotty/tools/dotc/cc/Capability.scala b/compiler/src/dotty/tools/dotc/cc/Capability.scala index 687ddf42dbab..45ac62521acc 100644 --- a/compiler/src/dotty/tools/dotc/cc/Capability.scala +++ b/compiler/src/dotty/tools/dotc/cc/Capability.scala @@ -823,7 +823,8 @@ object Capabilities: case info: OrType => viaInfo(info.tp1)(test) && viaInfo(info.tp2)(test) case _ => false - try (this eq y) + printOnAssertionError(i"error while subsumes $this >> $y"): + (this eq y) || maxSubsumes(y, canAddHidden = !vs.isOpen) // if vs is open, we should add new elements to the set containing `this` // instead of adding them to the hidden set of of `this`. @@ -882,9 +883,6 @@ object Capabilities: case x: ThisType if x.cls.is(Module) => x.cls.sourceModule.termRef.subsumes(y) case _ => false - catch case ex: AssertionError => - println(i"error while subsumes $this >> $y") - throw ex end subsumes /** This is a maximal capability that subsumes `y` in given context and VarState. diff --git a/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala b/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala index f8ef2acd40d9..7c586fd5d1e5 100644 --- a/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala +++ b/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala @@ -312,11 +312,8 @@ sealed abstract class CaptureSet extends Showable: capt.println(i"WIDEN ro $this with ${this.mutability} <:< $that with ${that.mutability} to $this1") this1.subCaptures(that, vs) else - try + printOnAssertionError(i"error while subcap $this <:< $that"): that.tryInclude(elems, this) && addDependent(that) - catch case ex: AssertionError => - println(i"error while subcap $this <:< $that") - throw ex /** Two capture sets are considered =:= equal if they mutually subcapture each other * in a frozen state. @@ -895,10 +892,8 @@ object CaptureSet: // id == 108 then assert(false, i"trying to add $elem to $this") assert(elem.isWellformed, elem) assert(!this.isInstanceOf[HiddenSet] || summon[VarState].isSeparating, summon[VarState]) - try includeElem(elem) - catch case ex: AssertionError => - println(i"error for incl $elem in $this, ${summon[VarState].toString}") - throw ex + printOnAssertionError(i"error for incl $elem in $this, ${summon[VarState].toString}"): + includeElem(elem) newElemAddedHandlers.foreach(_(elem)) val normElem = if isMaybeSet then elem else elem.stripMaybe // assert(id != 5 || elems.size != 3, this) @@ -1187,12 +1182,9 @@ object CaptureSet: else // Propagate backwards to source. The element will be added then by another // forward propagation from source that hits the first branch `if origin eq source then`. - try + printOnAssertionError(i"fail while prop backwards tryInclude $elem of ${elem.getClass} from $this # $id / ${this.summarize} to $source # ${source.id}"): reporting.trace(i"prop backwards $elem from $this # $id to $source # ${source.id} via $summarize"): source.tryInclude(bimap.inverse.mapCapability(elem), this) - catch case ex: AssertionError => - println(i"fail while prop backwards tryInclude $elem of ${elem.getClass} from $this # $id / ${this.summarize} to $source # ${source.id}") - throw ex /** For a BiTypeMap, supertypes of the mapped type also constrain * the source via the inverse type mapping and vice versa. That is, if diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index f0bad6e1a4e5..e04b61b53c37 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -1660,16 +1660,14 @@ class CheckCaptures extends Recheck, SymTransformer: CaptureSet.Var(curEnv.owner), curEnv) case _ => val res = - try - if capt eq noPrinter then - super.recheck(tree, pt) - else - trace.force(i"rechecking $tree with pt = $pt", recheckr, show = true): + printOnAssertionError(i"error while rechecking $tree against $pt"): + try + if capt eq noPrinter then super.recheck(tree, pt) - catch case ex: AssertionError => - println(i"error while rechecking $tree against $pt") - throw ex - finally curEnv = saved + else + trace.force(i"rechecking $tree with pt = $pt", recheckr, show = true): + super.recheck(tree, pt) + finally curEnv = saved if tree.isTerm && !pt.isBoxed && pt != LhsProto then markFree(res.boxedCaptureSet, tree) res @@ -1720,16 +1718,14 @@ class CheckCaptures extends Recheck, SymTransformer: */ override def checkConformsExpr(actual: Type, expected: Type, tree: Tree, notes: List[Note])(using Context): Type = val saved = ccState.ignoreClassifiers - try - tree match - case tree: TypeApply if tree.symbol == defn.Any_typeCast => ccState.ignoreClassifiers = true - case _ => - testAdapted(actual, expected, tree, notes)(err.typeMismatch) - catch case ex: AssertionError => - println(i"error while checking $tree: $actual against $expected") - throw ex - finally - ccState.ignoreClassifiers = saved + printOnAssertionError(i"error while checking $tree: $actual against $expected"): + try + tree match + case tree: TypeApply if tree.symbol == defn.Any_typeCast => ccState.ignoreClassifiers = true + case _ => + testAdapted(actual, expected, tree, notes)(err.typeMismatch) + finally + ccState.ignoreClassifiers = saved @annotation.tailrec private def findImpureUpperBound(tp: Type)(using Context): Type = tp match diff --git a/compiler/src/dotty/tools/dotc/cc/Setup.scala b/compiler/src/dotty/tools/dotc/cc/Setup.scala index 421557181012..8511945c57b1 100644 --- a/compiler/src/dotty/tools/dotc/cc/Setup.scala +++ b/compiler/src/dotty/tools/dotc/cc/Setup.scala @@ -405,14 +405,11 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: addVar(mapFollowingAliases(tp), tp) } - try + printOnAssertionError(i"error while mapping inferred $tp"): ccState.withNoVarsMapped: mapInferred(inCaptureRefinement = false)(tp) .tap: tp1 => if tp1 ne tp then capt.println(i"expanded inferred in ${ctx.owner}: $tp --> $tp1") - catch case ex: AssertionError => - println(i"error while mapping inferred $tp") - throw ex } /** Transform an explicitly given type by performing the following transformation @@ -756,14 +753,11 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: def paramsToCap(psymss: List[List[Symbol]], mt: Type)(using Context): Type = mt match case mt: MethodType => - try + printOnAssertionError(i"error while mapping params ${mt.paramInfos} of $sym"): mt.derivedLambdaType( paramInfos = psymss.head.lazyZip(mt.paramInfos).map(localCapToGlobal), resType = paramsToCap(psymss.tail, mt.resType)) - catch case ex: AssertionError => - println(i"error while mapping params ${mt.paramInfos} of $sym") - throw ex case mt: PolyType => mt.derivedLambdaType(resType = paramsToCap(psymss.tail, mt.resType)) case _ => mt diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index be15a1f6f427..8d1443f62dc1 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -167,6 +167,8 @@ private sealed trait WarningSettings: private val WrecurseWithDefault = BooleanSetting(WarningSetting, "Wrecurse-with-default", "Warn when a method calls itself with a default argument.") private val WwrongArrow = BooleanSetting(WarningSetting, "Wwrong-arrow", "Warn if function arrow was used instead of context literal ?=>.") private val WinferUnion = BooleanSetting(WarningSetting, "Winfer-union", "Warn if type argument was inferred as union type.") + private val WsafeInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.") + private val WunreasonableCatch: Setting[Boolean] = BooleanSetting(WarningSetting, "Wunreasonable-catch", "Warn when catching errors that should normally not be caught.") private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting( WarningSetting, name = "Wunused", @@ -297,8 +299,6 @@ private sealed trait WarningSettings: allOr("type-parameter-shadow") end WshadowHas - val WsafeInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.") - object Whas: def allOr(s: Setting[Boolean])(using Context): Boolean = Wall.value || s.value @@ -312,6 +312,7 @@ private sealed trait WarningSettings: def wrongArrow(using Context): Boolean = allOr(WwrongArrow) def inferUnion(using Context): Boolean = allOr(WinferUnion) def safeInit(using Context): Boolean = allOr(WsafeInit) + def unreasonableCatch(using Context): Boolean = allOr(WunreasonableCatch) /** "Optimizer" settings */ private sealed trait OptimizerSettings: diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 8d65165db280..382a1528c487 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -757,6 +757,7 @@ class Definitions { def ThrowableClass(using Context): ClassSymbol = ThrowableType.symbol.asClass @tu lazy val ExceptionClass: ClassSymbol = requiredClass("java.lang.Exception") @tu lazy val RuntimeExceptionClass: ClassSymbol = requiredClass("java.lang.RuntimeException") + @tu lazy val ErrorType: TypeRef = requiredClassRef("java.lang.Error") @tu lazy val SerializableType: TypeRef = JavaSerializableClass.typeRef def SerializableClass(using Context): ClassSymbol = SerializableType.symbol.asClass diff --git a/compiler/src/dotty/tools/dotc/core/RecursiveOperation.scala b/compiler/src/dotty/tools/dotc/core/RecursiveOperation.scala index 500218a1acdd..b2a6d0adfc27 100644 --- a/compiler/src/dotty/tools/dotc/core/RecursiveOperation.scala +++ b/compiler/src/dotty/tools/dotc/core/RecursiveOperation.scala @@ -44,7 +44,7 @@ object RecursiveOperation: * * @param ops the recursive operations, most recent first */ -final class RecursionOverflow(ops: List[RecursiveOperation])(using val ctx: Context) extends Error: +final class RecursionOverflow(ops: List[RecursiveOperation])(using val ctx: Context) extends Throwable: // We aren't going to show the stack trace anyway so might as well save the perf cost of throwing override def fillInStackTrace(): Throwable = this @@ -82,6 +82,6 @@ object RecursionOverflow: rawOverflowTitle: String, rawOverflowDetails: RecursiveOperationDetails, rawOverflowPosition: SrcPos | Null, - rawOverflowWeight: Int)(using Context): Error = + rawOverflowWeight: Int)(using Context): Throwable = val ops = RecursiveOperation(rawOverflowTitle, rawOverflowDetails, rawOverflowPosition, rawOverflowWeight) :: rawOps.map(_.copy()).reverse.toList new RecursionOverflow(ops) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 27473732ba1d..c72424d17d92 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -27,6 +27,7 @@ import NameKinds.WildcardParamName import MatchTypes.isConcrete import reporting.Message.Note import scala.util.boundary, boundary.break +import scala.util.control.NonFatal /** Provides methods to compare types. */ @@ -703,27 +704,25 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // the isSubinfo of hasMatchingMember has problems dealing with PolyTypes // (---> orphan params during pickling) def isSubInfo(info1: Type, info2: Type): Boolean = - try (info1, info2) match - case (info1: PolyType, info2: PolyType) => - info1.paramNames.hasSameLengthAs(info2.paramNames) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1)) - case (info1: MethodType, info2: MethodType) => - matchingMethodParams(info1, info2, precise = false) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1)) - case (info1 @ CapturingType(parent1, refs1), info2: Type) - if info2.stripCapturing.isInstanceOf[MethodOrPoly] => - compareCaptures(info1, refs1, info2) - && isSubInfo(parent1, info2) - case (info1: Type, CapturingType(parent2, _)) - if info1.stripCapturing.isInstanceOf[MethodOrPoly] => - val refs1 = info1.captureSet - (refs1.isAlwaysEmpty || compareCaptures(info1, refs1, info2)) - && isSubInfo(info1, parent2) - case _ => - isSubType(info1, info2) - catch case ex: AssertionError => - println(i"error while subinfo $info1 <:< $info2") - throw ex + printOnAssertionError(i"error while subinfo $info1 <:< $info2"): + (info1, info2) match + case (info1: PolyType, info2: PolyType) => + info1.paramNames.hasSameLengthAs(info2.paramNames) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1)) + case (info1: MethodType, info2: MethodType) => + matchingMethodParams(info1, info2, precise = false) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1)) + case (info1 @ CapturingType(parent1, refs1), info2: Type) + if info2.stripCapturing.isInstanceOf[MethodOrPoly] => + compareCaptures(info1, refs1, info2) + && isSubInfo(parent1, info2) + case (info1: Type, CapturingType(parent2, _)) + if info1.stripCapturing.isInstanceOf[MethodOrPoly] => + val refs1 = info1.captureSet + (refs1.isAlwaysEmpty || compareCaptures(info1, refs1, info2)) + && isSubInfo(info1, parent2) + case _ => + isSubType(info1, info2) if defn.isFunctionType(tp2) then if tp2.derivesFrom(defn.PolyFunctionClass) then @@ -732,10 +731,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling tp1w.widenDealias match case tp1: RefinedType => return - try isSubInfo(tp1.refinedInfo, tp2.refinedInfo) - catch case ex: AssertionError => - println(i"error while subInfo ${tp1.refinedInfo} <:< ${tp2.refinedInfo}") - throw ex + printOnAssertionError(i"error while subInfo ${tp1.refinedInfo} <:< ${tp2.refinedInfo}"): + isSubInfo(tp1.refinedInfo, tp2.refinedInfo) case _ => end if @@ -906,7 +903,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling case CapturingType(parent2, refs2) => def compareCapturing: Boolean = val refs1 = tp1.captureSet - try + printOnAssertionError(i"assertion failed while compare captured $tp1 <:< $tp2"): if refs1.isAlwaysEmpty && refs1.mutability == CaptureSet.Mutability.Ignored then recur(tp1, parent2) else parent2 match @@ -932,9 +929,6 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // this alternative is needed in case the right hand side is a // capturing type that contains the lhs as an alternative of a union type. ) - catch case ex: AssertionError => - println(i"assertion failed while compare captured $tp1 <:< $tp2") - throw ex compareCapturing || fourthTry case tp2: AnnotatedType if tp2.isRefining => (tp1.derivesAnnotWith(tp2.annot.sameAnnotation) || tp1.isBottomType) && @@ -1678,13 +1672,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling if (Stats.monitored) recordStatistics(result, savedSuccessCount) result catch - case ex: AssertionError => - showGoal(tp1, tp2) - recCount -= 1 - restore() - successCount = savedSuccessCount - throw ex - case ex: Exception => + case NonFatal(ex) => + if ex.isInstanceOf[AssertionError] then showGoal(tp1, tp2) recCount -= 1 restore() successCount = savedSuccessCount @@ -2975,11 +2964,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling protected def subCaptures(refs1: CaptureSet, refs2: CaptureSet, vs: CaptureSet.VarState = makeVarState())(using Context): Boolean = - try + printOnAssertionError(i"fail while subCaptures $refs1 <:< $refs2"): refs1.subCaptures(refs2, vs) - catch case ex: AssertionError => - println(i"fail while subCaptures $refs1 <:< $refs2") - throw ex /** * - Compare capture sets using subCaptures. If the lower type derives from Stateful and the diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index ccd8a0fc57a2..dd496dcda3db 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -1050,7 +1050,7 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst * * Note: Need to ensure correspondence with erasure! */ - private def sigName(tp: Type)(using Context): TypeName = try + private def sigName(tp: Type)(using Context): TypeName = printOnAssertionError(s"no sig for $tp"): tp match { case tp: TypeRef => if (!tp.denot.exists) @@ -1112,9 +1112,4 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst assert(erasedTp ne tp, tp) sigName(erasedTp) } - catch { - case ex: AssertionError => - println(s"no sig for $tp because of ${ex.printStackTrace()}") - throw ex - } } diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index 2b42bf926387..da1db23aae47 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -18,6 +18,7 @@ import config.Feature.sourceVersion import collection.mutable import reporting.{Profile, NoProfile} import dotty.tools.tasty.TastyFormat.ASTsSection +import scala.util.control.NonFatal class TreePickler(pickler: TastyPickler, attributes: Attributes) { val buf: TreeBuffer = new TreeBuffer @@ -170,7 +171,7 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) { def pickleType(tpe0: Type, richTypes: Boolean = false)(using Context): Unit = { val tpe = tpe0.stripTypeVar - try { + printOnAssertionError(i"error when pickling type $tpe") { val prev: Addr | Null = pickledTypes.lookup(tpe) if (prev == null) { pickledTypes(tpe) = currentAddr @@ -181,11 +182,6 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) { writeRef(prev) } } - catch { - case ex: AssertionError => - println(i"error when pickling type $tpe") - throw ex - } } private def pickleNewType(tpe: Type, richTypes: Boolean)(using Context): Unit = tpe match { @@ -820,12 +816,9 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) { case ex: TypeError => report.error(ex.toMessage, tree.srcPos.focus) pickleErrorType() - case ex: AssertionError => - println(i"error when pickling tree $tree of class ${tree.getClass}") - throw ex - case ex: MatchError => + case NonFatal(t) => println(i"error when pickling tree $tree of class ${tree.getClass}") - throw ex + throw t } } diff --git a/compiler/src/dotty/tools/dotc/plugins/Plugin.scala b/compiler/src/dotty/tools/dotc/plugins/Plugin.scala index b2a4122bc162..0eaa72f6aa1b 100644 --- a/compiler/src/dotty/tools/dotc/plugins/Plugin.scala +++ b/compiler/src/dotty/tools/dotc/plugins/Plugin.scala @@ -99,6 +99,7 @@ object Plugin { /** Use a class loader to load the plugin class. */ + @nowarn("msg=Catching NoClassDefFoundError can lead to unexpected behavior") // backwards compat def load(classname: String, loader: ClassLoader): Try[AnyClass] = { try Success[AnyClass](loader.loadClass(classname)) diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index f2782d43c25d..1a3e4e8e2422 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -248,6 +248,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case IllegalIdentifierID // errorNumber: 230 case ConcreteClassHasUnimplementedMethodsID // errorNumber: 231 case UseOfAnyMethodAsInterpolatorID // errorNumber: 232 + case UnreasonableCatchID // errorNumber: 233 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 7e5fbb1d17ed..64664342ba38 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -4,12 +4,19 @@ package reporting import core.* import Contexts.* -import Decorators.*, Symbols.*, Names.*, NameOps.*, Types.*, Flags.*, Phases.* +import Decorators.* +import Symbols.* +import Names.* +import NameOps.* +import Types.{Type, *} +import Flags.* +import Phases.* import Denotations.SingleDenotation import SymDenotations.SymDenotation -import NameKinds.{WildcardParamName, ContextFunctionParamName} +import NameKinds.{ContextFunctionParamName, WildcardParamName} import parsing.Scanners.Token -import parsing.Tokens, Tokens.showToken +import parsing.Tokens +import Tokens.showToken import printing.Highlighting.* import printing.Formatting import ErrorMessageID.* @@ -21,11 +28,12 @@ import config.{Feature, MigrationVersion, ScalaVersion} import transform.patmat.Space import transform.patmat.SpaceEngine import typer.ErrorReporting.{err, matchReductionAddendum, substitutableTypeSymbolsInScope} -import typer.ProtoTypes.{ViewProto, FunProto} +import typer.ProtoTypes.{FunProto, ViewProto} import typer.Implicits.* import typer.Inferencing import StdNames.nme -import Formatting.{hl, delay} +import Formatting.{delay, hl} + import scala.util.matching.Regex import java.util.regex.Matcher.quoteReplacement import cc.CaptureSet @@ -3944,4 +3952,17 @@ class UseOfAnyMethodAsInterpolator(interpolator: Name)(using Context) i"""String interpolation resolves to methods calls on ${hl("StringContext")}, |which can target methods declared by ${hl("Any")} such as ${hl(i"$interpolator")}. |This is unlikely to be what you intended.""" +} + +class UnreasonableCatch(tpe: Type)(using Context) + extends Message(UnreasonableCatchID) { + def kind = MessageKind.PotentialIssue + def msg(using Context) = i"Catching $tpe can lead to unexpected behavior" + def explain(using Context) = + i"""Catching ${hl("Error")} subclasses, including by catching all ${hl("Throwable")}s, + |can lead to unexpected behavior because an ${hl("Error")} being thrown indicates + |an unrecoverable problem. + |The JDK documentation states that "a reasonable application should not + |try to catch" ${hl("Error")}s, and on some platforms such as Scala.js, + |such errors will immediately terminate the application and cannot be caught.""" } \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index 9d368a6a8ab8..b37863bc4f0c 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -17,7 +17,7 @@ import dotty.tools.dotc.core.Denotations.StaleSymbol import dotty.tools.dotc.core.Types.* import dotty.tools.dotc.typer.Applications.* import dotty.tools.dotc.util.{NoSourcePosition, SrcPos} -import dotty.tools.io +import dotty.tools.{io, printOnAssertionError} import dotty.tools.io.AbstractFile import xsbti.UseScope import xsbti.api.DependencyContext @@ -121,7 +121,7 @@ private class ExtractDependenciesCollector(rec: DependencyRecorder) extends Abst * can be retrieved using DependencyRecorder. */ override def traverse(tree: Tree)(using Context): Unit = - try + printOnAssertionError(i"assertion failed while traversing $tree"): recordTree(tree) recordInlineCallArgs(tree) @@ -141,10 +141,6 @@ private class ExtractDependenciesCollector(rec: DependencyRecorder) extends Abst t.body.foreach(traverse) case _ => traverseChildren(tree) - catch - case ex: AssertionError => - println(i"asserted failed while traversing $tree") - throw ex end ExtractDependenciesCollector /** Extract the dependency information of a compilation unit. diff --git a/compiler/src/dotty/tools/dotc/transform/Inlining.scala b/compiler/src/dotty/tools/dotc/transform/Inlining.scala index bf2c28b64936..37184baf88fd 100644 --- a/compiler/src/dotty/tools/dotc/transform/Inlining.scala +++ b/compiler/src/dotty/tools/dotc/transform/Inlining.scala @@ -3,8 +3,7 @@ package transform import java.util.Arrays - -import dotty.tools.io +import dotty.tools.{io, printOnAssertionError} import ast.tpd import ast.Trees.* import ast.TreeMapWithTrackedStats @@ -19,7 +18,7 @@ import DenotTransformers.IdentityDenotTransformer import MacroAnnotations.hasMacroAnnotation import inlines.Inlines import quoted.* -import sbt.{ AbstractExtractDependenciesCollector, DependencyRecorder } +import sbt.{AbstractExtractDependenciesCollector, DependencyRecorder} import staging.StagingLevel import util.Property @@ -113,7 +112,7 @@ class Inlining extends MacroTransform, IdentityDenotTransformer { val inlineFinder = new tpd.TreeTraverser: override def traverse(tree: Tree)(using Context): Unit = - try + printOnAssertionError(i"assertion failed while traversing $tree"): tree match case tree: Inlined => collector.traverse(tree) @@ -126,10 +125,6 @@ class Inlining extends MacroTransform, IdentityDenotTransformer { t.body.foreach(traverse) case _ => traverseChildren(tree) - catch - case ex: AssertionError => - println(i"asserted failed while traversing $tree") - throw ex override def transform(tree: Tree)(using Context): Tree = { val result = tree match diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index e1906ddf6763..98a227843d34 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -560,236 +560,232 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => end flattenSpreads override def transform(tree: Tree)(using Context): Tree = - try tree match { - // TODO move CaseDef case lower: keep most probable trees first for performance - case CaseDef(pat, _, _) => - val gadtCtx = - pat.removeAttachment(typer.Typer.InferredGadtConstraints) match - case Some(gadt) => ctx.fresh.setGadtState(GadtState(gadt)) - case None => - ctx - super.transform(tree)(using gadtCtx) - case tree: Ident => - if tree.isType then - checkNotPackage(tree) - else + printOnAssertionError(i"error while transforming $tree"): + tree match { + // TODO move CaseDef case lower: keep most probable trees first for performance + case CaseDef(pat, _, _) => + val gadtCtx = + pat.removeAttachment(typer.Typer.InferredGadtConstraints) match + case Some(gadt) => ctx.fresh.setGadtState(GadtState(gadt)) + case None => + ctx + super.transform(tree)(using gadtCtx) + case tree: Ident => + if tree.isType then + checkNotPackage(tree) + else + registerNeedsInlining(tree) + val tree1 = checkUsableAsValue(tree) + tree1.tpe match { + case tpe: ThisType => This(tpe.cls).withSpan(tree.span) + case _ => tree1 + } + case tree @ Select(qual, name) => + registerNeedsInlining(tree) + if name.isTypeName then + Checking.checkRealizable(qual.tpe, qual.srcPos) + withMode(Mode.Type)(super.transform(checkNotPackage(tree))) + else + checkUsableAsValue(tree) match + case tree1: Select => transformSelect(tree1, Nil) + case tree1 => tree1 + case app: Apply => + val methType = app.fun.tpe.widen.asInstanceOf[MethodType] + if (methType.hasErasedParams) + for (arg, isErased) <- app.args.lazyZip(methType.paramErasureStatuses) do + if isErased then + if methType.isResultDependent then + Checking.checkRealizable(arg.tpe, arg.srcPos, "erased argument") + def app1 = + // reverse order of transforming args and fun. This way, we get a chance to see other + // well-formedness errors before reporting errors in possible inferred type args of fun. + val args1 = transform(app.args) + cpy.Apply(app)(transform(app.fun), args1) + methPart(app) match + case Select(nu: New, nme.CONSTRUCTOR) if isCheckable(nu) => + // need to check instantiability here, because the type of the New itself + // might be a type constructor. + def checkClassType(tpe: Type, stablePrefixReq: Boolean) = + ctx.typer.checkClassType(tpe, tree.srcPos, + traitReq = false, stablePrefixReq = stablePrefixReq, + refinementOK = Feature.enabled(Feature.modularity)) + checkClassType(tree.tpe, stablePrefixReq = true) + if !nu.tpe.isLambdaSub then + // Check the constructor type as well; it could be an illegal singleton type + // which would not be reflected as `tree.tpe` + checkClassType(nu.tpe, stablePrefixReq = false) + Checking.checkInstantiable(tree.tpe, nu.tpe, nu.srcPos) + withNoCheckNews(nu :: Nil)(app1) + case _ => + app1 + case UnApply(fun, implicits, patterns) => + // Reverse transform order for the same reason as in `app1` above. + val patterns1 = transform(patterns) + val tree1 = cpy.UnApply(tree)(transform(fun), transform(implicits), patterns1) + // The pickling of UnApply trees uses the tpe of the tree, + // so we need to clean retains from it here + tree1.withType(transformAnnotsIn(CleanupRetains()(tree1.tpe))) + case tree: TypeApply => + if tree.symbol == defn.QuotedTypeModule_of then + ctx.compilationUnit.needsStaging = true registerNeedsInlining(tree) - val tree1 = checkUsableAsValue(tree) - tree1.tpe match { - case tpe: ThisType => This(tpe.cls).withSpan(tree.span) - case _ => tree1 + val tree1 @ TypeApply(fn, args) = normalizeTypeArgs(tree) + for arg <- args do + checkInferredWellFormed(arg) + if (fn.symbol != defn.ChildAnnot.primaryConstructor) + // Make an exception for ChildAnnot, which should really have AnyKind bounds + Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType]) + val args1 = + if Feature.ccEnabled && fn.symbol.isInlineMethod + then transform(args).mapConserve(markInferred) + else transform(args) + val fn1 = fn match + case sel: Select => + transformSelect(sel, args1) // skip the checkUsableAsValue of normal transform + case _ => + transform(fn) + cpy.TypeApply(tree1)(fn1, args1) + case tree @ Inlined(call, bindings, expansion) if !tree.inlinedFromOuterScope => + val pos = call.sourcePos + CrossVersionChecks.checkRef(call.symbol, pos) + withMode(Mode.NoInline)(transform(call)) + val callTrace = Inlines.inlineCallTrace(call.symbol, pos)(using ctx.withSource(pos.source)) + cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(using inlineContext(tree))) + case templ: Template => + Checking.checkPolyFunctionExtension(templ) + withNoCheckNews(templ.parents.flatMap(newPart)) { + forwardParamAccessors(templ) + synthMbr.addSyntheticMembers( + beanProps.addBeanMethods( + superAcc.wrapTemplate(templ)( + super.transform(_).asInstanceOf[Template])) + ) } - case tree @ Select(qual, name) => - registerNeedsInlining(tree) - if name.isTypeName then - Checking.checkRealizable(qual.tpe, qual.srcPos) - withMode(Mode.Type)(super.transform(checkNotPackage(tree))) - else - checkUsableAsValue(tree) match - case tree1: Select => transformSelect(tree1, Nil) - case tree1 => tree1 - case app: Apply => - val methType = app.fun.tpe.widen.asInstanceOf[MethodType] - if (methType.hasErasedParams) - for (arg, isErased) <- app.args.lazyZip(methType.paramErasureStatuses) do - if isErased then - if methType.isResultDependent then - Checking.checkRealizable(arg.tpe, arg.srcPos, "erased argument") - def app1 = - // reverse order of transforming args and fun. This way, we get a chance to see other - // well-formedness errors before reporting errors in possible inferred type args of fun. - val args1 = transform(app.args) - cpy.Apply(app)(transform(app.fun), args1) - methPart(app) match - case Select(nu: New, nme.CONSTRUCTOR) if isCheckable(nu) => - // need to check instantiability here, because the type of the New itself - // might be a type constructor. - def checkClassType(tpe: Type, stablePrefixReq: Boolean) = - ctx.typer.checkClassType(tpe, tree.srcPos, - traitReq = false, stablePrefixReq = stablePrefixReq, - refinementOK = Feature.enabled(Feature.modularity)) - checkClassType(tree.tpe, stablePrefixReq = true) - if !nu.tpe.isLambdaSub then - // Check the constructor type as well; it could be an illegal singleton type - // which would not be reflected as `tree.tpe` - checkClassType(nu.tpe, stablePrefixReq = false) - Checking.checkInstantiable(tree.tpe, nu.tpe, nu.srcPos) - withNoCheckNews(nu :: Nil)(app1) - case _ => - app1 - case UnApply(fun, implicits, patterns) => - // Reverse transform order for the same reason as in `app1` above. - val patterns1 = transform(patterns) - val tree1 = cpy.UnApply(tree)(transform(fun), transform(implicits), patterns1) - // The pickling of UnApply trees uses the tpe of the tree, - // so we need to clean retains from it here - tree1.withType(transformAnnotsIn(CleanupRetains()(tree1.tpe))) - case tree: TypeApply => - if tree.symbol == defn.QuotedTypeModule_of then - ctx.compilationUnit.needsStaging = true - registerNeedsInlining(tree) - val tree1 @ TypeApply(fn, args) = normalizeTypeArgs(tree) - for arg <- args do - checkInferredWellFormed(arg) - if (fn.symbol != defn.ChildAnnot.primaryConstructor) - // Make an exception for ChildAnnot, which should really have AnyKind bounds - Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType]) - val args1 = - if Feature.ccEnabled && fn.symbol.isInlineMethod - then transform(args).mapConserve(markInferred) - else transform(args) - val fn1 = fn match - case sel: Select => - transformSelect(sel, args1) // skip the checkUsableAsValue of normal transform - case _ => - transform(fn) - cpy.TypeApply(tree1)(fn1, args1) - case tree @ Inlined(call, bindings, expansion) if !tree.inlinedFromOuterScope => - val pos = call.sourcePos - CrossVersionChecks.checkRef(call.symbol, pos) - withMode(Mode.NoInline)(transform(call)) - val callTrace = Inlines.inlineCallTrace(call.symbol, pos)(using ctx.withSource(pos.source)) - cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(using inlineContext(tree))) - case templ: Template => - Checking.checkPolyFunctionExtension(templ) - withNoCheckNews(templ.parents.flatMap(newPart)) { - forwardParamAccessors(templ) - synthMbr.addSyntheticMembers( - beanProps.addBeanMethods( - superAcc.wrapTemplate(templ)( - super.transform(_).asInstanceOf[Template])) + case tree: ValDef => + annotateExperimentalCompanion(tree.symbol) + registerIfHasMacroAnnotations(tree) + Checking.checkPolyFunctionType(tree.tpt) + val tree1 = cpy.ValDef(tree)(tpt = explicifyTpt(tree)) + if tree1.removeAttachment(desugar.UntupledParam).isDefined then + checkStableSelection(tree.rhs) + processValOrDefDef(super.transform(tree1)) + case tree: DefDef => + registerIfHasMacroAnnotations(tree) + Checking.checkPolyFunctionType(tree.tpt) + annotateContextResults(tree) + val tree1 = cpy.DefDef(tree)(tpt = explicifyTpt(tree)) + processValOrDefDef(superAcc.wrapDefDef(tree1)(super.transform(tree1).asInstanceOf[DefDef])) + case tree: TypeDef => + registerIfHasMacroAnnotations(tree) + val sym = tree.symbol + if (sym.isClass) + VarianceChecker.check(tree) + annotateExperimentalCompanion(sym) + checkMacroAnnotation(sym) + if sym.isOneOf(GivenOrImplicit) then + sym.keepAnnotationsCarrying(thisPhase, Set(defn.CompanionClassMetaAnnot), orNoneOf = defn.MetaAnnots) + tree.rhs match + case impl: Template => + for parent <- impl.parents do + Checking.checkTraitInheritance(parent.tpe.classSymbol, sym.asClass, parent.srcPos) + // Constructor parameters are in scope when typing a parent. + // While they can safely appear in a parent tree, to preserve + // soundness we need to ensure they don't appear in a parent + // type (#16270). We can strip any refinement of a parent type since + // these refinements are split off from the parent type constructor + // application `parent` in Namer and don't show up as parent types + // of the class. + val illegalRefs = parent.tpe.dealias.stripRefinement.namedPartsWith: + p => p.symbol.is(ParamAccessor) && (p.symbol.owner eq sym) + if illegalRefs.nonEmpty then + report.error( + em"The type of a class parent cannot refer to constructor parameters, but ${parent.tpe} refers to ${illegalRefs.map(_.name.show).mkString(",")}", parent.srcPos) + else + if !sym.is(Param) && !sym.owner.isOneOf(AbstractOrTrait) then + Checking.checkGoodBounds(tree.symbol) + // Delete all context bound companions of this TypeDef + if sym.owner.isClass && sym.hasAnnotation(defn.WitnessNamesAnnot) then + val decls = sym.owner.info.decls + for cbCompanion <- decls.lookupAll(sym.name.toTermName) do + if cbCompanion.isContextBoundCompanion then + decls.openForMutations.unlink(cbCompanion) + (tree.rhs, sym.info) match + case (rhs: LambdaTypeTree, bounds: TypeBounds) => + VarianceChecker.checkLambda(rhs, bounds) + if sym.isOpaqueAlias then + VarianceChecker.checkLambda(rhs, TypeBounds.upper(sym.opaqueAlias)) + case _ => + processMemberDef(super.transform(scala2LibPatch(tree))) + case tree: Bind => + val sym = tree.symbol + if sym.isType && !sym.name.is(WildcardParamName) then + Checking.checkGoodBounds(sym) + // Cleanup retains from the info of the Bind symbol + sym.copySymDenotation(info = transformAnnotsIn(CleanupRetains()(sym.info))).installAfter(thisPhase) + super.transform(tree) + case tree: New if isCheckable(tree) => + Checking.checkInstantiable(tree.tpe, tree.tpe, tree.srcPos) + super.transform(tree) + case tree: Closure if !tree.tpt.isEmpty => + Checking.checkRealizable(tree.tpt.tpe, tree.srcPos, "SAM type") + super.transform(tree) + case tree @ Annotated(annotated, annot) => + cpy.Annotated(tree)(transform(annotated), transformAnnotTree(annot)) + case tree: AppliedTypeTree => + if (tree.tpt.symbol == defn.andType) + Checking.checkNonCyclicInherited(tree.tpe, tree.args.tpes, EmptyScope, tree.srcPos) + // Ideally, this should be done by Typer, but we run into cyclic references + // when trying to typecheck self types which are intersections. + else if (tree.tpt.symbol == defn.orType) + () // nothing to do + else + Checking.checkAppliedType(tree) + super.transform(tree) + case SingletonTypeTree(ref) => + if !ctx.mode.is(Mode.InCaptureSet) then + Checking.checkRealizable(ref.tpe, ref.srcPos) + super.transform(tree) + case tree: TypeBoundsTree => + val TypeBoundsTree(lo, hi, alias) = tree + if !alias.isEmpty then + val bounds = TypeBounds(lo.tpe, hi.tpe) + if !bounds.contains(alias.tpe) then + report.error(em"type ${alias.tpe} outside bounds $bounds", tree.srcPos) + super.transform(tree) + case tree: TypeTree => + val tpe = if tree.isInferred then CleanupRetains()(tree.tpe) else tree.tpe + tree.withType(transformAnnotsIn(tpe)) + case Typed(Ident(nme.WILDCARD), _) => + withMode(Mode.Pattern)(super.transform(tree)) + // The added mode signals that bounds in a pattern need not + // conform to selector bounds. I.e. assume + // type Tree[T >: Null <: Type] + // One is still allowed to write + // case x: Tree[?] + // (which translates to) + // case x: (_: Tree[?]) + case m @ MatchTypeTree(bounds, selector, cases) => + // Analog to the case above for match types + def transformIgnoringBoundsCheck(x: CaseDef): CaseDef = + withMode(Mode.Pattern)(super.transform(x)).asInstanceOf[CaseDef] + cpy.MatchTypeTree(tree)( + super.transform(bounds), + super.transform(selector), + cases.mapConserve(transformIgnoringBoundsCheck) ) - } - case tree: ValDef => - annotateExperimentalCompanion(tree.symbol) - registerIfHasMacroAnnotations(tree) - Checking.checkPolyFunctionType(tree.tpt) - val tree1 = cpy.ValDef(tree)(tpt = explicifyTpt(tree)) - if tree1.removeAttachment(desugar.UntupledParam).isDefined then - checkStableSelection(tree.rhs) - processValOrDefDef(super.transform(tree1)) - case tree: DefDef => - registerIfHasMacroAnnotations(tree) - Checking.checkPolyFunctionType(tree.tpt) - annotateContextResults(tree) - val tree1 = cpy.DefDef(tree)(tpt = explicifyTpt(tree)) - processValOrDefDef(superAcc.wrapDefDef(tree1)(super.transform(tree1).asInstanceOf[DefDef])) - case tree: TypeDef => - registerIfHasMacroAnnotations(tree) - val sym = tree.symbol - if (sym.isClass) - VarianceChecker.check(tree) - annotateExperimentalCompanion(sym) - checkMacroAnnotation(sym) - if sym.isOneOf(GivenOrImplicit) then - sym.keepAnnotationsCarrying(thisPhase, Set(defn.CompanionClassMetaAnnot), orNoneOf = defn.MetaAnnots) - tree.rhs match - case impl: Template => - for parent <- impl.parents do - Checking.checkTraitInheritance(parent.tpe.classSymbol, sym.asClass, parent.srcPos) - // Constructor parameters are in scope when typing a parent. - // While they can safely appear in a parent tree, to preserve - // soundness we need to ensure they don't appear in a parent - // type (#16270). We can strip any refinement of a parent type since - // these refinements are split off from the parent type constructor - // application `parent` in Namer and don't show up as parent types - // of the class. - val illegalRefs = parent.tpe.dealias.stripRefinement.namedPartsWith: - p => p.symbol.is(ParamAccessor) && (p.symbol.owner eq sym) - if illegalRefs.nonEmpty then - report.error( - em"The type of a class parent cannot refer to constructor parameters, but ${parent.tpe} refers to ${illegalRefs.map(_.name.show).mkString(",")}", parent.srcPos) - else - if !sym.is(Param) && !sym.owner.isOneOf(AbstractOrTrait) then - Checking.checkGoodBounds(tree.symbol) - // Delete all context bound companions of this TypeDef - if sym.owner.isClass && sym.hasAnnotation(defn.WitnessNamesAnnot) then - val decls = sym.owner.info.decls - for cbCompanion <- decls.lookupAll(sym.name.toTermName) do - if cbCompanion.isContextBoundCompanion then - decls.openForMutations.unlink(cbCompanion) - (tree.rhs, sym.info) match - case (rhs: LambdaTypeTree, bounds: TypeBounds) => - VarianceChecker.checkLambda(rhs, bounds) - if sym.isOpaqueAlias then - VarianceChecker.checkLambda(rhs, TypeBounds.upper(sym.opaqueAlias)) - case _ => - processMemberDef(super.transform(scala2LibPatch(tree))) - case tree: Bind => - val sym = tree.symbol - if sym.isType && !sym.name.is(WildcardParamName) then - Checking.checkGoodBounds(sym) - // Cleanup retains from the info of the Bind symbol - sym.copySymDenotation(info = transformAnnotsIn(CleanupRetains()(sym.info))).installAfter(thisPhase) - super.transform(tree) - case tree: New if isCheckable(tree) => - Checking.checkInstantiable(tree.tpe, tree.tpe, tree.srcPos) - super.transform(tree) - case tree: Closure if !tree.tpt.isEmpty => - Checking.checkRealizable(tree.tpt.tpe, tree.srcPos, "SAM type") - super.transform(tree) - case tree @ Annotated(annotated, annot) => - cpy.Annotated(tree)(transform(annotated), transformAnnotTree(annot)) - case tree: AppliedTypeTree => - if (tree.tpt.symbol == defn.andType) - Checking.checkNonCyclicInherited(tree.tpe, tree.args.tpes, EmptyScope, tree.srcPos) - // Ideally, this should be done by Typer, but we run into cyclic references - // when trying to typecheck self types which are intersections. - else if (tree.tpt.symbol == defn.orType) - () // nothing to do - else - Checking.checkAppliedType(tree) - super.transform(tree) - case SingletonTypeTree(ref) => - if !ctx.mode.is(Mode.InCaptureSet) then - Checking.checkRealizable(ref.tpe, ref.srcPos) - super.transform(tree) - case tree: TypeBoundsTree => - val TypeBoundsTree(lo, hi, alias) = tree - if !alias.isEmpty then - val bounds = TypeBounds(lo.tpe, hi.tpe) - if !bounds.contains(alias.tpe) then - report.error(em"type ${alias.tpe} outside bounds $bounds", tree.srcPos) - super.transform(tree) - case tree: TypeTree => - val tpe = if tree.isInferred then CleanupRetains()(tree.tpe) else tree.tpe - tree.withType(transformAnnotsIn(tpe)) - case Typed(Ident(nme.WILDCARD), _) => - withMode(Mode.Pattern)(super.transform(tree)) - // The added mode signals that bounds in a pattern need not - // conform to selector bounds. I.e. assume - // type Tree[T >: Null <: Type] - // One is still allowed to write - // case x: Tree[?] - // (which translates to) - // case x: (_: Tree[?]) - case m @ MatchTypeTree(bounds, selector, cases) => - // Analog to the case above for match types - def transformIgnoringBoundsCheck(x: CaseDef): CaseDef = - withMode(Mode.Pattern)(super.transform(x)).asInstanceOf[CaseDef] - cpy.MatchTypeTree(tree)( - super.transform(bounds), - super.transform(selector), - cases.mapConserve(transformIgnoringBoundsCheck) - ) - case Block(_, Closure(_, _, tpt)) if ExpandSAMs.needsWrapperClass(tpt.tpe) => - superAcc.withInvalidCurrentClass(super.transform(tree)) - case tree: RefinedTypeTree => - Checking.checkPolyFunctionType(tree) - super.transform(tree) - case tree: SeqLiteral if tree.hasAttachment(HasSpreads) => - flattenSpreads(tree) - case _: Quote | _: QuotePattern => - ctx.compilationUnit.needsStaging = true - super.transform(tree) - case tree => - super.transform(tree) - } - catch { - case ex : AssertionError => - println(i"error while transforming $tree") - throw ex - } + case Block(_, Closure(_, _, tpt)) if ExpandSAMs.needsWrapperClass(tpt.tpe) => + superAcc.withInvalidCurrentClass(super.transform(tree)) + case tree: RefinedTypeTree => + Checking.checkPolyFunctionType(tree) + super.transform(tree) + case tree: SeqLiteral if tree.hasAttachment(HasSpreads) => + flattenSpreads(tree) + case _: Quote | _: QuotePattern => + ctx.compilationUnit.needsStaging = true + super.transform(tree) + case tree => + super.transform(tree) + } override def transformStats[T](trees: List[Tree], exprOwner: Symbol, wrapResult: List[Tree] => Context ?=> T)(using Context): T = Checking.checkAndAdaptExperimentalImports(trees) diff --git a/compiler/src/dotty/tools/dotc/transform/Recheck.scala b/compiler/src/dotty/tools/dotc/transform/Recheck.scala index 76911eb85aa9..124f4fb4fe36 100644 --- a/compiler/src/dotty/tools/dotc/transform/Recheck.scala +++ b/compiler/src/dotty/tools/dotc/transform/Recheck.scala @@ -647,18 +647,15 @@ abstract class Recheck extends Phase, SymTransformer: case _ => checkConformsExpr(tpe.widenExpr, pt.widenExpr, tree) def isCompatible(actual: Type, expected: Type)(using Context): Boolean = - try - actual <:< expected - || expected.isRepeatedParam - && isCompatible(actual, - expected.translateFromRepeated(toArray = actual.isRef(defn.ArrayClass))) - || { - val widened = widenSkolems(expected) - (widened ne expected) && isCompatible(actual, widened) - } - catch case ex: AssertionError => - println(i"fail while $actual iscompat $expected") - throw ex + printOnAssertionError(i"fail while $actual iscompat $expected"): + actual <:< expected + || expected.isRepeatedParam + && isCompatible(actual, + expected.translateFromRepeated(toArray = actual.isRef(defn.ArrayClass))) + || { + val widened = widenSkolems(expected) + (widened ne expected) && isCompatible(actual, widened) + } def checkConformsExpr(actual: Type, expected: Type, tree: Tree, notes: List[Note] = Nil)(using Context): Type = //println(i"check conforms $actual <:< $expected") diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 58911f58772e..10d788ec1cda 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -26,6 +26,8 @@ import staging.StagingLevel import inlines.Inlines.inInlineMethod import cc.RetainingAnnotation +import scala.annotation.nowarn + /** Run by -Ycheck option after a given phase, this class retypes all syntax trees * and verifies that the type of each tree node so obtained conforms to the type found in the tree node. * It also performs the following checks: @@ -868,6 +870,7 @@ object TreeChecker { if nowDefinedSyms.contains(tree.symbol.maybeOwner) then super.assertDefined(tree) + @nowarn("msg=Catching AssertionError can lead to unexpected behavior") // backwards compat def checkMacroGeneratedTree(original: tpd.Tree, expansion: tpd.Tree)(using Context): Unit = if ctx.settings.XcheckMacros.value then // We want to make sure that transparent inline macros are checked in the same way that @@ -908,9 +911,10 @@ object TreeChecker { original ) - try treeChecker.typed(expansion)(using checkingCtx) + try + treeChecker.typed(expansion)(using checkingCtx) catch - case err: java.lang.AssertionError => + case err: AssertionError => reportMalformedMacroTree(err.getMessage(), err) case err: UnhandledError => reportMalformedMacroTree(err.diagnostic.message, err) diff --git a/compiler/src/dotty/tools/dotc/transform/TryCatchPatterns.scala b/compiler/src/dotty/tools/dotc/transform/TryCatchPatterns.scala index 6243f8ac1cad..f2b4b5e0fa24 100644 --- a/compiler/src/dotty/tools/dotc/transform/TryCatchPatterns.scala +++ b/compiler/src/dotty/tools/dotc/transform/TryCatchPatterns.scala @@ -7,10 +7,14 @@ import core.Types.* import core.NameKinds.ExceptionBinderName import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Contexts.* +import dotty.tools.dotc.reporting.UnreasonableCatch import dotty.tools.dotc.transform.MegaPhase.MiniPhase import dotty.tools.dotc.util.Spans.Span +import scala.annotation.tailrec + /** Compiles the cases that can not be handled by primitive catch cases as a common pattern match. + * Also emits warnings for cases that are bad ideas, such as catching Throwable or Error. * * The following code: * ``` @@ -66,11 +70,17 @@ class TryCatchPatterns extends MiniPhase { /** Is this pattern node a catch-all or type-test pattern? */ private def isCatchCase(cdef: CaseDef)(using Context): Boolean = cdef match { - case CaseDef(Typed(Ident(nme.WILDCARD), tpt), EmptyTree, _) => isSimpleThrowable(tpt.tpe) - case CaseDef(Bind(_, Typed(Ident(nme.WILDCARD), tpt)), EmptyTree, _) => isSimpleThrowable(tpt.tpe) - case _ => isDefaultCase(cdef) + case CaseDef(Typed(Ident(nme.WILDCARD), tpt), guard, _) => + warnIfUnreasonableCatch(tpt) + guard == EmptyTree && isSimpleThrowable(tpt.tpe) + case CaseDef(Bind(_, Typed(Ident(nme.WILDCARD), tpt)), guard, _) => + warnIfUnreasonableCatch(tpt) + guard == EmptyTree && isSimpleThrowable(tpt.tpe) + case _ => + isDefaultCase(cdef) } + @tailrec private def isSimpleThrowable(tp: Type)(using Context): Boolean = tp.strippedDealias match { case tp @ TypeRef(pre, _) => (pre == NoPrefix || pre.typeSymbol.isStatic) && // Does not require outer class check @@ -97,6 +107,11 @@ class TryCatchPatterns extends MiniPhase { transformFollowing(Match(sel, patternMatchCases ::: rethrow :: Nil))) ) } + + private def warnIfUnreasonableCatch(tpt: Tree)(using Context): Unit = { + if ctx.settings.Whas.unreasonableCatch && tpt.tpe <:< defn.ErrorType || tpt.tpe =:= defn.ThrowableType then + report.warning(UnreasonableCatch(tpt.tpe), tpt) + } } object TryCatchPatterns: diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 04ac34c1a12e..2482934f151c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -4,16 +4,16 @@ package typer import backend.sjs.JSDefinitions import core.* -import ast.{TreeTypeMap, untpd, tpd} +import ast.{TreeTypeMap, tpd, untpd} import util.Spans.* -import util.Stats.{record, monitored} -import printing.{Showable, Printer} +import util.Stats.{monitored, record} +import printing.{Printer, Showable} import printing.Texts.* import Contexts.* import Types.* import Flags.* import Mode.ImplicitsEnabled -import NameKinds.{LazyImplicitName, ContextBoundParamName} +import NameKinds.{ContextBoundParamName, LazyImplicitName} import Symbols.* import Types.* import Decorators.* @@ -23,7 +23,8 @@ import ProtoTypes.* import ErrorReporting.* import Inferencing.{fullyDefinedType, isFullyDefined} import Scopes.newScope -import Typer.BindingPrec, BindingPrec.* +import Typer.BindingPrec +import BindingPrec.* import Hashable.* import util.{EqHashMap, Stats} import config.{Config, Feature, SourceVersion} @@ -37,7 +38,7 @@ import annotation.tailrec import NullOpsDecorator.stripNull import scala.annotation.internal.sharable -import scala.annotation.threadUnsafe +import scala.annotation.{nowarn, threadUnsafe} import scala.compiletime.uninitialized /** Implicit resolution */ @@ -893,6 +894,7 @@ trait Implicits: /** Find an implicit conversion to apply to given tree `from` so that the * result is compatible with type `to`. */ + @nowarn("msg=Catching AssertionError can lead to unexpected behavior") // we immediately rethrow def inferView(from: Tree, to: Type)(using Context): SearchResult = { record("inferView") if !ctx.mode.is(Mode.ImplicitsEnabled) || from.isInstanceOf[Super] then diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index b6c4a972e276..2757b57af626 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -4651,6 +4651,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // is a temporary hack to keep projects compiling that would fail otherwise due to // searching more arguments to instantiate implicits (PR #23532). A failing project // is described in issue #23609. + @nowarn("msg=unexpected behavior") // will go away once this workaround is unneeded def tryConstrainResult(pt: Type): Boolean = try constrainResult(tree.symbol, wtp, pt) catch case ex: TyperState.BadTyperStateAssertion => false diff --git a/compiler/src/dotty/tools/package.scala b/compiler/src/dotty/tools/package.scala index edd1f28f379e..ff116f6392a0 100644 --- a/compiler/src/dotty/tools/package.scala +++ b/compiler/src/dotty/tools/package.scala @@ -1,5 +1,7 @@ package dotty +import scala.annotation.nowarn + package object tools { /** Cached single-element list of Nil. (Whether this helps performance has not been tested) */ @@ -25,6 +27,18 @@ package object tools { setter(res) res + /** + * Prints the given text if the given operation throws an AssertionError, then rethrows. + */ + @nowarn("msg=Catching AssertionError can lead to unexpected behavior") // we immediately rethrow + inline def printOnAssertionError[T](text: => String)(inline op: => T): T = + try + op + catch + case ex: AssertionError => + println(text) + throw ex + /** * Infrastructure to shorten method calls by not requiring a lambda. * Instead of `def f(x: ... => ...)` that must be called as, e.g., `f(x => x + 1)`, diff --git a/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala b/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala index 39c7d202241b..6afdcfc4ca12 100644 --- a/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala +++ b/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala @@ -37,14 +37,7 @@ class PrintingTest { if (!(new File(flagsFilePath)).exists) Nil else Using(Source.fromFile(flagsFilePath, StandardCharsets.UTF_8.name))(_.getLines().toList).get - try { - Main.process((path.toString :: options(phase, flags)).toArray, reporter, null) - } catch { - case e: Throwable => - println(s"Compile $path exception:") - e.printStackTrace() - throw e - } + Main.process((path.toString :: options(phase, flags)).toArray, reporter, null) val actualLines = byteStream.toString(StandardCharsets.UTF_8.name).linesIterator FileDiff.checkAndDumpOrUpdate(path.toString, actualLines.toIndexedSeq, checkFilePath, tolerateMissingCheckFile = false) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 12497af6bf47..1ee0407cbde2 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -15,6 +15,7 @@ import scala.io.{Codec, Source} import scala.jdk.CollectionConverters.* import scala.util.{Random, Try, Using} import scala.util.Properties.{isJavaAtLeast, javaSpecVersion} +import scala.util.control.NonFatal import dotc.{Compiler, Driver} import dotty.tools.dotc.CoverageSupport @@ -329,7 +330,7 @@ trait ParallelTesting extends RunnerOrchestration with CoverageSupport: case None => onSuccess(testSource, reporters, logger) } case _ => - catch case ex: Throwable => + catch case NonFatal(ex) => echo(s"Exception thrown onComplete (probably by a reporter) in $testSource: ${ex.getClass}") Try(ex.printStackTrace()) .recover{ _ => @@ -501,7 +502,7 @@ trait ParallelTesting extends RunnerOrchestration with CoverageSupport: protected def tryCompile(testSource: TestSource)(op: => Unit): Unit = try op catch - case e: Throwable => + case NonFatal(e) => // if an exception is thrown during compilation, the complete test // run should fail failTestSource(testSource) diff --git a/docs/_docs/reference/error-codes/E233.md b/docs/_docs/reference/error-codes/E233.md new file mode 100644 index 000000000000..cd91994a8197 --- /dev/null +++ b/docs/_docs/reference/error-codes/E233.md @@ -0,0 +1,64 @@ +--- +title: "E233: Unreasonable Catch" +kind: Warning +since: 3.10.0 +--- +# E233: Unreasonable Catch + +This warning is emitted when you write a `catch` case that can catch `Error`. + +Catching `Error`s, including by catching all `Throwable`s, +can lead to unexpected behavior because an ${hl("Error")} being thrown indicates an unrecoverable problem. + +The JDK documentation states that "a reasonable application should not try to catch" `Error`. +The JDK has had a number of bugs around expected behavior when an `Error` is caught, such as `finally` blocks not executing +([source](https://bugs.openjdk.org/browse/JDK-8177802)) and locks remaining locked ([source](https://bugs.openjdk.org/browse/JDK-8318888)). + +On some platforms such as Scala.js, such errors will immediately terminate the application and cannot be caught. + + +--- + +## Example + +```scala sc:fail sc-opts:-Wunreasonable-catch,-Werror,-explain +def foo(): Unit = ??? + +def bar(): Unit = + try foo() + catch case t: Throwable => ??? +``` + +### Error + +```scala sc:nocompile +-- [E233] Potential Issue Warning: example.scala:5:16 -------------------------- +5 | catch case t: Throwable => ??? + | ^^^^^^^^^ + | Catching Throwable can lead to unexpected behavior + |----------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | Catching Error subclasses, including by catching all Throwables, + | can lead to unexpected behavior because an Error being thrown indicates + | an unrecoverable problem. + | The JDK documentation states that "a reasonable application should not + | try to catch" Errors, and on some platforms such as Scala.js, + | such errors will immediately terminate the application and cannot be caught. + ----------------------------------------------------------------------------- +``` + +### Solution + +Use `NonFatal` from `scala.util.control` to only catch non-fatal throwables: + +```scala sc:compile sc-opts:-Wunreasonable-catch,-Werror +import scala.util.control.NonFatal + +def foo(): Unit = ??? + +def bar(): Unit = + try foo() + catch case NonFatal(t) => ??? +``` + diff --git a/docs/sidebar.yml b/docs/sidebar.yml index 73a1703d04b8..e6ba99a71e32 100644 --- a/docs/sidebar.yml +++ b/docs/sidebar.yml @@ -440,4 +440,5 @@ subsection: - page: reference/error-codes/E230.md - page: reference/error-codes/E231.md - page: reference/error-codes/E232.md + - page: reference/error-codes/E233.md diff --git a/language-server/src/dotty/tools/languageserver/decompiler/TastyDecompilerService.scala b/language-server/src/dotty/tools/languageserver/decompiler/TastyDecompilerService.scala index 708eb9fb4b86..294ca00efcfc 100644 --- a/language-server/src/dotty/tools/languageserver/decompiler/TastyDecompilerService.scala +++ b/language-server/src/dotty/tools/languageserver/decompiler/TastyDecompilerService.scala @@ -5,6 +5,7 @@ package decompiler import java.net.URI import java.nio.file._ import java.util.concurrent.CompletableFuture +import scala.util.control.NonFatal import dotty.tools.tasty.UnpickleException import dotty.tools.io.{PlainFile, Path} @@ -38,7 +39,7 @@ trait TastyDecompilerService { } catch { case _: UnpickleException => TastyDecompileResult(error = TastyDecompileResult.ErrorTastyVersion) - case t: Throwable => + case NonFatal(t) => t.printStackTrace() TastyDecompileResult(error = TastyDecompileResult.ErrorOther) } diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala index 53c7a180c406..f9bc59a3e936 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala @@ -11,6 +11,7 @@ import org.eclipse.lsp4j.jsonrpc.services._//{JsonSegment, JsonRequest} import java.net.URI import java.util.concurrent.{CompletableFuture, ConcurrentHashMap} +import scala.util.control.NonFatal @JsonSegment("worksheet") trait WorksheetService { thisServer: DottyLanguageServer => @@ -28,7 +29,7 @@ trait WorksheetService { thisServer: DottyLanguageServer => cancelChecker.checkCanceled() WorksheetRunResult(success = true) } catch { - case _: Throwable => + case NonFatal(_) => WorksheetRunResult(success = false) } }) diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index 936286cf8879..45f4dbf8630c 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -279,7 +279,7 @@ class CodeTester(projects: List[Project]) { try { action.execute()(using testServer, testServer.client, positions) } catch { - case ex: AssertionError => + case ex: Exception => val sourcesStr = sources.zip(files).map { case ((project, source), file) => diff --git a/library/src/scala/MatchError.scala b/library/src/scala/MatchError.scala index fa5e483ab951..5f6b52310da6 100644 --- a/library/src/scala/MatchError.scala +++ b/library/src/scala/MatchError.scala @@ -13,6 +13,7 @@ package scala import scala.language.`2.13` +import scala.util.control.NonFatal /** This class implements errors which are thrown whenever an * object doesn't match any pattern of a pattern matching @@ -30,7 +31,7 @@ final class MatchError(@transient obj: Any) extends RuntimeException { else try s"$obj ($ofClass)" catch { - case _: Throwable => "an instance " + ofClass + case NonFatal(_) => "an instance " + ofClass } } diff --git a/library/src/scala/collection/immutable/LazyList.scala b/library/src/scala/collection/immutable/LazyList.scala index 7c21ff759c8b..e4ef14be0a12 100644 --- a/library/src/scala/collection/immutable/LazyList.scala +++ b/library/src/scala/collection/immutable/LazyList.scala @@ -324,14 +324,14 @@ final class LazyList[+A] private (lazyState: AnyRef /* EmptyMarker.type | () => // this way, there is no allocation in the common case where there's no race. // if multiple threads attempt to initialize a LazyList, an `InRace` instance is created to coordinate. if (_tailUpdater.compareAndSet(this, fun, Thread.currentThread)) { - var ex: Throwable | Null = null + var ex: Exception | Null = null // `fun` returns a LazyList that represents the state (head/tail) of `this`. We call `evaluated` to ensure // the result is initialized, to prevent races when reading `rawTail` / `rawHead` below. // Often, lazy lists are created with `newLL(eagerCons(...))` so `l` is already initialized, but `newLL` // also accepts non-evaluated lazy lists. val l = try fun().asInstanceOf[LazyList[A]].evaluated catch { - case t: Throwable => - ex = t + case e: Exception => + ex = e null } // update `_tail` before `_head`, because `_head` is used to test `isEvaluated` diff --git a/library/src/scala/collection/immutable/LazyListIterable.scala b/library/src/scala/collection/immutable/LazyListIterable.scala index 18015bc44575..50c90d3d7638 100644 --- a/library/src/scala/collection/immutable/LazyListIterable.scala +++ b/library/src/scala/collection/immutable/LazyListIterable.scala @@ -324,14 +324,14 @@ final class LazyListIterable[+A] private (lazyState: LazyListIterable.EmptyMarke // this way, there is no allocation in the common case where there's no race. // if multiple threads attempt to initialize a LazyList, an `InRace` instance is created to coordinate. if (_tailUpdater.compareAndSet(this, fun, Thread.currentThread)) { - var ex: Throwable | Null = null + var ex: Exception | Null = null // `fun` returns a LazyList that represents the state (head/tail) of `this`. We call `evaluated` to ensure // the result is initialized, to prevent races when reading `rawTail` / `rawHead` below. // Often, lazy lists are created with `newLL(eagerCons(...))` so `l` is already initialized, but `newLL` // also accepts non-evaluated lazy lists. val l = try fun.asInstanceOf[() ->{this} LazyListIterable[A]^{this}].apply().evaluated catch { - case t: Throwable => - ex = t + case e: Exception => + ex = e null } // update `_tail` before `_head`, because `_head` is used to test `isEvaluated` diff --git a/library/src/scala/concurrent/BatchingExecutor.scala b/library/src/scala/concurrent/BatchingExecutor.scala index 1f86859a5658..94f169390b7a 100644 --- a/library/src/scala/concurrent/BatchingExecutor.scala +++ b/library/src/scala/concurrent/BatchingExecutor.scala @@ -172,7 +172,7 @@ private[concurrent] trait BatchingExecutor extends Executor { runN(BatchingExecutorStatics.runLimit) null } catch { - case t: Throwable => t // We are handling exceptions on the outside of this method + case NonFatal(t) => t // We are handling exceptions on the outside of this method } finally { parentBlockContext = BatchingExecutorStatics.MissingParentBlockContext _tasksLocal.remove() @@ -186,12 +186,10 @@ private[concurrent] trait BatchingExecutor extends Executor { private final def resubmit(cause: Throwable | Null): Throwable | Null = if (this.size > 0) { try { submitForExecution(this); cause } catch { - case inner: Throwable => - if (NonFatal(inner)) { - val e = new ExecutionException("Non-fatal error occurred and resubmission failed, see suppressed exception.", cause) - e.addSuppressed(inner) - e - } else inner + case NonFatal(inner) => + val e = new ExecutionException("Non-fatal error occurred and resubmission failed, see suppressed exception.", cause) + e.addSuppressed(inner) + e } } else cause // TODO: consider if NonFatals should simply be `reportFailure`:ed rather than rethrown diff --git a/library/src/scala/concurrent/impl/FutureConvertersImpl.scala b/library/src/scala/concurrent/impl/FutureConvertersImpl.scala index b19865e10be9..e6951276daa2 100644 --- a/library/src/scala/concurrent/impl/FutureConvertersImpl.scala +++ b/library/src/scala/concurrent/impl/FutureConvertersImpl.scala @@ -19,6 +19,7 @@ import java.util.function.{BiConsumer, BiFunction, Consumer, Function => JFuncti import scala.concurrent.Future import scala.concurrent.impl.Promise.DefaultPromise import scala.util.{Failure, Success, Try} +import scala.util.control.NonFatal private[scala] object FutureConvertersImpl { final class CF[T](val wrapped: Future[T]) extends CompletableFuture[T] with (Try[T] => Unit) { @@ -62,7 +63,7 @@ private[scala] object FutureConvertersImpl { try { fn(e).asInstanceOf[AnyRef] } catch { - case thr: Throwable => + case NonFatal(thr) => cf.completeExceptionally(thr) this } diff --git a/library/src/scala/concurrent/impl/Promise.scala b/library/src/scala/concurrent/impl/Promise.scala index 1551592923f1..110545527d7b 100644 --- a/library/src/scala/concurrent/impl/Promise.scala +++ b/library/src/scala/concurrent/impl/Promise.scala @@ -490,7 +490,7 @@ private[concurrent] object Promise { val e = _ec try e.nn.execute(this) /* Safe publication of _arg, _fun, _ec */ catch { - case t: Throwable => + case NonFatal(t) => _fun = null // allow to GC _arg = null // see above _ec = null // see above again @@ -565,7 +565,7 @@ private[concurrent] object Promise { if (resolvedResult ne null) tryComplete0(get(), resolvedResult.asInstanceOf[Try[T]]) // T is erased anyway so we won't have any use for it above } catch { - case t: Throwable => handleFailure(t, ec) + case NonFatal(t) => handleFailure(t, ec) } } } diff --git a/library/src/scala/util/Using.scala b/library/src/scala/util/Using.scala index 5a218a56128c..d398401296c1 100644 --- a/library/src/scala/util/Using.scala +++ b/library/src/scala/util/Using.scala @@ -13,6 +13,7 @@ package scala.util import scala.language.`2.13` +import scala.annotation.nowarn import scala.util.control.{ControlThrowable, NonFatal} import scala.runtime.ScalaRunTime.nullForGC @@ -143,6 +144,7 @@ import scala.runtime.ScalaRunTime.nullForGC * @define suppressionBehavior See the main doc for [[Using `Using`]] for full details of * suppression behavior. */ +@nowarn("msg=Catching Throwable can lead to unexpected behavior") // backwards compat object Using { /** Performs an operation using a resource, and then releases the resource, * even if the operation throws an exception. diff --git a/library/test/scala/collection/IndexedSeqTest.scala b/library/test/scala/collection/IndexedSeqTest.scala index d77bb0fe898c..52ea2d71a1b1 100644 --- a/library/test/scala/collection/IndexedSeqTest.scala +++ b/library/test/scala/collection/IndexedSeqTest.scala @@ -226,7 +226,6 @@ abstract class IndexedTest[T, E] { val res = fn fail(s"expected exception was not thrown: $res") } catch { - case failed: AssertionError => throw failed case e: Exception if manifest[EX].runtimeClass.isAssignableFrom(e.getClass) => } } diff --git a/library/test/scala/collection/immutable/ListSetTest.scala b/library/test/scala/collection/immutable/ListSetTest.scala index b950cf60c7e8..14127acc08dc 100644 --- a/library/test/scala/collection/immutable/ListSetTest.scala +++ b/library/test/scala/collection/immutable/ListSetTest.scala @@ -1,8 +1,8 @@ package scala.collection.immutable -import tools.AssertUtil.{assertSameElements, fail} +import tools.AssertUtil.assertSameElements -import org.junit.Assert.{assertEquals, assertSame, fail => _} +import org.junit.Assert.{assertEquals, assertSame} import org.junit.Test class ListSetTest { @@ -22,9 +22,7 @@ class ListSetTest { @Test def hasTailRecursiveDelete(): Unit = { val s = ListSet(1 to 50000*) - try s - 25000 catch { - case e: StackOverflowError => fail("A stack overflow occurred") - } + s - 25000 // should not stack overflow } @Test diff --git a/library/test/scala/collection/immutable/TreeMapProps.scala b/library/test/scala/collection/immutable/TreeMapProps.scala index 8b79f578a23b..50b77b3d657a 100644 --- a/library/test/scala/collection/immutable/TreeMapProps.scala +++ b/library/test/scala/collection/immutable/TreeMapProps.scala @@ -6,6 +6,7 @@ import Gen.* import Arbitrary.* import util.* import Buildable.* +import scala.util.control.NonFatal object TreeMapProps extends Properties("TreeMap") { def genTreeMap[A: {Arbitrary, Ordering}, B: Arbitrary]: Gen[TreeMap[A, B]] = @@ -37,7 +38,7 @@ object TreeMapProps extends Properties("TreeMap") { val values = (1 to highest).reverse val subject = TreeMap(values zip values*) val it = subject.iterator - try { while (it.hasNext) it.next(); true } catch { case _: Throwable => false } + try { while (it.hasNext) it.next(); true } catch { case NonFatal(_) => false } } property("sorted") = forAll { (subject: TreeMap[Int, String]) => (subject.size >= 3) ==> { diff --git a/library/test/scala/collection/immutable/TreeSetProps.scala b/library/test/scala/collection/immutable/TreeSetProps.scala index 8e8be2ef5fff..05044b7bc534 100644 --- a/library/test/scala/collection/immutable/TreeSetProps.scala +++ b/library/test/scala/collection/immutable/TreeSetProps.scala @@ -4,6 +4,7 @@ import org.scalacheck.* import Prop.* import Gen.* import Arbitrary.* +import scala.util.control.NonFatal object TreeSetProps extends Properties("TreeSet") { def genTreeSet[A: {Arbitrary, Ordering}]: Gen[TreeSet[A]] = @@ -34,7 +35,7 @@ object TreeSetProps extends Properties("TreeSet") { val values = (1 to highest).reverse val subject = TreeSet(values*) val it = subject.iterator - try { while (it.hasNext) it.next(); true } catch { case _: Throwable => false } + try { while (it.hasNext) it.next(); true } catch { case NonFatal(_) => false } } property("sorted") = forAll { (subject: TreeSet[Int]) => (subject.size >= 3) ==> { diff --git a/library/test/scala/collection/immutable/VectorTest.scala b/library/test/scala/collection/immutable/VectorTest.scala index e1b484f9c345..2a8196e3d4a5 100644 --- a/library/test/scala/collection/immutable/VectorTest.scala +++ b/library/test/scala/collection/immutable/VectorTest.scala @@ -6,6 +6,7 @@ import org.junit.Test import scala.annotation.{nowarn, unused} import scala.collection.immutable.VectorInline.{WIDTH3, WIDTH4, WIDTH5} import scala.collection.mutable.{ListBuffer, StringBuilder} +import scala.util.control.NonFatal import tools.AssertUtil.intercept class VectorTest { @@ -663,7 +664,7 @@ object VectorUtils { def validateDebug[T](v: Vector[T]): Unit = { try validate(v) catch { - case ex: Throwable => + case NonFatal(ex) => throw new RuntimeException("Validation failed: " + ex.getMessage + "\n" + toDebugString(v), ex) } } diff --git a/library/test/scala/util/TryTest.scala b/library/test/scala/util/TryTest.scala index 21d1b3a898de..5019b5521b28 100644 --- a/library/test/scala/util/TryTest.scala +++ b/library/test/scala/util/TryTest.scala @@ -296,7 +296,7 @@ class TryTest { val res = try { t.fold(_ => throw new Exception("bar"), "Returns " + _) } catch { - case e: Throwable => "Throws " + e + case e: Exception => "Throws " + e } assertEquals("Throws java.lang.Exception: bar", res) } diff --git a/library/test/scala/util/UsingTest.scala b/library/test/scala/util/UsingTest.scala index d785112f1bb4..50ce219dba7d 100644 --- a/library/test/scala/util/UsingTest.scala +++ b/library/test/scala/util/UsingTest.scala @@ -3,10 +3,11 @@ package scala.util import org.junit.Test import org.junit.Assert.* -import scala.annotation.unused +import scala.annotation.{nowarn, unused} import scala.reflect.ClassTag import scala.util.control.ControlThrowable +@nowarn("msg=unexpected behavior") // catching Throwables; but Using must, because of backwards compat @deprecated("ThreadDeath is deprecated on JDK 20", "") class UsingTest { import UsingTest.* @@ -722,6 +723,7 @@ class UsingTest { } } +@nowarn("msg=unexpected behavior") // catching Throwables; but Using must, because of backwards compat @deprecated("ThreadDeath is deprecated on JDK 20", "") object UsingTest { final class ClosingVMError(message: String) extends VirtualMachineError(message) diff --git a/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala b/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala index b43849aba28b..8011fff4dacf 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala @@ -31,9 +31,6 @@ class CompilerSearchVisitor( try (sym != NoSymbol && sym.isAccessibleFrom(ctx.owner.info) && sym.isStatic) || isAccessibleImplicitClass(sym) catch - case err: AssertionError => - logger.log(Level.WARNING, err.getMessage()) - false case NonFatal(e) => reports.incognito.create(() => Report( diff --git a/project/scripts/bisect.test.scala b/project/scripts/bisect.test.scala index 5f139b6f0c7f..86361275083d 100644 --- a/project/scripts/bisect.test.scala +++ b/project/scripts/bisect.test.scala @@ -106,7 +106,7 @@ class BisectOptionsTest extends munit.FunSuite: } test("invalid boolean value throws") { - intercept[Throwable] { + intercept[Exception] { parse("--dry-run=maybe", "compile", "foo.scala") } } diff --git a/repl/src/dotty/tools/repl/Rendering.scala b/repl/src/dotty/tools/repl/Rendering.scala index d4aa401d2e0e..de0339f52703 100644 --- a/repl/src/dotty/tools/repl/Rendering.scala +++ b/repl/src/dotty/tools/repl/Rendering.scala @@ -8,6 +8,7 @@ import printing.SyntaxHighlighting import reporting.Diagnostic import StackTraceOps.* +import scala.annotation.nowarn import scala.compiletime.uninitialized import scala.jdk.CollectionConverters.* import org.objectweb.asm.* @@ -319,6 +320,8 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): end renderVal /** Force module initialization in the absence of members. */ + // the module statements are executing in can fail to initialize if there's a problem + @nowarn("msg=Catching ExceptionInInitializerError can lead to unexpected behavior") def forceModule(sym: Symbol)(using Context): Seq[Diagnostic] = def load() = val objectName = sym.fullName.encode.toString diff --git a/repl/src/dotty/tools/repl/ReplDriver.scala b/repl/src/dotty/tools/repl/ReplDriver.scala index 38f61a90a3a3..30b3eb5829f8 100644 --- a/repl/src/dotty/tools/repl/ReplDriver.scala +++ b/repl/src/dotty/tools/repl/ReplDriver.scala @@ -51,6 +51,7 @@ import scala.compiletime.uninitialized import scala.jdk.CollectionConverters.* import org.objectweb.asm.ClassReader import scala.util.Using +import scala.util.control.NonFatal /** The state of the REPL contains necessary bindings instead of having to have * mutation @@ -728,7 +729,7 @@ class ReplDriver(settings: Array[String], out.println(s"Added '$path' to classpath.") } catch { - case e: Throwable => + case NonFatal(e) => out.println(s"Failed to load '$path' to classpath: ${e.getMessage}") } state diff --git a/repl/src/dotty/tools/repl/ScalaClassLoader.scala b/repl/src/dotty/tools/repl/ScalaClassLoader.scala index 417d5a19cdfe..8eb8dd77c498 100644 --- a/repl/src/dotty/tools/repl/ScalaClassLoader.scala +++ b/repl/src/dotty/tools/repl/ScalaClassLoader.scala @@ -8,7 +8,7 @@ import java.lang.reflect.{ InvocationTargetException, UndeclaredThrowableExcepti import scala.annotation.internal.sharable import scala.annotation.tailrec -import scala.util.control.Exception.catching +import scala.util.control.NonFatal object ScalaClassLoader { def setContext(cl: ClassLoader) = Thread.currentThread.setContextClassLoader(cl) @@ -20,7 +20,7 @@ object ScalaClassLoader { if scala.util.Properties.isJavaAtLeast("9") then try ClassLoader.getSystemClassLoader.getParent - catch case _: Throwable => null + catch case NonFatal(_) => null else null extension (classLoader: ClassLoader) diff --git a/repl/test-resources/repl-macros/i5551 b/repl/test-resources/repl-macros/i5551 index fe4e73ffa5d5..c72d29a4b60d 100644 --- a/repl/test-resources/repl-macros/i5551 +++ b/repl/test-resources/repl-macros/i5551 @@ -1,10 +1,12 @@ scala> import scala.quoted._ -scala> def assertImpl(expr: Expr[Boolean])(using q: Quotes) = '{ if !($expr) then throw new AssertionError("failed assertion")} +scala> class MyException extends Exception { } +// defined class MyException +scala> def assertImpl(expr: Expr[Boolean])(using q: Quotes) = '{ if !($expr) then throw new MyException()} def assertImpl(expr: Expr[Boolean])(using q: Quotes): Expr[Unit] scala> inline def assert(expr: => Boolean): Unit = ${ assertImpl('{expr}) } def assert(expr: => Boolean): Unit scala> assert(0 == 0) -scala> try assert(0 == 1) catch { case _: AssertionError => println("ok") } +scala> try assert(0 == 1) catch { case _: MyException => println("ok") } ok diff --git a/repl/test/dotty/tools/repl/ReplInteractiveTests.scala b/repl/test/dotty/tools/repl/ReplInteractiveTests.scala index b95d2b28f144..796792c379e3 100644 --- a/repl/test/dotty/tools/repl/ReplInteractiveTests.scala +++ b/repl/test/dotty/tools/repl/ReplInteractiveTests.scala @@ -2,6 +2,7 @@ package dotty.tools package repl import scala.language.unsafeNulls +import scala.util.control.NonFatal import java.io.{ByteArrayOutputStream, PipedInputStream, PipedOutputStream} import java.nio.charset.StandardCharsets @@ -125,7 +126,7 @@ class ReplInteractiveTests: waitsForMore finally executor.shutdownNow() - try jlt.close() catch case _: Throwable => () + try jlt.close() catch case NonFatal(_) => () @Test def `command then incomplete code is not submitted as one line`(): Unit = val incomplete = ":settings -deprecation\nif true then" diff --git a/repl/test/dotty/tools/repl/ReplTest.scala b/repl/test/dotty/tools/repl/ReplTest.scala index 99edb638136c..4b18460d09f8 100644 --- a/repl/test/dotty/tools/repl/ReplTest.scala +++ b/repl/test/dotty/tools/repl/ReplTest.scala @@ -15,6 +15,7 @@ import java.nio.charset.StandardCharsets import scala.io.Source import scala.util.Using +import scala.util.control.NonFatal import scala.collection.mutable.ArrayBuffer import dotc.core.Contexts.Context @@ -72,7 +73,7 @@ extends ReplDriver(options, new PrintStream(out, true, StandardCharsets.UTF_8.na (out, nstate) } catch { - case ex: Throwable => + case NonFatal(ex) => System.err.println(s"failed while running script: $name, on:\n$input") throw ex } diff --git a/repl/test/dotty/tools/repl/StackTraceTest.scala b/repl/test/dotty/tools/repl/StackTraceTest.scala index 03863ffce1cc..50a385f3bd74 100644 --- a/repl/test/dotty/tools/repl/StackTraceTest.scala +++ b/repl/test/dotty/tools/repl/StackTraceTest.scala @@ -5,6 +5,7 @@ import scala.language.unsafeNulls import scala.util.{Failure, Success, Try} import scala.util.chaining.given +import scala.util.control.NonFatal import org.junit.Assert.{assertEquals, assertTrue} import org.junit.Test @@ -18,20 +19,20 @@ class StackTraceTest: def sampler: String = sample // repackage with message - def resample: String = try sample catch case e: Throwable => throw new RuntimeException("resample", e) + def resample: String = try sample catch case NonFatal(e) => throw new RuntimeException("resample", e) def resampler: String = resample // simple wrapper - def wrapper: String = try sample catch case e: Throwable => throw new RuntimeException(e) + def wrapper: String = try sample catch case NonFatal(e) => throw new RuntimeException(e) // another onion skin - def rewrapper: String = try wrapper catch case e: Throwable => throw new RuntimeException(e) + def rewrapper: String = try wrapper catch case NonFatal(e) => throw new RuntimeException(e) def rewrapperer: String = rewrapper // circular cause - def insane: String = try sample catch case e: Throwable => throw new RuntimeException(e).tap(e.initCause) + def insane: String = try sample catch case NonFatal(e) => throw new RuntimeException(e).tap(e.initCause) def insaner: String = insane - def repressed: String = try sample catch case e: Throwable => throw new RuntimeException("My problem").tap(_.addSuppressed(e)) + def repressed: String = try sample catch case NonFatal(e) => throw new RuntimeException("My problem").tap(_.addSuppressed(e)) def represser: String = repressed // evaluating s should throw, p trims stack trace, t is the test of resulting trace string diff --git a/scaladoc/src/dotty/tools/scaladoc/site/LoadedTemplate.scala b/scaladoc/src/dotty/tools/scaladoc/site/LoadedTemplate.scala index 15e168467c8a..a3606af2f1e0 100644 --- a/scaladoc/src/dotty/tools/scaladoc/site/LoadedTemplate.scala +++ b/scaladoc/src/dotty/tools/scaladoc/site/LoadedTemplate.scala @@ -7,7 +7,7 @@ import java.nio.file.Paths import org.jsoup.Jsoup import scala.jdk.CollectionConverters._ - +import scala.util.control.NonFatal case class LazyEntry(getKey: String, value: () => String) extends JMapEntry[String, Object]: lazy val getValue: Object = value() @@ -24,7 +24,7 @@ case class LoadedTemplate( val code = Jsoup.parse(resolveToHtml(ctx).code) Option(code.select("p").first()).fold("...")(_.outerHtml()) catch - case e: Throwable => + case NonFatal(e) => val msg = s"[ERROR] Unable to process brief for ${templateFile.file}" report.error(msg, templateFile.file, e)(using ctx.outerCtx) "..." diff --git a/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetChecker.scala b/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetChecker.scala index 3150335e8d95..b9adbbebd00a 100644 --- a/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetChecker.scala +++ b/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetChecker.scala @@ -13,7 +13,8 @@ class SnippetChecker(val args: Scaladoc.Args)(using cctx: CompilerContext): args.tastyFiles .map(_.getAbsolutePath()) .map(AbstractFile.getFile(_).nn) - .flatMap(t => try TastyFileUtil.getClassPath(t) catch case _: AssertionError => Seq.empty) + .filter(t => t.exists && t.ext.isTasty) + .flatMap(t => TastyFileUtil.getClassPath(t)) .distinct .mkString(sep), args.classpath diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala index 9bb739e15246..1c00f99862fc 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala @@ -5,6 +5,7 @@ package tasty import java.util.regex.Pattern import scala.util.{Try, Success, Failure} +import scala.util.control.NonFatal import scala.tasty.inspector.{ScaladocInternalTastyInspector, Inspector, Tasty} import scala.quoted._ @@ -243,7 +244,7 @@ case class TastyParser( seen = seen.tail try Traverser.traverseTree(root)(Symbol.spliceOwner) - catch case e: Throwable => + catch case NonFatal(e) => report.error(s"Problem parsing ${root.pos}, documentation may not be generated. (Error message: ${e.getMessage})") // e.printStackTrace() diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala index ab65711331e5..eb1b2fc17321 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala @@ -402,7 +402,7 @@ trait TypesSupport: case t: dotty.tools.dotc.core.Types.LazyRef => try { inner(t.ref(using ctx.compilerContext).asInstanceOf[TypeRepr], skipThisTypePrefix) } catch { - case e: AssertionError => tpe("LazyRef(...)").l + case NonFatal(_) => tpe("LazyRef(...)").l } case tpe => diff --git a/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala b/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala index fc215b1de229..b78911b794e4 100644 --- a/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala +++ b/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala @@ -34,7 +34,7 @@ class ReportingTest: } @Test - def errorsInCaseOfIncompletClasspath = + def errorsInCaseOfIncompleteClasspath = val notTasty = Files.createTempFile("broken", ".notTasty") try Files.write(notTasty, "Random file".getBytes) diff --git a/tests/warn/catch.check b/tests/warn/catch.check new file mode 100644 index 000000000000..a48e64a930b6 --- /dev/null +++ b/tests/warn/catch.check @@ -0,0 +1,18 @@ +-- [E233] Potential Issue Warning: tests/warn/catch.scala:12:14 -------------------------------------------------------- +12 | case _: Throwable => () // warn + | ^^^^^^^^^ + | Catching Throwable can lead to unexpected behavior + | + | longer explanation available when compiling with `-explain` +-- [E233] Potential Issue Warning: tests/warn/catch.scala:17:14 -------------------------------------------------------- +17 | case _: Error => () // warn + | ^^^^^ + | Catching Error can lead to unexpected behavior + | + | longer explanation available when compiling with `-explain` +-- [E233] Potential Issue Warning: tests/warn/catch.scala:22:14 -------------------------------------------------------- +22 | case _: AssertionError => () // warn + | ^^^^^^^^^^^^^^ + | Catching AssertionError can lead to unexpected behavior + | + | longer explanation available when compiling with `-explain` diff --git a/tests/warn/catch.scala b/tests/warn/catch.scala new file mode 100644 index 000000000000..e3f2b45ec593 --- /dev/null +++ b/tests/warn/catch.scala @@ -0,0 +1,38 @@ +//> using options -Wunreasonable-catch + +import scala.util.control.* + +object O: + def foo() = ??? + + def m(): Unit = + try + foo() + catch + case _: Throwable => () // warn + + try + foo() + catch + case _: Error => () // warn + + try + foo() + catch + case _: AssertionError => () // warn + + try + foo() + catch + case NonFatal(_) => () // ok + + try + foo() + catch + case _: Exception => () // ok + + try + foo() + catch + case _: IndexOutOfBoundsException => () // ok +