Skip to content

Commit

Permalink
REPL: Fix crash when printing instances of value classes (#16393)
Browse files Browse the repository at this point in the history
By fixing `Rendering`'s `rewrapValueClass` to use the fully qualified
class name

Sequel to #15545

Fixes #16322
Fixes #16387
  • Loading branch information
smarter authored Dec 9, 2022
2 parents 4a3747a + 2deb5f5 commit 231f9ab
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 58 deletions.
24 changes: 24 additions & 0 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,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 =
Expand Down
7 changes: 1 addition & 6 deletions compiler/src/dotty/tools/dotc/quoted/Interpreter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,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]
}

Expand Down
29 changes: 1 addition & 28 deletions compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 23 additions & 24 deletions compiler/src/dotty/tools/repl/Rendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,14 @@ package repl

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.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

/** 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
Expand Down Expand Up @@ -131,8 +127,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 valueClassName = sym.flatName.encode.toString
val valueClass = Class.forName(valueClassName, true, classLoader())
val valueClass = Class.forName(sym.binaryClassName, true, classLoader())
valueClass.getConstructors.headOption.map(_.newInstance(value))
else
Some(value)
Expand All @@ -148,35 +143,31 @@ 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
Right(
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. */
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())
Nil
try load()
catch
case e: ExceptionInInitializerError => List(renderError(e, sym.denot))
case NonFatal(e) => List(renderError(InvocationTargetException(e), 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(thr: Throwable, d: Denotation)(using Context): Diagnostic =
val cause = rootCause(thr)
// detect
//at repl$.rs$line$2$.<clinit>(rs$line$2:1)
//at repl$.rs$line$2.res1(rs$line$2)
Expand All @@ -190,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

Expand All @@ -200,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
5 changes: 5 additions & 0 deletions compiler/test-resources/repl/i15493
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 231f9ab

Please sign in to comment.