From 22569257cecc0ad83b1235eb6d2ac281d0ee3082 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 22 Nov 2022 18:21:05 +0000 Subject: [PATCH 1/5] REPL: Fix Rendering's rewrapValueClass --- compiler/src/dotty/tools/repl/Rendering.scala | 17 ++++++++++++++--- compiler/test-resources/repl/i15493 | 5 +++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/repl/Rendering.scala b/compiler/src/dotty/tools/repl/Rendering.scala index 5cba672ce7b0..258d870aa3fc 100644 --- a/compiler/src/dotty/tools/repl/Rendering.scala +++ b/compiler/src/dotty/tools/repl/Rendering.scala @@ -10,12 +10,15 @@ import dotc.core.Contexts._ import dotc.core.Denotations.Denotation import dotc.core.Flags import dotc.core.Flags._ +import dotc.core.NameOps.* import dotc.core.Symbols.{Symbol, defn} import dotc.core.StdNames.{nme, str} import dotc.printing.ReplPrinter import dotc.reporting.Diagnostic import dotc.transform.ValueClasses +import scala.util.control.NonFatal + /** This rendering object uses `ClassLoader`s to accomplish crossing the 4th * wall (i.e. fetching back values from the compiled class files put into a * specific class loader capable of loading from memory) and rendering them. @@ -106,7 +109,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): * * Calling this method evaluates the expression using reflection */ - private def valueOf(sym: Symbol)(using Context): Option[String] = + private def valueOf(sym: Symbol)(using Context): Option[String] = try val objectName = sym.owner.fullName.encode.toString.stripSuffix("$") val resObj: Class[?] = Class.forName(objectName, true, classLoader()) val symValue = resObj @@ -123,6 +126,9 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): else s } + catch + case e: InvocationTargetException => throw e + case e: ReflectiveOperationException => throw InvocationTargetException(e) /** Rewrap value class to their Wrapper class * @@ -131,7 +137,13 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): */ private def rewrapValueClass(sym: Symbol, value: Object)(using Context): Option[Object] = if ValueClasses.isDerivedValueClass(sym) then - val valueClassName = sym.flatName.encode.toString + val pkg = sym.enclosingPackageClass + val pkgName = if pkg.isEmptyPackage then "" else s"${pkg.fullName.mangledString}." + val clsFlatName = if sym.isOneOf(JavaDefined | ConstructorProxy) then + // See ExtractDependencies.recordDependency + sym.flatName.stripModuleClassSuffix + else sym.flatName + val valueClassName = pkgName + clsFlatName.mangledString val valueClass = Class.forName(valueClassName, true, classLoader()) valueClass.getConstructors.headOption.map(_.newInstance(value)) else @@ -161,7 +173,6 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): /** Force module initialization in the absence of members. */ def forceModule(sym: Symbol)(using Context): Seq[Diagnostic] = - import scala.util.control.NonFatal def load() = val objectName = sym.fullName.encode.toString Class.forName(objectName, true, classLoader()) diff --git a/compiler/test-resources/repl/i15493 b/compiler/test-resources/repl/i15493 index f543f5c1d0f7..670cf8ebcbd2 100644 --- a/compiler/test-resources/repl/i15493 +++ b/compiler/test-resources/repl/i15493 @@ -142,3 +142,8 @@ val res33: Outer.Foo = Outer$Foo@17 scala> res33.toString val res34: String = Outer$Foo@17 +scala> Vector.unapplySeq(Vector(2)) +val res35: scala.collection.SeqFactory.UnapplySeqWrapper[Int] = scala.collection.SeqFactory$UnapplySeqWrapper@df507bfd + +scala> new scala.concurrent.duration.DurationInt(5) +val res36: scala.concurrent.duration.package.DurationInt = scala.concurrent.duration.package$DurationInt@5 From 41a584aeeaa58e0ad7e8d90096b2421f4417c3ed Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 1 Dec 2022 10:13:28 +0000 Subject: [PATCH 2/5] REPL: Switch renderVal to handle all ReflectiveOperationExceptions --- compiler/src/dotty/tools/repl/Rendering.scala | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/compiler/src/dotty/tools/repl/Rendering.scala b/compiler/src/dotty/tools/repl/Rendering.scala index 258d870aa3fc..932939adb081 100644 --- a/compiler/src/dotty/tools/repl/Rendering.scala +++ b/compiler/src/dotty/tools/repl/Rendering.scala @@ -6,16 +6,12 @@ import scala.language.unsafeNulls import java.lang.{ ClassLoader, ExceptionInInitializerError } import java.lang.reflect.InvocationTargetException -import dotc.core.Contexts._ -import dotc.core.Denotations.Denotation -import dotc.core.Flags -import dotc.core.Flags._ -import dotc.core.NameOps.* -import dotc.core.Symbols.{Symbol, defn} -import dotc.core.StdNames.{nme, str} -import dotc.printing.ReplPrinter -import dotc.reporting.Diagnostic -import dotc.transform.ValueClasses +import dotc.*, core.* +import Contexts.*, Denotations.*, Flags.*, NameOps.*, StdNames.*, Symbols.* +import printing.ReplPrinter +import reporting.Diagnostic +import transform.ValueClasses +import util.StackTraceOps.* import scala.util.control.NonFatal @@ -109,7 +105,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): * * Calling this method evaluates the expression using reflection */ - private def valueOf(sym: Symbol)(using Context): Option[String] = try + private def valueOf(sym: Symbol)(using Context): Option[String] = val objectName = sym.owner.fullName.encode.toString.stripSuffix("$") val resObj: Class[?] = Class.forName(objectName, true, classLoader()) val symValue = resObj @@ -126,9 +122,6 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): else s } - catch - case e: InvocationTargetException => throw e - case e: ReflectiveOperationException => throw InvocationTargetException(e) /** Rewrap value class to their Wrapper class * @@ -160,7 +153,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): infoDiagnostic(d.symbol.showUser, d) /** Render value definition result */ - def renderVal(d: Denotation)(using Context): Either[InvocationTargetException, Option[Diagnostic]] = + def renderVal(d: Denotation)(using Context): Either[ReflectiveOperationException, Option[Diagnostic]] = val dcl = d.symbol.showUser def msg(s: String) = infoDiagnostic(s, d) try @@ -168,7 +161,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): if d.symbol.is(Flags.Lazy) then Some(msg(dcl)) else valueOf(d.symbol).map(value => msg(s"$dcl = $value")) ) - catch case e: InvocationTargetException => Left(e) + catch case e: ReflectiveOperationException => Left(e) end renderVal /** Force module initialization in the absence of members. */ @@ -179,15 +172,11 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): Nil try load() catch - case e: ExceptionInInitializerError => List(renderError(e, sym.denot)) - case NonFatal(e) => List(renderError(InvocationTargetException(e), sym.denot)) + case e: ExceptionInInitializerError => List(renderError(e.getCause, sym.denot)) + case NonFatal(e) => List(renderError(e, sym.denot)) /** Render the stack trace of the underlying exception. */ - def renderError(ite: InvocationTargetException | ExceptionInInitializerError, d: Denotation)(using Context): Diagnostic = - import dotty.tools.dotc.util.StackTraceOps._ - val cause = ite.getCause match - case e: ExceptionInInitializerError => e.getCause - case e => e + def renderError(cause: Throwable, d: Denotation)(using Context): Diagnostic = // detect //at repl$.rs$line$2$.(rs$line$2:1) //at repl$.rs$line$2.res1(rs$line$2) From 6ef28dfd375a9794fbf968c56d5a7d5868ad673e Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 1 Dec 2022 11:53:53 +0000 Subject: [PATCH 3/5] Dedupe sym.binaryClassName --- .../tools/dotc/core/SymDenotations.scala | 24 +++++++++++++++ .../dotty/tools/dotc/quoted/Interpreter.scala | 7 +---- .../tools/dotc/sbt/ExtractDependencies.scala | 29 +------------------ compiler/src/dotty/tools/repl/Rendering.scala | 9 +----- 4 files changed, 27 insertions(+), 42 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 4cd0e400e2d1..bb93adeca58e 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -505,6 +505,30 @@ object SymDenotations { /** `fullName` where `.' is the separator character */ def fullName(using Context): Name = fullNameSeparated(QualifiedName) + /** The fully qualified name on the JVM of the class corresponding to this symbol. */ + def binaryClassName(using Context): String = + val builder = new StringBuilder + val pkg = enclosingPackageClass + if !pkg.isEffectiveRoot then + builder.append(pkg.fullName.mangledString) + builder.append(".") + val flatName = this.flatName + // Some companion objects are fake (that is, they're a compiler fiction + // that doesn't correspond to a class that exists at runtime), this + // can happen in two cases: + // - If a Java class has static members. + // - If we create constructor proxies for a class (see NamerOps#addConstructorProxies). + // + // In both cases it's may be vital that we don't return the object name. + // For instance, sending it to zinc: when sbt is restarted, zinc will inspect the binary + // dependencies to see if they're still on the classpath, if it + // doesn't find them it will invalidate whatever referenced them, so + // any reference to a fake companion will lead to extra recompilations. + // Instead, use the class name since it's guaranteed to exist at runtime. + val clsFlatName = if isOneOf(JavaDefined | ConstructorProxy) then flatName.stripModuleClassSuffix else flatName + builder.append(clsFlatName.mangledString) + builder.toString + private var myTargetName: Name | Null = null private def computeTargetName(targetNameAnnot: Option[Annotation])(using Context): Name = diff --git a/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala b/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala index 5a9490c3723e..bcfdbe72c62e 100644 --- a/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala +++ b/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala @@ -200,12 +200,7 @@ abstract class Interpreter(pos: SrcPos, classLoader: ClassLoader)(using Context) } else { // nested object in an object - val className = { - val pack = sym.topLevelClass.owner - if (pack == defn.RootPackage || pack == defn.EmptyPackageClass) sym.flatName.toString - else pack.showFullName + "." + sym.flatName - } - val clazz = loadClass(className) + val clazz = loadClass(sym.binaryClassName) clazz.getConstructor().newInstance().asInstanceOf[Object] } diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index f7b15dc21eb0..52e50b0e4a64 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -143,34 +143,7 @@ class ExtractDependencies extends Phase { def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance if (depFile.extension == "class") { // Dependency is external -- source is undefined - - // The fully qualified name on the JVM of the class corresponding to `dep.to` - val binaryClassName = { - val builder = new StringBuilder - val pkg = dep.to.enclosingPackageClass - if (!pkg.isEffectiveRoot) { - builder.append(pkg.fullName.mangledString) - builder.append(".") - } - val flatName = dep.to.flatName - // Some companion objects are fake (that is, they're a compiler fiction - // that doesn't correspond to a class that exists at runtime), this - // can happen in two cases: - // - If a Java class has static members. - // - If we create constructor proxies for a class (see NamerOps#addConstructorProxies). - // - // In both cases it's vital that we don't send the object name to - // zinc: when sbt is restarted, zinc will inspect the binary - // dependencies to see if they're still on the classpath, if it - // doesn't find them it will invalidate whatever referenced them, so - // any reference to a fake companion will lead to extra recompilations. - // Instead, use the class name since it's guaranteed to exist at runtime. - val clsFlatName = if (dep.to.isOneOf(JavaDefined | ConstructorProxy)) flatName.stripModuleClassSuffix else flatName - builder.append(clsFlatName.mangledString) - builder.toString - } - - processExternalDependency(depFile, binaryClassName) + processExternalDependency(depFile, dep.to.binaryClassName) } else if (allowLocal || depFile.file != sourceFile) { // We cannot ignore dependencies coming from the same source file because // the dependency info needs to propagate. See source-dependencies/trait-trait-211. diff --git a/compiler/src/dotty/tools/repl/Rendering.scala b/compiler/src/dotty/tools/repl/Rendering.scala index 932939adb081..1e1bbcda20c1 100644 --- a/compiler/src/dotty/tools/repl/Rendering.scala +++ b/compiler/src/dotty/tools/repl/Rendering.scala @@ -130,14 +130,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): */ private def rewrapValueClass(sym: Symbol, value: Object)(using Context): Option[Object] = if ValueClasses.isDerivedValueClass(sym) then - val pkg = sym.enclosingPackageClass - val pkgName = if pkg.isEmptyPackage then "" else s"${pkg.fullName.mangledString}." - val clsFlatName = if sym.isOneOf(JavaDefined | ConstructorProxy) then - // See ExtractDependencies.recordDependency - sym.flatName.stripModuleClassSuffix - else sym.flatName - val valueClassName = pkgName + clsFlatName.mangledString - val valueClass = Class.forName(valueClassName, true, classLoader()) + val valueClass = Class.forName(sym.binaryClassName, true, classLoader()) valueClass.getConstructors.headOption.map(_.newInstance(value)) else Some(value) From e2a529041ad9d904fec483dd9ddfb688cb7887d2 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 1 Dec 2022 12:57:59 +0000 Subject: [PATCH 4/5] Refix WorksheetTest.evaluationException --- compiler/src/dotty/tools/repl/Rendering.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/repl/Rendering.scala b/compiler/src/dotty/tools/repl/Rendering.scala index 1e1bbcda20c1..1b5c6021c1d6 100644 --- a/compiler/src/dotty/tools/repl/Rendering.scala +++ b/compiler/src/dotty/tools/repl/Rendering.scala @@ -165,11 +165,14 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): Nil try load() catch - case e: ExceptionInInitializerError => List(renderError(e.getCause, sym.denot)) + case e: ExceptionInInitializerError => List(renderError(e, sym.denot)) case NonFatal(e) => List(renderError(e, sym.denot)) /** Render the stack trace of the underlying exception. */ - def renderError(cause: Throwable, d: Denotation)(using Context): Diagnostic = + def renderError(thr: Throwable, d: Denotation)(using Context): Diagnostic = + val cause = thr.getCause match + case e: ExceptionInInitializerError => e.getCause + case e => e // detect //at repl$.rs$line$2$.(rs$line$2:1) //at repl$.rs$line$2.res1(rs$line$2) From 2deb5f51817a3ddb1f76284fedd5be61bea703d0 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 1 Dec 2022 23:12:10 +0000 Subject: [PATCH 5/5] REPL: Fix root cause unwrapping Fixing ReplCompilerTests i14701. --- compiler/src/dotty/tools/repl/Rendering.scala | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/repl/Rendering.scala b/compiler/src/dotty/tools/repl/Rendering.scala index 1b5c6021c1d6..c647ef302bb9 100644 --- a/compiler/src/dotty/tools/repl/Rendering.scala +++ b/compiler/src/dotty/tools/repl/Rendering.scala @@ -3,9 +3,6 @@ package repl import scala.language.unsafeNulls -import java.lang.{ ClassLoader, ExceptionInInitializerError } -import java.lang.reflect.InvocationTargetException - import dotc.*, core.* import Contexts.*, Denotations.*, Flags.*, NameOps.*, StdNames.*, Symbols.* import printing.ReplPrinter @@ -170,9 +167,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): /** Render the stack trace of the underlying exception. */ def renderError(thr: Throwable, d: Denotation)(using Context): Diagnostic = - val cause = thr.getCause match - case e: ExceptionInInitializerError => e.getCause - case e => e + val cause = rootCause(thr) // detect //at repl$.rs$line$2$.(rs$line$2:1) //at repl$.rs$line$2.res1(rs$line$2) @@ -186,7 +181,6 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None): private def infoDiagnostic(msg: String, d: Denotation)(using Context): Diagnostic = new Diagnostic.Info(msg, d.symbol.sourcePos) - object Rendering: final val REPL_WRAPPER_NAME_PREFIX = str.REPL_SESSION_LINE @@ -196,3 +190,12 @@ object Rendering: val text = printer.dclText(s) text.mkString(ctx.settings.pageWidth.value, ctx.settings.printLines.value) } + + def rootCause(x: Throwable): Throwable = x match + case _: ExceptionInInitializerError | + _: java.lang.reflect.InvocationTargetException | + _: java.lang.reflect.UndeclaredThrowableException | + _: java.util.concurrent.ExecutionException + if x.getCause != null => + rootCause(x.getCause) + case _ => x