From 120e14fadf30b4c39f953832108d19b736dc6f2d Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Tue, 2 Oct 2012 11:21:16 -0700 Subject: Fix for rangepos crasher. wrapClassTagUnapply was generating an unpositioned tree which would crash under -Yrangepos. See SI-6338. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 5 +++-- test/files/neg/t3015.check | 5 +---- test/files/pos/classtag-pos.flags | 1 + test/files/pos/classtag-pos.scala | 5 +++++ 4 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 test/files/pos/classtag-pos.flags create mode 100644 test/files/pos/classtag-pos.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 12e26a812d..cf9a07a7e4 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -3298,12 +3298,13 @@ trait Typers extends Modes with Adaptations with Tags { // println(util.Position.formatMessage(uncheckedPattern.pos, "made unchecked type test into a checked one", true)) val args = List(uncheckedPattern) + val app = atPos(uncheckedPattern.pos)(Apply(classTagExtractor, args)) // must call doTypedUnapply directly, as otherwise we get undesirable rewrites // and re-typechecks of the target of the unapply call in PATTERNmode, // this breaks down when the classTagExtractor (which defineds the unapply member) is not a simple reference to an object, // but an arbitrary tree as is the case here - doTypedUnapply(Apply(classTagExtractor, args), classTagExtractor, classTagExtractor, args, PATTERNmode, pt) - } + doTypedUnapply(app, classTagExtractor, classTagExtractor, args, PATTERNmode, pt) + } // if there's a ClassTag that allows us to turn the unchecked type test for `pt` into a checked type test // return the corresponding extractor (an instance of ClassTag[`pt`]) diff --git a/test/files/neg/t3015.check b/test/files/neg/t3015.check index 4a03c940f4..6948392bb0 100644 --- a/test/files/neg/t3015.check +++ b/test/files/neg/t3015.check @@ -3,7 +3,4 @@ t3015.scala:7: error: scrutinee is incompatible with pattern type; required: String val b(foo) = "foo" ^ -error: type mismatch; - found : _$1 - required: String -two errors found +one error found diff --git a/test/files/pos/classtag-pos.flags b/test/files/pos/classtag-pos.flags new file mode 100644 index 0000000000..281f0a10cd --- /dev/null +++ b/test/files/pos/classtag-pos.flags @@ -0,0 +1 @@ +-Yrangepos diff --git a/test/files/pos/classtag-pos.scala b/test/files/pos/classtag-pos.scala new file mode 100644 index 0000000000..768d2e27f4 --- /dev/null +++ b/test/files/pos/classtag-pos.scala @@ -0,0 +1,5 @@ +import scala.reflect.runtime.universe._ + +class A { + def f[T: TypeTag] = typeOf[T] match { case TypeRef(_, _, args) => args } +} -- cgit v1.2.3 From 1f99df2c66cb1933dd4db74aa872497a4e26975b Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Tue, 2 Oct 2012 11:24:24 -0700 Subject: Eliminated pattern matcher warning. Implicit extractor must be available to pattern match on abstract type. Requires prior commit not to crash under -Yrangepos. --- src/compiler/scala/tools/nsc/interpreter/TypeStrings.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/scala/tools/nsc/interpreter/TypeStrings.scala b/src/compiler/scala/tools/nsc/interpreter/TypeStrings.scala index fe1c4c0ca8..0bf4999fd6 100644 --- a/src/compiler/scala/tools/nsc/interpreter/TypeStrings.scala +++ b/src/compiler/scala/tools/nsc/interpreter/TypeStrings.scala @@ -212,6 +212,7 @@ trait TypeStrings { } private def tparamString[T: ru.TypeTag] : String = { + import ru._ def typeArguments: List[ru.Type] = ru.typeOf[T] match { case ru.TypeRef(_, _, args) => args; case _ => Nil } // [Eugene to Paul] need to use not the `rootMirror`, but a mirror with the REPL's classloader // how do I get to it? acquiring context classloader seems unreliable because of multithreading @@ -256,4 +257,4 @@ trait TypeStrings { ) } -object TypeStrings extends TypeStrings { } \ No newline at end of file +object TypeStrings extends TypeStrings { } -- cgit v1.2.3 From d7354838948be58b8045e1218a9c757d9b90df76 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Tue, 2 Oct 2012 10:37:13 -0700 Subject: Fix for spurious warning. Eliminates spurious "catch block may intercept non-local return" seen in recent builds of master. Unified some catch logic in TreeInfo, and removed some which never worked. --- .../scala/tools/nsc/transform/UnCurry.scala | 19 +++++++-------- .../scala/tools/nsc/typechecker/Typers.scala | 14 +++++------ src/reflect/scala/reflect/internal/TreeInfo.scala | 28 +++++++++++++++------- test/files/neg/catch-all.check | 9 ++++--- test/files/neg/nonlocal-warning.check | 3 ++- test/files/neg/nonlocal-warning.scala | 11 +++++++++ 6 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index f68cbfc141..ea93ad1bd4 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -205,11 +205,8 @@ abstract class UnCurry extends InfoTransform val keyDef = ValDef(key, New(ObjectClass.tpe)) val tryCatch = Try(body, pat -> rhs) - body foreach { - case Try(t, catches, _) if catches exists treeInfo.catchesThrowable => - unit.warning(body.pos, "catch block may intercept non-local return from " + meth) - case _ => - } + for (Try(t, catches, _) <- body ; cdef <- catches ; if treeInfo catchesThrowable cdef) + unit.warning(body.pos, "catch block may intercept non-local return from " + meth) Block(List(keyDef), tryCatch) } @@ -691,16 +688,16 @@ abstract class UnCurry extends InfoTransform else tree } - + def isThrowable(pat: Tree): Boolean = pat match { - case Typed(Ident(nme.WILDCARD), tpt) => + case Typed(Ident(nme.WILDCARD), tpt) => tpt.tpe =:= ThrowableClass.tpe - case Bind(_, pat) => + case Bind(_, pat) => isThrowable(pat) case _ => false } - + def isDefaultCatch(cdef: CaseDef) = isThrowable(cdef.pat) && cdef.guard.isEmpty def postTransformTry(tree: Try) = { @@ -764,10 +761,10 @@ abstract class UnCurry extends InfoTransform case tree: Try => postTransformTry(tree) - + case Apply(Apply(fn, args), args1) => treeCopy.Apply(tree, fn, args ::: args1) - + case Ident(name) => assert(name != tpnme.WILDCARD_STAR, tree) applyUnary() diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index cf9a07a7e4..9c6f6a0f99 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5133,14 +5133,12 @@ trait Typers extends Modes with Adaptations with Tags { var block1 = typed(tree.block, pt) var catches1 = typedCases(tree.catches, ThrowableClass.tpe, pt) - for (cdef <- catches1 if cdef.guard.isEmpty) { - def warn(name: Name) = context.warning(cdef.pat.pos, s"This catches all Throwables. If this is really intended, use `case ${name.decoded} : Throwable` to clear this warning.") - def unbound(t: Tree) = t.symbol == null || t.symbol == NoSymbol - cdef.pat match { - case Bind(name, i @ Ident(_)) if unbound(i) => warn(name) - case i @ Ident(name) if unbound(i) => warn(name) - case _ => - } + for (cdef <- catches1; if treeInfo catchesThrowable cdef) { + val name = (treeInfo assignedNameOfPattern cdef).decoded + context.warning(cdef.pat.pos, + s"""|This catches all Throwables, which often has undesirable consequences. + |If intentional, use `case $name : Throwable` to clear this warning.""".stripMargin + ) } val finalizer1 = diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index db062d138f..d4a22886dd 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -397,19 +397,31 @@ abstract class TreeInfo { case _ => false } - /** Does this CaseDef catch Throwable? */ - def catchesThrowable(cdef: CaseDef) = catchesAllOf(cdef, ThrowableClass.tpe) + private def hasNoSymbol(t: Tree) = t.symbol == null || t.symbol == NoSymbol - /** Does this CaseDef catch everything of a certain Type? */ - def catchesAllOf(cdef: CaseDef, threshold: Type) = { - def unbound(t: Tree) = t.symbol == null || t.symbol == NoSymbol + /** If this CaseDef assigns a name to its top-level pattern, + * in the form 'expr @ pattern' or 'expr: pattern', returns + * the name. Otherwise, nme.NO_NAME. + * + * Note: in the case of Constant patterns such as 'case x @ "" =>', + * the pattern matcher eliminates the binding and inlines the constant, + * so as far as this method is likely to be able to determine, + * the name is NO_NAME. + */ + def assignedNameOfPattern(cdef: CaseDef): Name = cdef.pat match { + case Bind(name, _) => name + case Ident(name) => name + case _ => nme.NO_NAME + } + + /** Does this CaseDef catch Throwable? */ + def catchesThrowable(cdef: CaseDef) = ( cdef.guard.isEmpty && (unbind(cdef.pat) match { case Ident(nme.WILDCARD) => true - case i@Ident(name) => unbound(i) - case Typed(_, tpt) => (tpt.tpe != null) && (threshold <:< tpt.tpe) + case i@Ident(name) => hasNoSymbol(i) case _ => false }) - } + ) /** Is this pattern node a catch-all or type-test pattern? */ def isCatchCase(cdef: CaseDef) = cdef match { diff --git a/test/files/neg/catch-all.check b/test/files/neg/catch-all.check index aaf51480c3..2d58dd99a8 100644 --- a/test/files/neg/catch-all.check +++ b/test/files/neg/catch-all.check @@ -1,10 +1,13 @@ -catch-all.scala:2: warning: This catches all Throwables. If this is really intended, use `case _ : Throwable` to clear this warning. +catch-all.scala:2: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case _ : Throwable` to clear this warning. try { "warn" } catch { case _ => } ^ -catch-all.scala:4: warning: This catches all Throwables. If this is really intended, use `case x : Throwable` to clear this warning. +catch-all.scala:4: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case x : Throwable` to clear this warning. try { "warn" } catch { case x => } ^ -catch-all.scala:6: warning: This catches all Throwables. If this is really intended, use `case x : Throwable` to clear this warning. +catch-all.scala:6: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case x : Throwable` to clear this warning. try { "warn" } catch { case _: RuntimeException => ; case x => } ^ error: No warnings can be incurred under -Xfatal-warnings. diff --git a/test/files/neg/nonlocal-warning.check b/test/files/neg/nonlocal-warning.check index 5202df655a..67b3b10095 100644 --- a/test/files/neg/nonlocal-warning.check +++ b/test/files/neg/nonlocal-warning.check @@ -1,4 +1,5 @@ -nonlocal-warning.scala:4: warning: This catches all Throwables. If this is really intended, use `case x : Throwable` to clear this warning. +nonlocal-warning.scala:4: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case x : Throwable` to clear this warning. catch { case x => 11 } ^ nonlocal-warning.scala:2: warning: catch block may intercept non-local return from method foo diff --git a/test/files/neg/nonlocal-warning.scala b/test/files/neg/nonlocal-warning.scala index cc98bd631a..f908a86302 100644 --- a/test/files/neg/nonlocal-warning.scala +++ b/test/files/neg/nonlocal-warning.scala @@ -4,4 +4,15 @@ class Foo { catch { case x => 11 } 22 } + + val pf: PartialFunction[Throwable, Unit] = { + case x if false => () + } + + def bar(l: List[Int]): Int = { + try l foreach { _ => return 5 } + catch pf + finally println() + 22 + } } -- cgit v1.2.3