Apply review suggestions

Author: Florian3k

compiler/src/dotty/tools/backend/jvm/GenBCode.scala

@@ -17,7 +17,7 @@ class GenBCode extends Phase { self =>
 
   override def description: String = GenBCode.description
 
-  override def isRunnable(using Context) = super.isRunnable && !ctx.usesBestEffortTasty
+  override def isRunnable(using Context) = super.isRunnable && !ctx.usedBestEffortTasty
 
   private val superCallsMap = new MutableSymbolMap[Set[ClassSymbol]]
   def registerSuperCall(sym: Symbol, calls: ClassSymbol): Unit = {

compiler/src/dotty/tools/backend/sjs/GenSJSIR.scala

@@ -12,7 +12,7 @@ class GenSJSIR extends Phase {
   override def description: String = GenSJSIR.description
 
   override def isRunnable(using Context): Boolean =
-    super.isRunnable && ctx.settings.scalajs.value && !ctx.usesBestEffortTasty
+    super.isRunnable && ctx.settings.scalajs.value && !ctx.usedBestEffortTasty
 
   def run(using Context): Unit =
     new JSCodeGen().run()

compiler/src/dotty/tools/dotc/Driver.scala

@@ -39,7 +39,7 @@ class Driver {
       catch
         case ex: FatalError =>
           report.error(ex.getMessage.nn) // signals that we should fail compilation.
-        case ex: Throwable if ctx.usesBestEffortTasty =>
+        case ex: Throwable if ctx.usedBestEffortTasty =>
           report.bestEffortError(ex, "Some best-effort tasty files were not able to be read.")
         case ex: TypeError if !runOrNull.enrichedErrorMessage =>
           println(runOrNull.enrichErrorMessage(s"${ex.toMessage} while compiling ${files.map(_.path).mkString(", ")}"))

compiler/src/dotty/tools/dotc/Run.scala

@@ -301,10 +301,6 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
       ctx.settings.Yskip.value, ctx.settings.YstopBefore.value, stopAfter, ctx.settings.Ycheck.value)
     ctx.base.usePhases(phases, runCtx)
 
-    var forceReachPhaseMaybe =
-      if (ctx.isBestEffort && phases.exists(_.phaseName == "typer")) Some("typer")
-      else None
-
     if ctx.settings.YnoDoubleBindings.value then
       ctx.base.checkNoDoubleBindings = true
 
@@ -313,6 +309,10 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
       val profiler = ctx.profiler
       var phasesWereAdjusted = false
 
+      var forceReachPhaseMaybe =
+        if (ctx.isBestEffort && phases.exists(_.phaseName == "typer")) Some("typer")
+        else None
+
       for phase <- allPhases do
         doEnterPhase(phase)
         val phaseWillRun = phase.isRunnable || forceReachPhaseMaybe.nonEmpty
@@ -329,11 +329,8 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
                   ctx.fresh.setPhase(phase.next).setCompilationUnit(unit))
                 lastPrintedTree = printTree(lastPrintedTree)(using printCtx(unit))
 
-            forceReachPhaseMaybe match {
-              case Some(forceReachPhase) if phase.phaseName == forceReachPhase =>
-                forceReachPhaseMaybe = None
-              case _ =>
-            }
+            if forceReachPhaseMaybe.contains(phase.phaseName) then
+              forceReachPhaseMaybe = None
 
             report.informTime(s"$phase ", start)
             Stats.record(s"total trees at end of $phase", ast.Trees.ntrees)

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -47,7 +47,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
     case _: RefTree | _: GenericApply | _: Inlined | _: Hole =>
       ta.assignType(untpd.Apply(fn, args), fn, args)
     case _ =>
-      assert(ctx.isBestEffort || ctx.usesBestEffortTasty || ctx.reporter.errorsReported)
+      assert(ctx.reporter.errorsReported || ctx.tolerateErrorsForBestEffort)
       ta.assignType(untpd.Apply(fn, args), fn, args)
 
   def TypeApply(fn: Tree, args: List[Tree])(using Context): TypeApply = fn match
@@ -56,7 +56,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
     case _: RefTree | _: GenericApply =>
       ta.assignType(untpd.TypeApply(fn, args), fn, args)
     case _ =>
-      assert(ctx.isBestEffort || ctx.usesBestEffortTasty || ctx.reporter.errorsReported, s"unexpected tree for type application: $fn")
+      assert(ctx.reporter.errorsReported || ctx.tolerateErrorsForBestEffort, s"unexpected tree for type application: $fn")
       ta.assignType(untpd.TypeApply(fn, args), fn, args)
 
   def Literal(const: Constant)(using Context): Literal =

compiler/src/dotty/tools/dotc/core/Contexts.scala

@@ -475,17 +475,20 @@ object Contexts {
     /** Is the flexible types option set? */
     def flexibleTypes: Boolean = base.settings.YexplicitNulls.value && !base.settings.YnoFlexibleTypes.value
 
-    /** Is best-effort-dir option set? */
+    /** Is the best-effort option set? */
     def isBestEffort: Boolean = base.settings.YbestEffort.value
 
-    /** Is the from-best-effort-tasty option set to true? */
+    /** Is the with-best-effort-tasty option set? */
     def withBestEffortTasty: Boolean = base.settings.YwithBestEffortTasty.value
 
     /** Were any best effort tasty dependencies used during compilation? */
-    def usesBestEffortTasty: Boolean = base.usedBestEffortTasty
+    def usedBestEffortTasty: Boolean = base.usedBestEffortTasty
 
     /** Confirm that a best effort tasty dependency was used during compilation. */
-    def setUsesBestEffortTasty(): Unit = base.usedBestEffortTasty = true
+    def setUsedBestEffortTasty(): Unit = base.usedBestEffortTasty = true
+
+    /** Is either the best-effort option set or .betasty files were used during compilation? */
+    def tolerateErrorsForBestEffort = isBestEffort || usedBestEffortTasty
 
     /** A fresh clone of this context embedded in this context. */
     def fresh: FreshContext = freshOver(this)

compiler/src/dotty/tools/dotc/core/DenotTransformers.scala

@@ -29,7 +29,7 @@ object DenotTransformers {
     /** The transformation method */
     def transform(ref: SingleDenotation)(using Context): SingleDenotation
 
-    override def isRunnable(using Context) = super.isRunnable && !ctx.usesBestEffortTasty 
+    override def isRunnable(using Context) = super.isRunnable && !ctx.usedBestEffortTasty
   }
 
   /** A transformer that only transforms the info field of denotations */

compiler/src/dotty/tools/dotc/core/Denotations.scala

@@ -720,7 +720,7 @@ object Denotations {
         || ctx.settings.YtestPickler.value // mixing test pickler with debug printing can travel back in time
         || ctx.mode.is(Mode.Printing)  // no use to be picky when printing error messages
         || symbol.isOneOf(ValidForeverFlags)
-        || ctx.isBestEffort || ctx.usesBestEffortTasty,
+        || ctx.tolerateErrorsForBestEffort,
         s"denotation $this invalid in run ${ctx.runId}. ValidFor: $validFor")
       var d: SingleDenotation = this
       while ({

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -720,7 +720,7 @@ object SymDenotations {
      *  TODO: Find a more robust way to characterize self symbols, maybe by
      *       spending a Flag on them?
      */
-    final def isSelfSym(using Context): Boolean = 
+    final def isSelfSym(using Context): Boolean =
       if !ctx.isBestEffort || exists then
         owner.infoOrCompleter match {
           case ClassInfo(_, _, _, _, selfInfo) =>
@@ -2007,7 +2007,7 @@ object SymDenotations {
         case p :: parents1 =>
           p.classSymbol match {
             case pcls: ClassSymbol => builder.addAll(pcls.baseClasses)
-            case _ => assert(isRefinementClass || p.isError || ctx.mode.is(Mode.Interactive) || ctx.isBestEffort || ctx.usesBestEffortTasty, s"$this has non-class parent: $p")
+            case _ => assert(isRefinementClass || p.isError || ctx.mode.is(Mode.Interactive) || ctx.tolerateErrorsForBestEffort, s"$this has non-class parent: $p")
           }
           traverse(parents1)
         case nil =>

compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala

@@ -439,16 +439,17 @@ class TastyLoader(val tastyFile: AbstractFile) extends SymbolLoader {
         val tastyBytes = tastyFile.toByteArray
         val unpickler = new tasty.DottyUnpickler(tastyFile, tastyBytes, isBestEffortTasty = isBestEffortTasty)
         unpickler.enter(roots = Set(classRoot, moduleRoot, moduleRoot.sourceModule))(using ctx.withSource(util.NoSource))
-        if mayLoadTreesFromTasty || (isBestEffortTasty && ctx.withBestEffortTasty) then
+        if mayLoadTreesFromTasty || isBestEffortTasty then
           classRoot.classSymbol.rootTreeOrProvider = unpickler
           moduleRoot.classSymbol.rootTreeOrProvider = unpickler
         if isBestEffortTasty then
           checkBeTastyUUID(tastyFile, tastyBytes)
-          ctx.setUsesBestEffortTasty()
+          ctx.setUsedBestEffortTasty()
         else
           checkTastyUUID()
       else
         report.error(em"Best Effort TASTy $tastyFile file could not be read.")
+
   private def handleUnpicklingExceptions[T](thunk: =>T): T =
     try thunk
     catch case e: RuntimeException =>

compiler/src/dotty/tools/dotc/core/TypeErasure.scala

@@ -747,7 +747,9 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst
     assert(!etp.isInstanceOf[WildcardType] || inSigName, i"Unexpected WildcardType erasure for $tp")
     etp
 
-  /** Like translucentSuperType, but issue a fatal error if it does not exist. */
+  /** Like translucentSuperType, but issue a fatal error if it does not exist.
+   *  If using the best-effort option, the fatal error will not be issued.
+  */
   private def checkedSuperType(tp: TypeProxy)(using Context): Type =
     val tp1 = tp.translucentSuperType
     if !tp1.exists then

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -3150,7 +3150,7 @@ object Types extends TypeUtils {
       else cls.info match {
         case cinfo: ClassInfo => cinfo.selfType
         case _: ErrorType | NoType
-          if ctx.mode.is(Mode.Interactive) || ctx.isBestEffort || ctx.usesBestEffortTasty => cls.info
+          if ctx.mode.is(Mode.Interactive) || ctx.tolerateErrorsForBestEffort => cls.info
           // can happen in IDE if `cls` is stale
       }
 
@@ -3720,8 +3720,9 @@ object Types extends TypeUtils {
 
     def apply(tp1: Type, tp2: Type, soft: Boolean)(using Context): OrType = {
       def where = i"in union $tp1 | $tp2"
-      if (!ctx.usesBestEffortTasty) expectValueTypeOrWildcard(tp1, where)
-      if (!ctx.usesBestEffortTasty) expectValueTypeOrWildcard(tp2, where)
+      if !ctx.usedBestEffortTasty then
+        expectValueTypeOrWildcard(tp1, where)
+        expectValueTypeOrWildcard(tp2, where)
       assertUnerased()
       unique(new CachedOrType(tp1, tp2, soft))
     }

compiler/src/dotty/tools/dotc/core/tasty/DottyUnpickler.scala

@@ -27,7 +27,7 @@ object DottyUnpickler {
     compilationUnitInfo: CompilationUnitInfo,
     posUnpickler: Option[PositionUnpickler],
     commentUnpickler: Option[CommentUnpickler],
-    isBestEffortTasty: Boolean = false
+    isBestEffortTasty: Boolean
   ) extends SectionUnpickler[TreeUnpickler](ASTsSection) {
     def unpickle(reader: TastyReader, nameAtRef: NameTable): TreeUnpickler =
       new TreeUnpickler(reader, nameAtRef, compilationUnitInfo, posUnpickler, commentUnpickler, isBestEffortTasty)
@@ -52,14 +52,14 @@ object DottyUnpickler {
 /** A class for unpickling Tasty trees and symbols.
  *  @param tastyFile         tasty file from which we unpickle (used for CompilationUnitInfo)
  *  @param bytes             the bytearray containing the Tasty file from which we unpickle
+ *  @param isBestEffortTasty specifies whether file should be unpickled as a Best Effort TASTy
  *  @param mode              the tasty file contains package (TopLevel), an expression (Term) or a type (TypeTree)
- *  @param isBestEffortTasty specifies wheather file should be unpickled as a Best Effort TASTy
  */
 class DottyUnpickler(
   tastyFile: AbstractFile,
   bytes: Array[Byte],
-  mode: UnpickleMode = UnpickleMode.TopLevel,
-  isBestEffortTasty: Boolean = false
+  isBestEffortTasty: Boolean,
+  mode: UnpickleMode = UnpickleMode.TopLevel
 ) extends ClassfileParser.Embedded with tpd.TreeProvider {
   import tpd.*
   import DottyUnpickler.*

compiler/src/dotty/tools/dotc/core/tasty/TastyHTMLPrinter.scala

@@ -2,7 +2,7 @@ package dotty.tools.dotc
 package core
 package tasty
 
-class TastyHTMLPrinter(bytes: Array[Byte]) extends TastyPrinter(bytes) {
+class TastyHTMLPrinter(bytes: Array[Byte]) extends TastyPrinter(bytes, isBestEffortTasty = false) {
   override protected def nameStr(str: String): String = s"<span class='name'>$str</span>"
   override protected def treeStr(str: String): String = s"<span class='tree'>$str</span>"
   override protected def lengthStr(str: String): String = s"<span class='length'>$str</span>"

compiler/src/dotty/tools/dotc/core/tasty/TastyPickler.scala

@@ -17,7 +17,7 @@ import Decorators.*
 object TastyPickler:
   private val versionString = s"Scala ${config.Properties.simpleVersionString}"
 
-class TastyPickler(val rootCls: ClassSymbol) {
+class TastyPickler(val rootCls: ClassSymbol, isBestEffortTasty: Boolean) {
 
   private val sections = new mutable.ArrayBuffer[(NameRef, TastyBuffer)]
 
@@ -26,7 +26,7 @@ class TastyPickler(val rootCls: ClassSymbol) {
   def newSection(name: String, buf: TastyBuffer): Unit =
     sections += ((nameBuffer.nameIndex(name.toTermName), buf))
 
-  def assembleParts(isBestEffortTasty: Boolean = false): Array[Byte] = {
+  def assembleParts(): Array[Byte] = {
     def lengthWithLength(buf: TastyBuffer) =
       buf.length + natSize(buf.length)
 

compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala

@@ -39,7 +39,7 @@ object TastyPrinter:
     val noColor = args.contains("-color:never")
     val allowBetasty = args.contains(betastyOpt)
     var printLastLine = false
-    def printTasty(fileName: String, bytes: Array[Byte], isBestEffortTasty: Boolean = false): Unit =
+    def printTasty(fileName: String, bytes: Array[Byte], isBestEffortTasty: Boolean): Unit =
       println(line)
       println(fileName)
       println(line)

compiler/src/dotty/tools/dotc/core/tasty/TastyUnpickler.scala

@@ -65,7 +65,7 @@ object TastyUnpickler {
 
 import TastyUnpickler.*
 
-class TastyUnpickler(protected val reader: TastyReader, isBestEffortTasty: Boolean = false) {
+class TastyUnpickler(protected val reader: TastyReader, isBestEffortTasty: Boolean) {
   import reader.*
 
   def this(bytes: Array[Byte]) = this(new TastyReader(bytes), false)

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

@@ -6,6 +6,7 @@ package tasty
 import scala.language.unsafeNulls
 
 import dotty.tools.tasty.TastyFormat.*
+import dotty.tools.tasty.besteffort.BestEffortTastyFormat.ERRORtype
 import dotty.tools.tasty.TastyBuffer.*
 
 import ast.Trees.*
@@ -65,12 +66,15 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
     fillRef(lengthAddr, currentAddr, relative = true)
   }
 
-  /* There are certain expectations with code naturally being able to reach pickling
-   * phase as opposed to one that uses best-effort compilation features. For betasty
-   * files, we try to avoid calling any assertions which can be unfullfilled.
+  /** There are certain expectations with code which is naturally able to reach pickling
+   *  phase as opposed to one that uses best-effort compilation features.
+   *  When pickling betasty files, we do some custom checks, in case those expectations
+   *  cannot be fulfilled, and if then we can try to do something else.
+   *  For regular non best-effort compilation (without best-effort and without using .betasty on classpath),
+   *  this will always return true.
    */
-  private inline def assertForBestEffort(assertion: Boolean)(using Context): Boolean =
-    ((!ctx.isBestEffort && ctx.reporter.errorsReported) || ctx.usesBestEffortTasty) || assertion
+  private inline def passesConditionForErroringBestEffortCode(condition: Boolean)(using Context): Boolean =
+    ((!ctx.isBestEffort && ctx.reporter.errorsReported) || ctx.usedBestEffortTasty) || condition
 
   def addrOfSym(sym: Symbol): Option[Addr] =
     symRefs.get(sym)
@@ -307,8 +311,6 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
       else if !pickled then pickleErrorType()
     case tpe: LazyRef =>
       pickleType(tpe.ref)
-    case tpe: ErrorType if ctx.isBestEffort =>
-      pickleErrorType()
     case _ if ctx.isBestEffort =>
       pickleErrorType()
   }
@@ -339,7 +341,7 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
   }
 
   def pickleTpt(tpt: Tree)(using Context): Unit =
-    if assertForBestEffort(tpt.isType) then pickleTree(tpt)
+    if passesConditionForErroringBestEffortCode(tpt.isType) then pickleTree(tpt)
     else pickleErrorType()
 
   def pickleTreeUnlessEmpty(tree: Tree)(using Context): Unit = {
@@ -354,7 +356,11 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
   def pickleDef(tag: Int, mdef: MemberDef, tpt: Tree, rhs: Tree = EmptyTree, pickleParams: => Unit = ())(using Context): Unit = {
     val sym = mdef.symbol
 
-    if assertForBestEffort(symRefs.get(sym) == Some(NoAddr) && !(tag == TYPEDEF && tpt.isInstanceOf[Template] && !tpt.symbol.exists)) then
+    def isDefSymPreRegisteredAndTreeHasCorrectStructure() =
+      symRefs.get(sym) == Some(NoAddr) && // check if symbol id preregistered (with the preRegister method)
+      !(tag == TYPEDEF && tpt.isInstanceOf[Template] && !tpt.symbol.exists) // in case this is a TEMPLATE, check if we are able to pickle it
+
+    if passesConditionForErroringBestEffortCode(isDefSymPreRegisteredAndTreeHasCorrectStructure()) then
       assert(symRefs(sym) == NoAddr, sym)
       registerDef(sym)
       writeByte(tag)
@@ -418,7 +424,7 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
     else
       try tree match {
         case Ident(name) =>
-          if assertForBestEffort(tree.hasType) then
+          if passesConditionForErroringBestEffortCode(tree.hasType) then
             tree.tpe match {
               case tp: TermRef if name != nme.WILDCARD =>
                 // wildcards are pattern bound, need to be preserved as ids.
@@ -458,7 +464,7 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
                 pickleType(tp)
               }
             case _ =>
-              if assertForBestEffort(tree.hasType) then
+              if passesConditionForErroringBestEffortCode(tree.hasType) then
                 val sig = tree.tpe.signature
                 var ename = tree.symbol.targetName
                 val selectFromQualifier =
@@ -478,7 +484,11 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
                     pickleTree(qual)
                     pickleType(tree.symbol.owner.typeRef)
                   }
-              else pickleErrorType()
+              else
+                writeByte(if name.isTypeName then SELECTtpt else SELECT)
+                val ename = tree.symbol.targetName
+                pickleNameAndSig(name, Signature.NotAMethod, ename)
+                pickleTree(qual)
           }
         case Apply(fun, args) =>
           if (fun.symbol eq defn.throwMethod) {
@@ -506,13 +516,14 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
             args.foreach(pickleTpt)
           }
         case Literal(const1) =>
-          if assertForBestEffort(tree.hasType) then
+          if passesConditionForErroringBestEffortCode(tree.hasType) then
             pickleConstant {
               tree.tpe match {
                 case ConstantType(const2) => const2
                 case _ => const1
               }
             }
+          else pickleConstant(const1)
         case Super(qual, mix) =>
           writeByte(SUPER)
           withLength {
@@ -684,15 +695,15 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
           writeByte(PACKAGE)
           withLength { pickleType(pid.tpe); pickleStats(stats) }
         case tree: TypeTree =>
-          if assertForBestEffort(tree.hasType) then pickleType(tree.tpe)
+          if passesConditionForErroringBestEffortCode(tree.hasType) then pickleType(tree.tpe)
           else pickleErrorType()
         case SingletonTypeTree(ref) =>
           writeByte(SINGLETONtpt)
           pickleTree(ref)
         case RefinedTypeTree(parent, refinements) =>
           if (refinements.isEmpty) pickleTree(parent)
           else {
-            if assertForBestEffort(refinements.head.symbol.exists) then
+            if passesConditionForErroringBestEffortCode(refinements.head.symbol.exists) then
               val refineCls = refinements.head.symbol.owner.asClass
               registerDef(refineCls)
               pickledTypes(refineCls.typeRef) = currentAddr