Skip to content

Fix more problems detected by fuzzing in #4389 #4411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Apr 30, 2018
Merged
34 changes: 16 additions & 18 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -769,28 +769,26 @@ object Trees {
def genericEmptyTree[T >: Untyped]: Thicket[T] = theEmptyTree.asInstanceOf[Thicket[T]]

def flatten[T >: Untyped](trees: List[Tree[T]]): List[Tree[T]] = {
var buf: ListBuffer[Tree[T]] = null
var xs = trees
while (!xs.isEmpty) {
xs.head match {
case Thicket(elems) =>
if (buf == null) {
buf = new ListBuffer
var ys = trees
while (ys ne xs) {
buf += ys.head
ys = ys.tail
def recur(buf: ListBuffer[Tree[T]], remaining: List[Tree[T]]): ListBuffer[Tree[T]] =
remaining match {
case Thicket(elems) :: remaining1 =>
var buf1 = buf
if (buf1 == null) {
buf1 = new ListBuffer[Tree[T]]
var scanned = trees
while (scanned `ne` remaining) {
buf1 += scanned.head
scanned = scanned.tail
}
}
for (elem <- elems) {
assert(!elem.isInstanceOf[Thicket[_]])
buf += elem
}
case tree =>
recur(recur(buf1, elems), remaining1)
case tree :: remaining1 =>
if (buf != null) buf += tree
recur(buf, remaining1)
case nil =>
buf
}
xs = xs.tail
}
val buf = recur(null, trees)
if (buf != null) buf.toList else trees
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Constants.scala
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ object Constants {
*/
def convertTo(pt: Type)(implicit ctx: Context): Constant = {
def classBound(pt: Type): Type = pt.dealias.stripTypeVar match {
case tref: TypeRef if !tref.symbol.isClass => classBound(tref.info.bounds.lo)
case tref: TypeRef if !tref.symbol.isClass && tref.info.exists =>
classBound(tref.info.bounds.lo)
case param: TypeParamRef =>
ctx.typerState.constraint.entry(param) match {
case TypeBounds(lo, hi) =>
Expand Down
13 changes: 7 additions & 6 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1466,10 +1466,11 @@ object SymDenotations {
if (classParents.isEmpty && !emptyParentsExpected)
onBehalf.signalProvisional()
val builder = new BaseDataBuilder
for (p <- classParents) {
if (p.typeSymbol.isClass) builder.addAll(p.typeSymbol.asClass.baseClasses)
else assert(isRefinementClass || ctx.mode.is(Mode.Interactive), s"$this has non-class parent: $p")
}
for (p <- classParents)
p.classSymbol match {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incorrect indentation.

case pcls: ClassSymbol => builder.addAll(pcls.baseClasses)
case _ => assert(isRefinementClass || ctx.mode.is(Mode.Interactive), s"$this has non-class parent: $p")
}
(classSymbol :: builder.baseClasses, builder.baseClassSet)
}

Expand Down Expand Up @@ -1598,7 +1599,7 @@ object SymDenotations {
def collect(denots: PreDenotation, parents: List[Type]): PreDenotation = parents match {
case p :: ps =>
val denots1 = collect(denots, ps)
p.typeSymbol.denot match {
p.classSymbol.denot match {
case parentd: ClassDenotation =>
denots1 union
parentd.nonPrivateMembersNamed(name)
Expand Down Expand Up @@ -1747,7 +1748,7 @@ object SymDenotations {
var names = Set[Name]()
def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name
for (p <- classParents)
for (name <- p.typeSymbol.asClass.memberNames(keepOnly))
for (name <- p.classSymbol.asClass.memberNames(keepOnly))
maybeAdd(name)
val ownSyms =
if (keepOnly eq implicitFilter)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
val accu1 = if (accu exists (_ derivesFrom c)) accu else c :: accu
if (cs == c.baseClasses) accu1 else dominators(rest, accu1)
case Nil => // this case can happen because after erasure we do not have a top class anymore
assert(ctx.erasedTypes)
assert(ctx.erasedTypes || ctx.reporter.errorsReported)
defn.ObjectClass :: Nil
}

Expand Down
31 changes: 16 additions & 15 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,12 @@ object Types {
/** Is this type a value type? */
final def isValueType: Boolean = this.isInstanceOf[ValueType]

/** Is the is value type or type lambda? */
/** Is this a value type or a type lambda? */
final def isValueTypeOrLambda: Boolean = isValueType || this.isInstanceOf[TypeLambda]

/** Is this a value type or a wildcard? */
final def isValueTypeOrWildcard: Boolean = isValueType || this.isInstanceOf[WildcardType]

/** Does this type denote a stable reference (i.e. singleton type)? */
final def isStable(implicit ctx: Context): Boolean = stripTypeVar match {
case tp: TermRef => tp.termSymbol.isStable && tp.prefix.isStable || tp.info.isStable
Expand Down Expand Up @@ -589,13 +592,14 @@ object Types {

def goRefined(tp: RefinedType) = {
val pdenot = go(tp.parent)
val pinfo = pdenot.info
val rinfo = tp.refinedInfo
if (name.isTypeName) { // simplified case that runs more efficiently
if (name.isTypeName && !pinfo.isInstanceOf[ClassInfo]) { // simplified case that runs more efficiently
val jointInfo =
if (rinfo.isAlias) rinfo
else if (pdenot.info.isAlias) pdenot.info
else if (ctx.pendingMemberSearches.contains(name)) pdenot.info safe_& rinfo
else pdenot.info recoverable_& rinfo
else if (pinfo.isAlias) pinfo
else if (ctx.pendingMemberSearches.contains(name)) pinfo safe_& rinfo
else pinfo recoverable_& rinfo
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, jointInfo)
} else {
pdenot & (
Expand Down Expand Up @@ -1523,11 +1527,8 @@ object Types {
/** A marker trait for types that can be types of values or prototypes of value types */
trait ValueTypeOrProto extends TermType

/** A marker trait for types that can be types of values or wildcards */
trait ValueTypeOrWildcard extends TermType

/** A marker trait for types that can be types of values or that are higher-kinded */
trait ValueType extends ValueTypeOrProto with ValueTypeOrWildcard
trait ValueType extends ValueTypeOrProto

/** A marker trait for types that are guaranteed to contain only a
* single non-null value (they might contain null in addition).
Expand Down Expand Up @@ -2479,8 +2480,8 @@ object Types {

object AndType {
def apply(tp1: Type, tp2: Type)(implicit ctx: Context): AndType = {
assert(tp1.isInstanceOf[ValueTypeOrWildcard] &&
tp2.isInstanceOf[ValueTypeOrWildcard], i"$tp1 & $tp2 / " + s"$tp1 & $tp2")
assert(tp1.isValueTypeOrWildcard &&
tp2.isValueTypeOrWildcard, i"$tp1 & $tp2 / " + s"$tp1 & $tp2")
unchecked(tp1, tp2)
}

Expand Down Expand Up @@ -2524,8 +2525,8 @@ object Types {
myBaseClasses
}

assert(tp1.isInstanceOf[ValueTypeOrWildcard] &&
tp2.isInstanceOf[ValueTypeOrWildcard], s"$tp1 $tp2")
assert(tp1.isValueTypeOrWildcard &&
tp2.isValueTypeOrWildcard, s"$tp1 $tp2")

private[this] var myJoin: Type = _
private[this] var myJoinPeriod: Period = Nowhere
Expand Down Expand Up @@ -3477,7 +3478,7 @@ object Types {
if (selfTypeCache == null)
selfTypeCache = {
val given = cls.givenSelfType
if (!given.exists) appliedRef
if (!given.isValueType) appliedRef
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth explicitly checking that an error happened like other commits in this PR do, e.g.:

if (!given.isValueType) {
  assert(!given.exists || ctx.reporter.errorsReported)
  appliedRef
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The given type might be higher-kinded (in fact that's what it was in the test) but this would be detected only later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, in that case maybe just add a comment explaining that non-value types self can only happen with erroneous code? Otherwise readers of this code might be puzzled.

else if (cls is Module) given
else if (ctx.erasedTypes) appliedRef
else AndType(given, appliedRef)
Expand Down Expand Up @@ -3756,7 +3757,7 @@ object Types {
object TryDynamicCallType extends FlexType

/** Wildcard type, possibly with bounds */
abstract case class WildcardType(optBounds: Type) extends CachedGroundType with ValueTypeOrWildcard {
abstract case class WildcardType(optBounds: Type) extends CachedGroundType with TermType {
def derivedWildcardType(optBounds: Type)(implicit ctx: Context) =
if (optBounds eq this.optBounds) this
else if (!optBounds.exists) WildcardType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class CheckReentrant extends MiniPhase {
}
}
for (parent <- cls.classInfo.classParents)
addVars(parent.typeSymbol.asClass)
addVars(parent.classSymbol.asClass)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ object GenericSignatures {
val validParents =
if (isTraitSignature)
// java is unthrilled about seeing interfaces inherit from classes
minParents filter (p => isInterfaceOrTrait(p.typeSymbol))
minParents filter (p => isInterfaceOrTrait(p.classSymbol))
else minParents

val ps = ensureClassAsFirstParent(validParents)
Expand Down Expand Up @@ -329,11 +329,11 @@ object GenericSignatures {
def isUnshadowed(psym: Symbol) =
!(psyms exists (qsym => (psym ne qsym) && (qsym isSubClass psym)))
val cs = parents.iterator.filter { p => // isUnshadowed is a bit expensive, so try classes first
val psym = p.typeSymbol
val psym = p.classSymbol
psym.ensureCompleted()
psym.isClass && !psym.is(Trait) && isUnshadowed(psym)
}
(if (cs.hasNext) cs else parents.iterator.filter(p => isUnshadowed(p.typeSymbol))).next()
(if (cs.hasNext) cs else parents.iterator.filter(p => isUnshadowed(p.classSymbol))).next()
}
}
}
Expand Down
20 changes: 13 additions & 7 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -879,19 +879,25 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
*/
def trySelectUnapply(qual: untpd.Tree)(fallBack: Tree => Tree): Tree = {
// try first for non-overloaded, then for overloaded ocurrences
def tryWithName(name: TermName)(fallBack: Tree => Tree)(implicit ctx: Context): Tree =
tryEither { implicit ctx =>
val specificProto = new UnapplyFunProto(selType, this)
typedExpr(untpd.Select(qual, name), specificProto)
def tryWithName(name: TermName)(fallBack: Tree => Tree)(implicit ctx: Context): Tree = {
def tryWithProto(pt: Type)(implicit ctx: Context) = {
val result = typedExpr(untpd.Select(qual, name), new UnapplyFunProto(pt, this))
if (!result.symbol.exists || result.symbol.name == name) result
else notAnExtractor(result)
// It might be that the result of typedExpr is an `apply` selection or implicit conversion.
// Reject in this case.
}
tryEither {
implicit ctx => tryWithProto(selType)
} {
(sel, _) =>
tryEither { implicit ctx =>
val genericProto = new UnapplyFunProto(WildcardType, this)
typedExpr(untpd.Select(qual, name), genericProto)
tryEither {
implicit ctx => tryWithProto(WildcardType)
} {
(_, _) => fallBack(sel)
}
}
}
// try first for unapply, then for unapplySeq
tryWithName(nme.unapply) {
sel => tryWithName(nme.unapplySeq)(_ => fallBack(sel)) // for backwards compatibility; will be dropped
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ trait ImplicitRunInfo { self: Run =>
comps += companion.asSeenFrom(pre, compSym.owner).asInstanceOf[TermRef]
}
def addParentScope(parent: Type): Unit =
iscopeRefs(tp.baseType(parent.typeSymbol)) foreach addRef
iscopeRefs(tp.baseType(parent.classSymbol)) foreach addRef
val companion = cls.companionModule
if (companion.exists) addRef(companion.termRef)
cls.classParents foreach addParentScope
Expand Down
14 changes: 10 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,11 @@ object NamerContextOps {
/** Find moduleClass/sourceModule in effective scope */
private def findModuleBuddy(name: Name, scope: Scope)(implicit ctx: Context) = {
val it = scope.lookupAll(name).filter(_ is Module)
assert(it.hasNext, s"no companion $name in $scope")
it.next()
if (it.hasNext) it.next()
else {
assert(ctx.reporter.errorsReported, s"no companion $name in $scope")
NoSymbol
}
}
}

Expand Down Expand Up @@ -1033,8 +1036,11 @@ class Namer { typer: Typer =>
*/
def moduleValSig(sym: Symbol)(implicit ctx: Context): Type = {
val clsName = sym.name.moduleClassName
val cls = ctx.denotNamed(clsName) suchThat (_ is ModuleClass)
ctx.owner.thisType select (clsName, cls)
val cls = ctx.denotNamed(clsName).suchThat(_ is ModuleClass).orElse {
assert(ctx.reporter.errorsReported)
ctx.newStubSymbol(ctx.owner, clsName)
}
ctx.owner.thisType.select(clsName, cls)
}

/** The type signature of a ValDef or DefDef
Expand Down
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -472,16 +472,18 @@ object ProtoTypes {
}

/** Create a new TypeVar that represents a dependent method parameter singleton */
def newDepTypeVar(tp: Type)(implicit ctx: Context): TypeVar =
def newDepTypeVar(tp: Type)(implicit ctx: Context): TypeVar = {
newTypeVar(TypeBounds.upper(AndType(tp.widenExpr, defn.SingletonClass.typeRef)))

}
/** The result type of `mt`, where all references to parameters of `mt` are
* replaced by either wildcards (if typevarsMissContext) or TypeParamRefs.
*/
def resultTypeApprox(mt: MethodType)(implicit ctx: Context): Type =
if (mt.isResultDependent) {
def replacement(tp: Type) =
if (ctx.mode.is(Mode.TypevarsMissContext)) WildcardType else newDepTypeVar(tp)
if (ctx.mode.is(Mode.TypevarsMissContext) ||
!tp.widenExpr.isValueTypeOrWildcard) WildcardType
Copy link
Member

@smarter smarter Apr 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too, the error recovery case could be more explicit:

if (!tp.widenExpr.isValueTypeOrWildcard) {
  assert(ctx.reporter.errorsReported)
  WildcardType
}
else if (ctx.mode.is(Mode.TypevarsMissContext))
  WildcardType
else ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response: I am not sure the error would always be detected beforehand.

else newDepTypeVar(tp)
mt.resultType.substParams(mt, mt.paramInfos.map(replacement))
}
else mt.resultType
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ object RefChecks {
cls.pos)
}
for (parent <- cinfo.classParents)
checkSelfConforms(parent.typeSymbol.asClass, "illegal inheritance", "parent")
checkSelfConforms(parent.classSymbol.asClass, "illegal inheritance", "parent")
for (reqd <- cinfo.cls.givenSelfType.classSymbols)
checkSelfConforms(reqd, "missing requirement", "required")
case _ =>
Expand Down
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1447,7 +1447,11 @@ class Typer extends Namer
assignType(cpy.TypeDef(tdef)(name, rhs1), sym)
}

def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(implicit ctx: Context) = track("typedClassDef") {
def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(implicit ctx: Context): Tree = track("typedClassDef") {
if (!cls.info.isInstanceOf[ClassInfo]) {
assert(ctx.reporter.errorsReported)
return cdef.withType(UnspecifiedErrorType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally we should have a way to recursively set the type of the tree children to maintain the invariant that a typed tree only contains typed trees.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that's not so easy. Sometimes, there's simply no way to proceed. And we cannot just assign an ErrorType, because a lot of trees are not typed trees, so we'd have to replace them with something else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess they could be replaced by ErrorTrees.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, maybe cdef should just be replaced by an ErrorTree since it's unusable anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, yes. Let's do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm wait. What do you mean by ErrorTree? We only have errorTree which gives an error type to an existing tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we can replace it with an EmptyTree instead.

}
val TypeDef(name, impl @ Template(constr, parents, self, _)) = cdef
val superCtx = ctx.superCallContext

Expand Down
6 changes: 6 additions & 0 deletions tests/neg/bad-selftype.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
trait x0[T] { self: x0 => } // error

trait x1[T] { self: (=> String) => } // error

trait x2[T] { self: ([X] => X) => } // error

5 changes: 5 additions & 0 deletions tests/neg/parser-stability-11.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package x0 {
case class x0 // error // error
}
package x0
class x0 // error // error
3 changes: 3 additions & 0 deletions tests/neg/parser-stability-12.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait x0[] // error
trait x1[x1 <:x0]
extends x1[ // error
4 changes: 4 additions & 0 deletions tests/neg/parser-stability-14.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object x0 {
{
val x1 x0 // error
object x1 // error // error
6 changes: 6 additions & 0 deletions tests/neg/parser-stability-15.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package x0
class x0 {
def x1 =
x2 match // error
] // error
case x3[] => x0 // error // error // error // error
5 changes: 5 additions & 0 deletions tests/neg/parser-stability-16.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class x0[x0] {
val x1 : x0
}
trait x3 extends x0 {
x1 = 0 object // error // error
2 changes: 2 additions & 0 deletions tests/neg/parser-stability-17.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
trait x0[] { x0: x0 => } // error // error
class x0[x1] extends x0[x0 x0] x2 x0 // error // error
4 changes: 4 additions & 0 deletions tests/neg/parser-stability-18.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
trait x0 {
class x1 (x1:x0
{
type x1 <: List[x1 <: // error // error
4 changes: 4 additions & 0 deletions tests/neg/parser-stability-19.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object x0 {
case class x0[] // error // error
def x0( ) ] // error // error
def x0 ( x0:x0 ):x0.type = x1 x0 // error // error // error
5 changes: 5 additions & 0 deletions tests/neg/parser-stability-20.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object x0 {
def unapply= Array
x0 match
x0 // error
case x0( // error // error
2 changes: 2 additions & 0 deletions tests/neg/parser-stability-21.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class x0[x1[]] // error
extends x1[ // error // error
Loading