Skip to content

Commit fde88c7

Browse files
committed
No longer crash on NoSymbol.owner.
Historically calling NoSymbol.owner has crashed the compiler. With this commit, NoSymbol owns itself. This is consistent with the way ownership chains are handled elsewhere in the compiler (e.g. NoContext.owner is NoContext, NoSymbol.enclClass is NoSymbol, and so on) and frees every call site which handles symbols from having to perform precondition tests against NoSymbol. Since calling NoSymbol.owner sometimes (not always) indicates a bug which we'd like to catch sooner than later, I have introduced a couple more methods for selected call sites. def owner: Symbol // NoSymbol.owner is self, log if -Xdev def safeOwner: Symbol // NoSymbol.owner is self, ignore def assertOwner: Symbol // NoSymbol.owner is fatal The idea is that everyone can call sym.owner without undue anxiety or paranoid null-like tests. When compiling under -Xdev calls to `owner` are logged with a stack trace, so any call sites for which that is an expected occurrence should call safeOwner instead to communicate the intention and stay out of the log. Conversely, any call site where crashing on the owner call was a desirable behavior can opt into calling assertOwner. This commit also includes all the safeOwner calls necessary to give us a silent log when compiling scala.
1 parent a124ab7 commit fde88c7

27 files changed

+96
-127
lines changed

