Skip to content

Fix #2220: Change handling of package objects and tweak hk type inference #2205

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 4 commits into from
Apr 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 51 additions & 21 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1493,6 +1493,9 @@ object SymDenotations {
myMemberCache
}

/** Hook to do a pre-enter test. Overridden in PackageDenotation */
protected def proceedWithEnter(sym: Symbol, mscope: MutableScope)(implicit ctx: Context): Boolean = true

/** Enter a symbol in current scope, and future scopes of same denotation.
* Note: We require that this does not happen after the first time
* someone does a findMember on a subclass.
Expand All @@ -1510,19 +1513,13 @@ object SymDenotations {
scope
case _ => unforcedDecls.openForMutations
}
if (this is PackageClass) {
val entry = mscope.lookupEntry(sym.name)
if (entry != null) {
if (entry.sym == sym) return
mscope.unlink(entry)
entry.sym.denot = sym.denot // to avoid stale symbols
if (proceedWithEnter(sym, mscope)) {
enterNoReplace(sym, mscope)
val nxt = this.nextInRun
if (nxt.validFor.code > this.validFor.code) {
this.nextInRun.asSymDenotation.asClass.enter(sym)
}
}
enterNoReplace(sym, mscope)
val nxt = this.nextInRun
if (nxt.validFor.code > this.validFor.code) {
this.nextInRun.asSymDenotation.asClass.enter(sym)
}
}

/** Enter a symbol in given `scope` without potentially replacing the old copy. */
Expand All @@ -1534,7 +1531,7 @@ object SymDenotations {
(scope ne this.unforcedDecls) ||
sym.hasAnnotation(defn.ScalaStaticAnnot) ||
sym.name.isInlineAccessor ||
isUsecase)
isUsecase, i"trying to enter $sym in $this, frozen = ${this is Frozen}")

scope.enter(sym)

