Fix according to review comments

Author: noti0na1

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

@@ -181,7 +181,7 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Src
             case _: XMLBlock =>
               // FIXME: Trees generated by the XML parser do not satisfy `checkPos`
             case _: WildcardFunction
-            if lastPositioned != null && lastPositioned.isInstanceOf[ValDef] && !p.isInstanceOf[ValDef] =>
+            if lastPositioned.isInstanceOf[ValDef] && !p.isInstanceOf[ValDef] =>
               // ignore transition from last wildcard parameter to body
             case _ =>
               assert(!lastSpan.exists || !p.span.exists || lastSpan.end <= p.span.start,

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

@@ -182,13 +182,13 @@ object Names {
     private var derivedNames: LinearMap[NameInfo, DerivedName] = LinearMap.empty
 
     private def add(info: NameInfo): TermName = synchronized {
-      derivedNames.lookup(info) match
+      val dnOpt = derivedNames.lookup(info)
+      dnOpt match
         case null =>
           val derivedName = new DerivedName(this, info)
           derivedNames = derivedNames.updated(info, derivedName)
           derivedName
-        case derivedName: DerivedName =>
-          derivedName
+        case _ => dnOpt
     }
 
     private def rewrap(underlying: TermName) =

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

@@ -51,6 +51,7 @@ object OrderingConstraint {
     def update(prev: OrderingConstraint, current: OrderingConstraint,
         poly: TypeLambda, idx: Int, entry: T)(using Context): OrderingConstraint = {
       var es = entries(current, poly)
+      // TODO: invest why flow typing is not working on `es`
       if (es != null && (es.nn(idx) eq entry)) current
       else {
         val result =
@@ -567,12 +568,12 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       boundsMap.foreachBinding { (poly, entries) =>
         for (i <- 0 until paramCount(entries))
           typeVar(entries, i) match {
-            case tv: TypeVar if !tv.inst.exists && isBounds(entries(i)) => myUninstVars.nn += tv
+            case tv: TypeVar if !tv.inst.exists && isBounds(entries(i)) => myUninstVars.uncheckedNN += tv
             case _ =>
           }
       }
     }
-    myUninstVars.nn
+    myUninstVars.uncheckedNN
   }
 
 // ---------- Checking -----------------------------------------------

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

@@ -134,7 +134,7 @@ object Scopes {
     final def lookupAll(name: Name)(using Context): Iterator[Symbol] = new Iterator[Symbol] {
       var e = lookupEntry(name)
       def hasNext: Boolean = e != null
-      def next(): Symbol = { val r = e.uncheckedNN.sym; e = lookupNextEntry(e.uncheckedNN); r }
+      def next(): Symbol = { val r = e.nn.sym; e = lookupNextEntry(e.uncheckedNN); r }
     }
 
     /** Does this scope contain a reference to `sym` when looking up `name`? */
@@ -168,7 +168,8 @@ object Scopes {
           if (result == null) result = cloneScope
           result.nn.unlink(sym)
         }
-      if (result == null) this else result.nn
+      // TODO: improve flow typing to handle this case
+      if (result == null) this else result.uncheckedNN
     }
 
     def implicitDecls(using Context): List[TermRef] = Nil

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

@@ -86,7 +86,7 @@ object Symbols {
       ctx.settings.YcheckInit.value        // initialization check
 
     /** The last denotation of this symbol */
-    private var lastDenot: SymDenotation | Null = _
+    private var lastDenot: SymDenotation = _
     private var checkedPeriod: Period = Nowhere
 
     private[core] def invalidateDenotCache(): Unit = { checkedPeriod = Nowhere }
@@ -101,9 +101,8 @@ object Symbols {
     /** The current denotation of this symbol */
     final def denot(using Context): SymDenotation = {
       util.Stats.record("Symbol.denot")
-      val lastd = lastDenot.nn
-      if (checkedPeriod == ctx.period) lastd
-      else computeDenot(lastd)
+      if (checkedPeriod == ctx.period) lastDenot
+      else computeDenot(lastDenot)
     }
 
     private def computeDenot(lastd: SymDenotation)(using Context): SymDenotation = {
@@ -123,14 +122,14 @@ object Symbols {
 
     /** The original denotation of this symbol, without forcing anything */
     final def originDenotation: SymDenotation =
-      lastDenot.nn.initial
+      lastDenot.initial
 
     /** The last known denotation of this symbol, without going through `current` */
     final def lastKnownDenotation: SymDenotation =
-      lastDenot.nn
+      lastDenot
 
     private[core] def defRunId: RunId =
-      if (lastDenot == null) NoRunId else lastDenot.nn.validFor.runId
+      lastDenot.validFor.runId
 
     private inline def associatedFileMatches(inline filter: AbstractFile => Boolean)(using Context): Boolean =
       try
@@ -153,18 +152,17 @@ object Symbols {
 
     /** Is symbol valid in current run? */
     final def isValidInCurrentRun(using Context): Boolean =
-      val d = lastDenot.nn
-      (d.validFor.runId == ctx.runId || stillValid(d)) &&
-      (d.symbol eq this)
+      (lastDenot.validFor.runId == ctx.runId || stillValid(lastDenot)) &&
+      (lastDenot.symbol eq this)
         // the last condition is needed because under ctx.staleOK overwritten
         // members keep denotations pointing to the new symbol, so the validity
         // periods check out OK. But once a package member is overridden it is not longer
         // valid. If the option would be removed, the check would be no longer needed.
 
     final def isTerm(using Context): Boolean =
-      (if (defRunId == ctx.runId) lastDenot.nn else denot).isTerm
+      (if (defRunId == ctx.runId) lastDenot else denot).isTerm
     final def isType(using Context): Boolean =
-      (if (defRunId == ctx.runId) lastDenot.nn else denot).isType
+      (if (defRunId == ctx.runId) lastDenot else denot).isType
     final def asTerm(using Context): TermSymbol = {
       assert(isTerm, s"asTerm called on not-a-Term $this" );
       asInstanceOf[TermSymbol]
@@ -181,10 +179,8 @@ object Symbols {
      *  conservatively returns `false` if symbol does not yet have a denotation, or denotation
      *  is a class that is not yet read.
      */
-    final def isPrivate(using Context): Boolean = {
-      val d = lastDenot
-      d != null && d.flagsUNSAFE.is(Private)
-    }
+    final def isPrivate(using Context): Boolean =
+      lastDenot.flagsUNSAFE.is(Private)
 
     /** Is the symbol a pattern bound symbol?
      */
@@ -193,14 +189,14 @@ object Symbols {
 
     /** The symbol's signature if it is completed or a method, NotAMethod otherwise. */
     final def signature(using Context): Signature =
-      if (lastDenot != null && (lastDenot.uncheckedNN.isCompleted || lastDenot.uncheckedNN.is(Method)))
+      if lastDenot.uncheckedNN.isCompleted || lastDenot.uncheckedNN.is(Method) then
         denot.signature
       else
         Signature.NotAMethod
 
     /** Special cased here, because it may be used on naked symbols in substituters */
     final def isStatic(using Context): Boolean =
-      lastDenot != null && lastDenot.uncheckedNN.initial.isStatic
+      lastDenot.initial.isStatic
 
     /** This symbol entered into owner's scope (owner must be a class). */
     final def entered(using Context): this.type = {
@@ -268,7 +264,7 @@ object Symbols {
      *  Overridden in ClassSymbol
      */
     def associatedFile(using Context): AbstractFile | Null =
-      if (lastDenot == null) null else lastDenot.uncheckedNN.topLevelClass.associatedFile
+      lastDenot.topLevelClass.associatedFile
 
     /** The class file from which this class was generated, null if not applicable. */
     final def binaryFile(using Context): AbstractFile | Null = {
@@ -356,8 +352,7 @@ object Symbols {
     protected def prefixString: String = "Symbol"
 
     override def toString: String =
-      if (lastDenot == null) s"Naked$prefixString#$id"
-      else lastDenot.nn.toString // + "#" + id // !!! DEBUG
+      lastDenot.toString // + "#" + id // !!! DEBUG
 
     def toText(printer: Printer): Text = printer.toText(this)
 

compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala

@@ -544,8 +544,8 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
           val dotIndex = pathName.indexOf('.')
           val globalRef =
             if (dotIndex < 0) pathName
-            else pathName.substring(0, dotIndex)
-          checkGlobalRefName(globalRef.nn)
+            else pathName.substring(0, dotIndex).nn
+          checkGlobalRefName(globalRef)
         }
 
         checkAndGetJSNativeLoadingSpecAnnotOf(pos, sym) match {

compiler/src/dotty/tools/dotc/typer/Implicits.scala

@@ -364,7 +364,7 @@ object Implicits:
       if (monitored) record(s"check eligible refs in irefCtx", refs.length)
       val ownEligible = filterMatching(tp)
       if isOuterMost then ownEligible
-      else combineEligibles(ownEligible, outerImplicits.uncheckedNN.eligible(tp))
+      else combineEligibles(ownEligible, outerImplicits.nn.eligible(tp))
     }
 
     override def isAccessible(ref: TermRef)(using Context): Boolean =
@@ -381,7 +381,7 @@ object Implicits:
     def exclude(root: Symbol): ContextualImplicits =
       if (this == NoContext.implicits) this
       else {
-        val outerExcluded = outerImplicits.uncheckedNN exclude root
+        val outerExcluded = outerImplicits.nn exclude root
         if (irefCtx.importInfo.nn.site.termSymbol == root) outerExcluded
         else if (outerExcluded eqn outerImplicits) this
         else new ContextualImplicits(refs, outerExcluded, isImport)(irefCtx)

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -539,6 +539,7 @@ class Namer { typer: Typer =>
           if (fromTempl.derived.nonEmpty) {
             if (modTempl.derived.nonEmpty)
               report.error(em"a class and its companion cannot both have `derives` clauses", mdef.srcPos)
+            // `res` is inside a closure, so the flow-typing doesn't work here.
             res.uncheckedNN.putAttachment(desugar.DerivingCompanion, fromTempl.srcPos.startPos)
           }
           res.uncheckedNN

compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala

@@ -383,6 +383,7 @@ object ProtoTypes {
             targ = arg.withType(WildcardType)
           case _ =>
             targ = typerFn(arg)
+            // TODO: invest why flow typing is not working on `targ`
             if ctx.reporter.hasUnreportedErrors then
               if hasInnerErrors(targ.nn) then
                 state.errorArgs += arg

compiler/src/dotty/tools/dotc/util/HashSet.scala

@@ -89,7 +89,7 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu
       e = entryAt(idx)
     null
 
-/** Add entry at `x` at index `idx` */
+  /** Add entry at `x` at index `idx` */
   protected def addEntryAt(idx: Int, x: T): T =
     Stats.record(statsItem("addEntryAt"))
     setEntry(idx, x)
@@ -102,6 +102,7 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu
     var idx = firstIndex(x)
     var e: T | Null = entryAt(idx)
     while e != null do
+      // TODO: remove uncheckedNN when explicit-nulls is enabled for regule compiling
       if isEqual(e.uncheckedNN, x) then return e.uncheckedNN
       idx = nextIndex(idx)
       e = entryAt(idx)