From 88c2994f4cdd750e179bebfa352237875c5eedbd Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sun, 19 Jan 2014 21:53:38 +0300 Subject: quasiquotes no longer evaluate debug logs when debug logging is off --- src/compiler/scala/tools/reflect/quasiquotes/Quasiquotes.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/reflect/quasiquotes/Quasiquotes.scala b/src/compiler/scala/tools/reflect/quasiquotes/Quasiquotes.scala index 7d777ef7d5..3e703924e8 100644 --- a/src/compiler/scala/tools/reflect/quasiquotes/Quasiquotes.scala +++ b/src/compiler/scala/tools/reflect/quasiquotes/Quasiquotes.scala @@ -11,7 +11,7 @@ abstract class Quasiquotes extends Parsers val global: c.universe.type = c.universe import c.universe._ - def debug(msg: String): Unit = + def debug(msg: => String): Unit = if (settings.Yquasiquotedebug.value) println(msg) lazy val (universe: Tree, args, parts, parse, reify, method) = c.macroApplication match { @@ -48,7 +48,7 @@ abstract class Quasiquotes extends Parsers val tree = parse(code) debug(s"parsed:\n${showRaw(tree)}\n$tree\n") val reified = reify(tree) - val sreified = + def sreified = reified .toString .replace("scala.reflect.runtime.`package`.universe.build.", "") -- cgit v1.2.3 From 6a6b485fe98890f73a03753e3981be5fa580ed02 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sun, 19 Jan 2014 22:05:52 +0300 Subject: fixes a typo in Types.scala --- src/reflect/scala/reflect/internal/Types.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index d0c322ca6b..92beffb761 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1456,7 +1456,7 @@ trait Types override def safeToString = scalaNotation(_.toString) - /** Bounds notation used in Scala sytanx. + /** Bounds notation used in Scala syntax. * For example +This <: scala.collection.generic.Sorted[K,This]. */ private[internal] def scalaNotation(typeString: Type => String): String = { -- cgit v1.2.3 From 936d60a2621088ba463a44c2f2d6450986022169 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sun, 19 Jan 2014 23:00:09 +0300 Subject: SI-8158 compiler hangs printing out fancy types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparently, even though the compiler has safeguards against infinite type printouts, having a depth counter, we didn’t account for the cases when printouts are both self-referential and self-multiplying. For one, SI-8158 provides an example of such a type, which is a structural type that refers to itself twice in return types of its methods. At first, printing such a type would go deeper and deeper, but then it will hit the depth limit and start multiply indefinitely. This commit fixes this particular problem by recognizing self-references as this.type’s and printing them out as such. The subsequent commit will introduce a more general facility. --- src/reflect/scala/reflect/internal/Symbols.scala | 13 ++++++--- test/files/neg/t8158.check | 4 +++ test/files/neg/t8158/Macros_1.scala | 34 ++++++++++++++++++++++++ test/files/neg/t8158/Test_2.scala | 14 ++++++++++ test/files/run/t5256h.check | 2 +- 5 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 test/files/neg/t8158.check create mode 100644 test/files/neg/t8158/Macros_1.scala create mode 100644 test/files/neg/t8158/Test_2.scala (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index c2689fe7e9..54e5b2781e 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -2519,6 +2519,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => if (settings.debug.value) parentsString(tp.parents) else briefParentsString(tp.parents) ) + def isStructuralThisType = ( + // prevents disasters like SI-8158 + owner.isInitialized && owner.isStructuralRefinement && tp == owner.tpe + ) if (isType) typeParamsString(tp) + ( if (isClass) " extends " + parents else if (isAliasType) " = " + tp.resultType @@ -2529,10 +2533,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => ) else if (isModule) "" // avoid "object X of type X.type" else tp match { - case PolyType(tparams, res) => typeParamsString(tp) + infoString(res) - case NullaryMethodType(res) => infoString(res) - case MethodType(params, res) => valueParamsString(tp) + infoString(res) - case _ => ": " + tp + case PolyType(tparams, res) => typeParamsString(tp) + infoString(res) + case NullaryMethodType(res) => infoString(res) + case MethodType(params, res) => valueParamsString(tp) + infoString(res) + case _ if isStructuralThisType => ": this.type" + case _ => ": " + tp } } diff --git a/test/files/neg/t8158.check b/test/files/neg/t8158.check new file mode 100644 index 0000000000..aaa62ddea4 --- /dev/null +++ b/test/files/neg/t8158.check @@ -0,0 +1,4 @@ +Test_2.scala:10: error: not enough patterns for <$anon: AnyRef> offering AnyRef{def isEmpty: Boolean; def get: this.type; def unapply(x: String): this.type}: expected 1, found 0 + case X() => + ^ +one error found diff --git a/test/files/neg/t8158/Macros_1.scala b/test/files/neg/t8158/Macros_1.scala new file mode 100644 index 0000000000..b84e3ed8d3 --- /dev/null +++ b/test/files/neg/t8158/Macros_1.scala @@ -0,0 +1,34 @@ +import scala.language.experimental.macros +import scala.reflect.macros.whitebox.Context + +object Max { + def impl(c: Context)(any: c.Expr[Any]): c.Expr[Any] = { + import c.universe._ + def fail(msg: String) = c.abort(c.enclosingPosition, msg) + val t = c.macroApplication match { + case q"$_.unapply($unargs)" => + /* hangs + */ + q""" + new { + def isEmpty = false + def get = this + def unapply(x: String) = this + }.unapply($unargs) + """ + /* + if get returns Unit or Boolean: + wrong number of patterns for <$anon: AnyRef> offering Unit: expected 1, found 0 + */ + /* straightforward + q""" + new { + def unapply(x: String) = true + }.unapply($unargs) + """ + */ + case _ => fail("bad appl") + } + c.Expr[Any](t) + } +} \ No newline at end of file diff --git a/test/files/neg/t8158/Test_2.scala b/test/files/neg/t8158/Test_2.scala new file mode 100644 index 0000000000..f5ac6616bb --- /dev/null +++ b/test/files/neg/t8158/Test_2.scala @@ -0,0 +1,14 @@ +import scala.language.experimental.macros + +object X { + def unapply(any: Any): Any = macro Max.impl +} + +class BugTest { + def bug(): Unit = { + "any" match { + case X() => + case _ => ??? + } + } +} \ No newline at end of file diff --git a/test/files/run/t5256h.check b/test/files/run/t5256h.check index 1a4a92a684..300a40f653 100644 --- a/test/files/run/t5256h.check +++ b/test/files/run/t5256h.check @@ -3,5 +3,5 @@ Test.$anon$1 java.lang.Object { final private val x: Int def x(): Int - def (): java.lang.Object{def x(): Int} + def (): this.type } -- cgit v1.2.3 From 35300b46311c93c692744701275dbb4807f851e2 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sun, 19 Jan 2014 22:25:20 +0300 Subject: introduces failsafe against endless type printing The parent commit works around a particular problem that led to a compiler freeze in SI-8158, whereas this commit introduces a general solution - a cache that tracks all types that we've recursed into during printing. I can't immediately come up with an example of a type that would be caught by this safety net, but unknown unknowns are the worst of them all, so why not guard against them while we can. --- src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala | 13 ++++++++++++- src/reflect/scala/reflect/runtime/JavaUniverseForce.scala | 1 + src/reflect/scala/reflect/runtime/SynchronizedTypes.scala | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala b/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala index ebc4394d25..b08aabef85 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala @@ -3,6 +3,8 @@ package reflect package internal package tpe +import scala.collection.mutable.HashSet + private[internal] trait TypeToStrings { self: SymbolTable => @@ -14,8 +16,15 @@ private[internal] trait TypeToStrings { def tostringRecursions = _tostringRecursions def tostringRecursions_=(value: Int) = _tostringRecursions = value + private var _tostringSubjects = HashSet[Type]() + def tostringSubjects = _tostringSubjects + protected def typeToString(tpe: Type): String = - if (tostringRecursions >= maxTostringRecursions) { + if (tostringSubjects contains tpe) { + // handles self-referential anonymous classes and who knows what else + "..." + } + else if (tostringRecursions >= maxTostringRecursions) { devWarning("Exceeded recursion depth attempting to print " + util.shortClassOfInstance(tpe)) if (settings.debug) (new Throwable).printStackTrace @@ -25,8 +34,10 @@ private[internal] trait TypeToStrings { else try { tostringRecursions += 1 + tostringSubjects += tpe tpe.safeToString } finally { + tostringSubjects -= tpe tostringRecursions -= 1 } } diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index 6b3985d434..ac6dd0783d 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -42,6 +42,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => // inaccessible: this._glbResults // inaccessible: this._indent // inaccessible: this._tostringRecursions + // inaccessible: this._tostringSubjects // inaccessible: this.atomicIds // inaccessible: this.atomicExistentialIds // inaccessible: this._recursionTable diff --git a/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala b/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala index de78e527a7..12ada07a56 100644 --- a/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala +++ b/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala @@ -85,6 +85,9 @@ private[reflect] trait SynchronizedTypes extends internal.Types { self: SymbolTa override def tostringRecursions = _tostringRecursions.get override def tostringRecursions_=(value: Int) = _tostringRecursions.set(value) + private lazy val _tostringSubjects = mkThreadLocalStorage(new mutable.HashSet[Type]) + override def tostringSubjects = _tostringSubjects.get + /* The idea of caches is as follows. * When in reflexive mode, a cache is either null, or one sentinal * value representing undefined or the final defined -- cgit v1.2.3 From 2d0aaf555dc3d43e245162a73ff06ed99667a6ee Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Mon, 20 Jan 2014 15:59:21 +0300 Subject: capitalizes “s” in tostring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As suggested by the reviewers, tostringXXX variables in TypeToStrings.scala have been renamed to toStringXXX. --- .../scala/reflect/internal/tpe/TypeToStrings.scala | 24 +++++++++++----------- .../scala/reflect/runtime/JavaUniverseForce.scala | 4 ++-- .../scala/reflect/runtime/SynchronizedTypes.scala | 10 ++++----- 3 files changed, 19 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala b/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala index b08aabef85..ceb667365e 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala @@ -10,21 +10,21 @@ private[internal] trait TypeToStrings { /** The maximum number of recursions allowed in toString */ - final val maxTostringRecursions = 50 + final val maxToStringRecursions = 50 - private var _tostringRecursions = 0 - def tostringRecursions = _tostringRecursions - def tostringRecursions_=(value: Int) = _tostringRecursions = value + private var _toStringRecursions = 0 + def toStringRecursions = _toStringRecursions + def toStringRecursions_=(value: Int) = _toStringRecursions = value - private var _tostringSubjects = HashSet[Type]() - def tostringSubjects = _tostringSubjects + private var _toStringSubjects = HashSet[Type]() + def toStringSubjects = _toStringSubjects protected def typeToString(tpe: Type): String = - if (tostringSubjects contains tpe) { + if (toStringSubjects contains tpe) { // handles self-referential anonymous classes and who knows what else "..." } - else if (tostringRecursions >= maxTostringRecursions) { + else if (toStringRecursions >= maxToStringRecursions) { devWarning("Exceeded recursion depth attempting to print " + util.shortClassOfInstance(tpe)) if (settings.debug) (new Throwable).printStackTrace @@ -33,11 +33,11 @@ private[internal] trait TypeToStrings { } else try { - tostringRecursions += 1 - tostringSubjects += tpe + toStringRecursions += 1 + toStringSubjects += tpe tpe.safeToString } finally { - tostringSubjects -= tpe - tostringRecursions -= 1 + toStringSubjects -= tpe + toStringRecursions -= 1 } } diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index ac6dd0783d..9224749864 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -41,8 +41,8 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => // inaccessible: this._lubResults // inaccessible: this._glbResults // inaccessible: this._indent - // inaccessible: this._tostringRecursions - // inaccessible: this._tostringSubjects + // inaccessible: this._toStringRecursions + // inaccessible: this._toStringSubjects // inaccessible: this.atomicIds // inaccessible: this.atomicExistentialIds // inaccessible: this._recursionTable diff --git a/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala b/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala index 12ada07a56..83d471f91e 100644 --- a/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala +++ b/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala @@ -81,12 +81,12 @@ private[reflect] trait SynchronizedTypes extends internal.Types { self: SymbolTa override def indent = _indent.get override def indent_=(value: String) = _indent.set(value) - private lazy val _tostringRecursions = mkThreadLocalStorage(0) - override def tostringRecursions = _tostringRecursions.get - override def tostringRecursions_=(value: Int) = _tostringRecursions.set(value) + private lazy val _toStringRecursions = mkThreadLocalStorage(0) + override def toStringRecursions = _toStringRecursions.get + override def toStringRecursions_=(value: Int) = _toStringRecursions.set(value) - private lazy val _tostringSubjects = mkThreadLocalStorage(new mutable.HashSet[Type]) - override def tostringSubjects = _tostringSubjects.get + private lazy val _toStringSubjects = mkThreadLocalStorage(new mutable.HashSet[Type]) + override def toStringSubjects = _toStringSubjects.get /* The idea of caches is as follows. * When in reflexive mode, a cache is either null, or one sentinal -- cgit v1.2.3 From 554fc3e71481159237dee4c343a9fb24b1c82e68 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Mon, 20 Jan 2014 16:02:53 +0300 Subject: addresses pull request feedback --- src/reflect/scala/reflect/internal/Symbols.scala | 2 +- test/files/neg/t8158.check | 2 +- test/files/run/t5256h.check | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 54e5b2781e..f49ddaf6ca 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -2536,7 +2536,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => case PolyType(tparams, res) => typeParamsString(tp) + infoString(res) case NullaryMethodType(res) => infoString(res) case MethodType(params, res) => valueParamsString(tp) + infoString(res) - case _ if isStructuralThisType => ": this.type" + case _ if isStructuralThisType => ": " + owner.name case _ => ": " + tp } } diff --git a/test/files/neg/t8158.check b/test/files/neg/t8158.check index aaa62ddea4..fa6b744ba5 100644 --- a/test/files/neg/t8158.check +++ b/test/files/neg/t8158.check @@ -1,4 +1,4 @@ -Test_2.scala:10: error: not enough patterns for <$anon: AnyRef> offering AnyRef{def isEmpty: Boolean; def get: this.type; def unapply(x: String): this.type}: expected 1, found 0 +Test_2.scala:10: error: not enough patterns for <$anon: AnyRef> offering AnyRef{def isEmpty: Boolean; def get: $anon; def unapply(x: String): $anon}: expected 1, found 0 case X() => ^ one error found diff --git a/test/files/run/t5256h.check b/test/files/run/t5256h.check index 300a40f653..dc3e919897 100644 --- a/test/files/run/t5256h.check +++ b/test/files/run/t5256h.check @@ -3,5 +3,5 @@ Test.$anon$1 java.lang.Object { final private val x: Int def x(): Int - def (): this.type + def (): $anon$1 } -- cgit v1.2.3 From 3f3014cf0edd8ea182d234d5987838223231d9c9 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Mon, 20 Jan 2014 16:57:39 +0300 Subject: temporarily disables the toStringSubjects cache --- .../scala/reflect/internal/tpe/TypeToStrings.scala | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala b/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala index ceb667365e..a062fc8209 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeToStrings.scala @@ -20,11 +20,12 @@ private[internal] trait TypeToStrings { def toStringSubjects = _toStringSubjects protected def typeToString(tpe: Type): String = - if (toStringSubjects contains tpe) { - // handles self-referential anonymous classes and who knows what else - "..." - } - else if (toStringRecursions >= maxToStringRecursions) { + // if (toStringSubjects contains tpe) { + // // handles self-referential anonymous classes and who knows what else + // "..." + // } + // else + if (toStringRecursions >= maxToStringRecursions) { devWarning("Exceeded recursion depth attempting to print " + util.shortClassOfInstance(tpe)) if (settings.debug) (new Throwable).printStackTrace @@ -34,10 +35,14 @@ private[internal] trait TypeToStrings { else try { toStringRecursions += 1 - toStringSubjects += tpe + // TODO: study performance impact of this cache + // to quote Jason: + // I'm a little uneasy with the performance impact of the fail-safe. We end up calling Type#toString + // when we generate error messages, including, importantly, errors issued during silent mode that are never issued. + // toStringSubjects += tpe tpe.safeToString } finally { - toStringSubjects -= tpe + // toStringSubjects -= tpe toStringRecursions -= 1 } } -- cgit v1.2.3