Expand Down Expand Up @@ -1800,7 +1797,7 @@ object SymDenotations {
/** The denotation of a package class.
* It overrides ClassDenotation to take account of package objects when looking for members
*/
class PackageClassDenotation private[SymDenotations] (
final class PackageClassDenotation private[SymDenotations] (
symbol: Symbol,
ownerIfExists: Symbol,
name: Name,
Expand All @@ -1823,15 +1820,33 @@ object SymDenotations {
packageObjCache
}

/** Look first for members in package; if none are found look in package object */
override def computeNPMembersNamed(name: Name, inherited: Boolean)(implicit ctx: Context): PreDenotation = {
val denots = super.computeNPMembersNamed(name, inherited)
if (denots.exists) denots
else packageObj.moduleClass.denot match {
case pcls: ClassDenotation => pcls.computeNPMembersNamed(name, inherited)
case _ => denots
/** Looks in both the package object and the package for members. The precise algorithm
* is as follows:
*
* If this is the scala package look in the package first, and if nothing is found
* there, look in the package object second. Otherwise, look in the package object
* first, and if nothing is found there, in the package second.
*
* The reason for the special treatment of the scala package is that if we
* complete it too early, we freeze its superclass Any, so that no members can
* be entered in it. As a consequence, there should be no entry in the scala package
* object that hides a class or object in the scala package of the same name, because
* the behavior would then be unintuitive for such members.
Copy link
Member

Choose a reason for hiding this comment

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

Would it possible to detect when a scala package object member hides a scala class/object and return an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. It would be tricky since everything we do in the scala package is fraught with causing all sorts of initialization errors. Not sure it's worth it really, given that we never had a problem so far.

*/
override def computeNPMembersNamed(name: Name, inherited: Boolean)(implicit ctx: Context): PreDenotation =
packageObj.moduleClass.denot match {
case pcls: ClassDenotation if !pcls.isCompleting =>
if (symbol eq defn.ScalaPackageClass) {
val denots = super.computeNPMembersNamed(name, inherited)
if (denots.exists) denots else pcls.computeNPMembersNamed(name, inherited)
}
else {
val denots = pcls.computeNPMembersNamed(name, inherited)
if (denots.exists) denots else super.computeNPMembersNamed(name, inherited)
}
case _ =>
super.computeNPMembersNamed(name, inherited)
}
}

/** The union of the member names of the package and the package object */
override def memberNames(keepOnly: NameFilter)(implicit ctx: Context): Set[Name] = {
Expand All @@ -1841,6 +1856,21 @@ object SymDenotations {
case _ => ownNames
}
}

/** If another symbol with the same name is entered, unlink it,
* and, if symbol is a package object, invalidate the packageObj cache.
* @return `sym` is not already entered
*/
override def proceedWithEnter(sym: Symbol, mscope: MutableScope)(implicit ctx: Context): Boolean = {
val entry = mscope.lookupEntry(sym.name)
if (entry != null) {
if (entry.sym == sym) return false
mscope.unlink(entry)
entry.sym.denot = sym.denot // to avoid stale symbols
if (sym.name == nme.PACKAGE) packageObjRunId = NoRunId
}
true
}
}

class NoDenotation extends SymDenotation(
Expand Down
7 changes: 3 additions & 4 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -726,10 +726,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {

tycon2 match {
case param2: TypeParamRef =>
isMatchingApply(tp1) || {
if (canConstrain(param2)) canInstantiate(param2)
else compareLower(bounds(param2), tyconIsTypeRef = false)
}
isMatchingApply(tp1) ||
canConstrain(param2) && canInstantiate(param2) ||
Copy link
Member

Choose a reason for hiding this comment

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

I have an alternative fix for #2201 at #2204. Your fix looks like it might help in some other situations, but we would need a testcase for 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.

Just merged #2204. I don't have a separate test case for this change, but would argue that the fact that we missed the case before means we cannot exclude that we will miss it in the future. So better play it safe.

compareLower(bounds(param2), tyconIsTypeRef = false)
case tycon2: TypeRef =>
isMatchingApply(tp1) ||
compareLower(tycon2.info.bounds, tyconIsTypeRef = true)
Expand Down
23 changes: 16 additions & 7 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1504,20 +1504,29 @@ object Types {
case _ => NoType
}
assert(
(lastSymbol eq sym) ||
(lastSymbol eq null) || {
(lastSymbol eq sym)
||
(lastSymbol eq null)
|| {
val lastDefRunId = lastDenotation match {
case d: SymDenotation => d.validFor.runId
case _ => lastSymbol.defRunId
}
(lastDefRunId != sym.defRunId) ||
(lastDefRunId == NoRunId)
} ||
(lastSymbol.infoOrCompleter.isInstanceOf[ErrorType] ||
}
||
lastSymbol.infoOrCompleter.isInstanceOf[ErrorType]
||
sym.isPackageObject // package objects can be visited before we get around to index them
||
sym.owner != lastSymbol.owner &&
(sym.owner.derivesFrom(lastSymbol.owner) ||
selfTypeOf(sym).derivesFrom(lastSymbol.owner) ||
selfTypeOf(lastSymbol).derivesFrom(sym.owner))),
(sym.owner.derivesFrom(lastSymbol.owner)
||
selfTypeOf(sym).derivesFrom(lastSymbol.owner)
||
selfTypeOf(lastSymbol).derivesFrom(sym.owner)
),
i"""data race? overwriting symbol of type $this,
|long form = $toString of class $getClass,
|last sym id = ${lastSymbol.id}, new sym id = ${sym.id},
Expand Down
6 changes: 6 additions & 0 deletions tests/pos/i2200/Hello.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package bar
import scala.language.higherKinds
class Fix[F[_]](unfix: F[Fix[F]])
object DocTree {
def docTree(s: StreamTree[DocTree]): DocTree = new Fix(s: StreamTree[DocTree])
}
4 changes: 4 additions & 0 deletions tests/pos/i2200/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package object bar {
type StreamTree[T] = Stream[Int]
type DocTree = Fix[StreamTree]
}