src/compiler/scala/reflect/macros/compiler/Validators.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,11 @@ trait Validators {
8181

8282
// Technically this can be just an alias to MethodType, but promoting it to a first-class entity
8383
// provides better encapsulation and convenient syntax for pattern matching.
84-
private case class MacroImplSig(tparams: List[Symbol], paramss: List[List[Symbol]], ret: Type)
84+
private case class MacroImplSig(tparams: List[Symbol], paramss: List[List[Symbol]], ret: Type) {
85+
private def tparams_s = if (tparams.isEmpty) "" else tparams.map(_.defString).mkString("[", ", ", "]")
86+
private def paramss_s = paramss map (ps => ps.map(s => s"${s.name}: ${s.tpe_*}").mkString("(", ", ", ")")) mkString ""
87+
override def toString = "MacroImplSig(" + tparams_s + paramss_s + ret + ")"
88+
}
8589

8690
/** An actual macro implementation signature extracted from a macro implementation method.
8791
*

src/compiler/scala/tools/nsc/Global.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,17 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
258258
if (settings.debug)
259259
body
260260
}
261+
262+
override protected def isDeveloper = settings.developer || super.isDeveloper
263+
261264
/** This is for WARNINGS which should reach the ears of scala developers
262265
* whenever they occur, but are not useful for normal users. They should
263266
* be precise, explanatory, and infrequent. Please don't use this as a
264267
* logging mechanism. !!! is prefixed to all messages issued via this route
265268
* to make them visually distinct.
266269
*/
267270
@inline final override def devWarning(msg: => String) {
268-
if (settings.developer || settings.debug)
271+
if (isDeveloper)
269272
warning("!!! " + msg)
270273
else
271274
log("!!! " + msg) // such warnings always at least logged

src/compiler/scala/tools/nsc/ast/Trees.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,7 @@ trait Trees extends scala.reflect.internal.Trees { self: Global =>
6666
*/
6767
def ClassDef(sym: Symbol, constrMods: Modifiers, vparamss: List[List[ValDef]], body: List[Tree], superPos: Position): ClassDef = {
6868
// "if they have symbols they should be owned by `sym`"
69-
assert(
70-
mforall(vparamss)(p => (p.symbol eq NoSymbol) || (p.symbol.owner == sym)),
71-
((mmap(vparamss)(_.symbol), sym))
72-
)
69+
assert(mforall(vparamss)(_.symbol.owner == sym), (mmap(vparamss)(_.symbol), sym))
7370

7471
ClassDef(sym,
7572
gen.mkTemplate(sym.info.parents map TypeTree,

src/compiler/scala/tools/nsc/backend/opt/Inliners.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,10 @@ abstract class Inliners extends SubComponent {
7878
assert(clazz != NoSymbol, "Walked up past Object.superClass looking for " + sym +
7979
", most likely this reveals the TFA at fault (receiver and callee don't match).")
8080
if (sym.owner == clazz || isBottomType(clazz)) sym
81-
else sym.overridingSymbol(clazz) match {
82-
case NoSymbol => if (sym.owner.isTrait) sym else lookup(clazz.superClass)
83-
case imp => imp
84-
}
81+
else sym.overridingSymbol(clazz) orElse (
82+
if (sym.owner.isTrait) sym
83+
else lookup(clazz.superClass)
84+
)
8585
}
8686
if (needsLookup) {
8787
val concreteMethod = lookup(clazz)

src/compiler/scala/tools/nsc/transform/AddInterfaces.scala

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -367,29 +367,3 @@ abstract class AddInterfaces extends InfoTransform { self: Erasure =>
367367
}
368368
}
369369
}
370-
/*
371-
val ensureNoEscapes = new TypeTraverser {
372-
def ensureNoEscape(sym: Symbol) {
373-
if (sym.hasFlag(PRIVATE)) {
374-
var o = currentOwner;
375-
while (o != NoSymbol && o != sym.owner && !o.isLocal && !o.hasFlag(PRIVATE))
376-
o = o.owner
377-
if (o == sym.owner) sym.makeNotPrivate(base);
378-
}
379-
}
380-
def traverse(t: Type): TypeTraverser = {
381-
t match {
382-
case TypeRef(qual, sym, args) =>
383-
ensureNoEscape(sym)
384-
mapOver(t)
385-
case ClassInfoType(parents, decls, clazz) =>
386-
parents foreach { p => traverse; () }
387-
traverse(t.typeOfThis)
388-
case _ =>
389-
mapOver(t)
390-
}
391-
this
392-
}
393-
}
394-
395-
*/

src/compiler/scala/tools/nsc/transform/Erasure.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,7 @@ abstract class Erasure extends AddInterfaces
525525
private def isDifferentErasedValueType(tpe: Type, other: Type) =
526526
isErasedValueType(tpe) && (tpe ne other)
527527

528-
private def isPrimitiveValueMember(sym: Symbol) =
529-
sym != NoSymbol && isPrimitiveValueClass(sym.owner)
528+
private def isPrimitiveValueMember(sym: Symbol) = isPrimitiveValueClass(sym.owner)
530529

531530
@inline private def box(tree: Tree, target: => String): Tree = {
532531
val result = box1(tree)

src/compiler/scala/tools/nsc/transform/LambdaLift.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ abstract class LambdaLift extends InfoTransform {
489489
treeCopy.Assign(tree, qual, rhs)
490490
case Ident(name) =>
491491
val tree1 =
492-
if (sym != NoSymbol && sym.isTerm && !sym.isLabel)
492+
if (sym.isTerm && !sym.isLabel)
493493
if (sym.isMethod)
494494
atPos(tree.pos)(memberRef(sym))
495495
else if (sym.isLocal && !isSameOwnerEnclosure(sym))

src/compiler/scala/tools/nsc/transform/Mixin.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -734,10 +734,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
734734
sym
735735
}
736736

737-
if (sym ne NoSymbol)
738-
sym
739-
else
740-
createBitmap
737+
sym orElse createBitmap
741738
}
742739

743740
def maskForOffset(offset: Int, sym: Symbol, kind: ClassSymbol): Tree = {

src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
222222
// so that we don't introduce new aliases for existing symbols, thus keeping the set of bound symbols minimal
223223
val (boundSubst, unboundSubst) = (subst.from zip subst.to) partition {
224224
case (f, t) =>
225-
t.isInstanceOf[Ident] && (t.symbol ne NoSymbol) && pointsToBound(f)
225+
t.isInstanceOf[Ident] && t.symbol.exists && pointsToBound(f)
226226
}
227227
val (boundFrom, boundTo) = boundSubst.unzip
228228
val (unboundFrom, unboundTo) = unboundSubst.unzip
@@ -624,9 +624,9 @@ trait MatchAnalysis extends MatchApproximation {
624624
private lazy val uniqueEqualTo = equalTo filterNot (subsumed => equalTo.exists(better => (better ne subsumed) && instanceOfTpImplies(better.tp, subsumed.tp)))
625625
private lazy val prunedEqualTo = uniqueEqualTo filterNot (subsumed => variable.staticTpCheckable <:< subsumed.tp)
626626
private lazy val ctor = (prunedEqualTo match { case List(TypeConst(tp)) => tp case _ => variable.staticTpCheckable }).typeSymbol.primaryConstructor
627-
private lazy val ctorParams = if (ctor == NoSymbol || ctor.paramss.isEmpty) Nil else ctor.paramss.head
628-
private lazy val cls = if (ctor == NoSymbol) NoSymbol else ctor.owner
629-
private lazy val caseFieldAccs = if (cls == NoSymbol) Nil else cls.caseFieldAccessors
627+
private lazy val ctorParams = if (ctor.paramss.isEmpty) Nil else ctor.paramss.head
628+
private lazy val cls = ctor.safeOwner
629+
private lazy val caseFieldAccs = cls.caseFieldAccessors
630630

631631
def addField(symbol: Symbol, assign: VariableAssignment) {
632632
// SI-7669 Only register this field if if this class contains it.

src/compiler/scala/tools/nsc/transform/patmat/MatchCodeGen.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,13 @@ trait MatchCodeGen extends Interface {
173173
// catchAll.isEmpty iff no synthetic default case needed (the (last) user-defined case is a default)
174174
// if the last user-defined case is a default, it will never jump to the next case; it will go immediately to matchEnd
175175
val catchAllDef = matchFailGen map { matchFailGen =>
176-
val scrutRef = if(scrutSym ne NoSymbol) REF(scrutSym) else EmptyTree // for alternatives
176+
val scrutRef = scrutSym.fold(EmptyTree: Tree)(REF) // for alternatives
177177

178178
LabelDef(_currCase, Nil, matchEnd APPLY (matchFailGen(scrutRef)))
179179
} toList // at most 1 element
180180

181181
// scrutSym == NoSymbol when generating an alternatives matcher
182-
val scrutDef = if(scrutSym ne NoSymbol) List(VAL(scrutSym) === scrut) else Nil // for alternatives
182+
val scrutDef = scrutSym.fold(List[Tree]())(sym => (VAL(sym) === scrut) :: Nil) // for alternatives
183183

184184
// the generated block is taken apart in TailCalls under the following assumptions
185185
// the assumption is once we encounter a case, the remainder of the block will consist of cases

0 commit comments

Comments
 (0)