Skip to content

use SELECTin by default in TASTy #11210

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 3 commits into from
Mar 23, 2021
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
35 changes: 23 additions & 12 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ object Denotations {
val jointInfo = infoMeet(info1, info2, safeIntersection)
if jointInfo.exists then
val sym = if symScore >= 0 then sym1 else sym2
JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor, pre)
JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor, pre, denot1.isRefinedMethod || denot2.isRefinedMethod)
else if symScore == 2 then denot1
else if symScore == -2 then denot2
else
Expand Down Expand Up @@ -569,7 +569,7 @@ object Denotations {

/** A non-overloaded denotation */
abstract class SingleDenotation(symbol: Symbol, initInfo: Type) extends Denotation(symbol, initInfo) {
protected def newLikeThis(symbol: Symbol, info: Type, pre: Type): SingleDenotation
protected def newLikeThis(symbol: Symbol, info: Type, pre: Type, isRefinedMethod: Boolean): SingleDenotation

final def name(using Context): Name = symbol.name

Expand All @@ -582,6 +582,9 @@ object Denotations {
*/
def prefix: Type = NoPrefix

/** True if the info of this denotation comes from a refinement. */
def isRefinedMethod: Boolean = false

/** For SymDenotations, the language-specific signature of the info, depending on
* where the symbol is defined. For non-SymDenotations, the Scala 3
* signature.
Expand Down Expand Up @@ -615,9 +618,9 @@ object Denotations {
case _ => Signature.NotAMethod
}

def derivedSingleDenotation(symbol: Symbol, info: Type, pre: Type = this.prefix)(using Context): SingleDenotation =
if ((symbol eq this.symbol) && (info eq this.info) && (pre eq this.prefix)) this
else newLikeThis(symbol, info, pre)
def derivedSingleDenotation(symbol: Symbol, info: Type, pre: Type = this.prefix, isRefinedMethod: Boolean = this.isRefinedMethod)(using Context): SingleDenotation =
if ((symbol eq this.symbol) && (info eq this.info) && (pre eq this.prefix) && (isRefinedMethod == this.isRefinedMethod)) this
else newLikeThis(symbol, info, pre, isRefinedMethod)

def mapInfo(f: Type => Type)(using Context): SingleDenotation =
derivedSingleDenotation(symbol, f(info))
Expand Down Expand Up @@ -1107,7 +1110,11 @@ object Denotations {
case sd: SymDenotation => true
case _ => info eq symbol.info

if !owner.membersNeedAsSeenFrom(pre) && ((pre ne owner.thisType) || hasOriginalInfo)
def ownerIsPrefix = pre match
case pre: ThisType => pre.sameThis(owner.thisType)
case _ => false

if !owner.membersNeedAsSeenFrom(pre) && (!ownerIsPrefix || hasOriginalInfo)
|| symbol.is(NonMember)
then this
else derived(symbol.info)
Expand All @@ -1126,26 +1133,30 @@ object Denotations {
prefix: Type) extends NonSymSingleDenotation(symbol, initInfo, prefix) {
validFor = initValidFor
override def hasUniqueSym: Boolean = true
protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
new UniqueRefDenotation(s, i, validFor, pre)
protected def newLikeThis(s: Symbol, i: Type, pre: Type, isRefinedMethod: Boolean): SingleDenotation =
if isRefinedMethod then
new JointRefDenotation(s, i, validFor, pre, isRefinedMethod)
else
new UniqueRefDenotation(s, i, validFor, pre)
}

class JointRefDenotation(
symbol: Symbol,
initInfo: Type,
initValidFor: Period,
prefix: Type) extends NonSymSingleDenotation(symbol, initInfo, prefix) {
prefix: Type,
override val isRefinedMethod: Boolean) extends NonSymSingleDenotation(symbol, initInfo, prefix) {
validFor = initValidFor
override def hasUniqueSym: Boolean = false
protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
new JointRefDenotation(s, i, validFor, pre)
protected def newLikeThis(s: Symbol, i: Type, pre: Type, isRefinedMethod: Boolean): SingleDenotation =
new JointRefDenotation(s, i, validFor, pre, isRefinedMethod)
}

class ErrorDenotation(using Context) extends NonSymSingleDenotation(NoSymbol, NoType, NoType) {
override def exists: Boolean = false
override def hasUniqueSym: Boolean = false
validFor = Period.allInRun(ctx.runId)
protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
protected def newLikeThis(s: Symbol, i: Type, pre: Type, isRefinedMethod: Boolean): SingleDenotation =
this
}

Expand Down
12 changes: 9 additions & 3 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -896,11 +896,14 @@ object SymDenotations {
* accessed via prefix `pre`?
*/
def membersNeedAsSeenFrom(pre: Type)(using Context): Boolean =
def preIsThis = pre match
case pre: ThisType => pre.sameThis(thisType)
case _ => false
!( this.isTerm
|| this.isStaticOwner && !this.seesOpaques
|| ctx.erasedTypes
|| (pre eq NoPrefix)
|| (pre eq thisType)
|| preIsThis
)

/** Is this symbol concrete, or that symbol deferred? */
Expand Down Expand Up @@ -1504,8 +1507,11 @@ object SymDenotations {

// ----- copies and transforms ----------------------------------------

protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
new UniqueRefDenotation(s, i, validFor, pre)
protected def newLikeThis(s: Symbol, i: Type, pre: Type, isRefinedMethod: Boolean): SingleDenotation =
if isRefinedMethod then
new JointRefDenotation(s, i, validFor, pre, isRefinedMethod)
else
new UniqueRefDenotation(s, i, validFor, pre)

/** Copy this denotation, overriding selective fields */
final def copySymDenotation(
Expand Down
18 changes: 14 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -772,15 +772,16 @@ object Types {
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, jointInfo)
}
else
val isRefinedMethod = rinfo.isInstanceOf[MethodOrPoly]
val joint = pdenot.meet(
new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre),
new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre, isRefinedMethod),
pre,
safeIntersection = ctx.base.pendingMemberSearches.contains(name))
joint match
case joint: SingleDenotation
if rinfo.isInstanceOf[MethodOrPoly] && rinfo <:< joint.info =>
if isRefinedMethod && rinfo <:< joint.info =>
// use `rinfo` to keep the right parameter names for named args. See i8516.scala.
joint.derivedSingleDenotation(joint.symbol, rinfo)
joint.derivedSingleDenotation(joint.symbol, rinfo, pre, isRefinedMethod)
case _ =>
joint
}
Expand Down Expand Up @@ -2662,8 +2663,11 @@ object Types {
* the future. See also NamedType#withDenot. Test case is neg/opaque-self-encoding.scala.
*/
private def designatorFor(prefix: Type, name: Name, denot: Denotation)(using Context): Designator = {
def ownerIsPrefix(owner: Symbol) = prefix match
case prefix: ThisType => prefix.sameThis(owner.thisType)
case _ => false
val sym = denot.symbol
if (sym.exists && (prefix.eq(NoPrefix) || prefix.ne(sym.owner.thisType)))
if (sym.exists && (prefix.eq(NoPrefix) || !ownerIsPrefix(sym.owner)))
sym
else
name
Expand Down Expand Up @@ -2735,6 +2739,12 @@ object Types {
case that: ThisType => tref.eq(that.tref)
case _ => false
}

/** Check that the rhs is a ThisType that refers to the same class.
*/
def sameThis(that: Type)(using Context): Boolean = (that eq this) || that.match
case that: ThisType => this.cls eq that.cls
case _ => false
}

final class CachedThisType(tref: TypeRef) extends ThisType(tref)
Expand Down
21 changes: 11 additions & 10 deletions compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -409,22 +409,23 @@ class TreePickler(pickler: TastyPickler) {
case _ =>
val sig = tree.tpe.signature
var ename = tree.symbol.targetName
val isAmbiguous =
sig != Signature.NotAMethod
&& qual.tpe.nonPrivateMember(name).match
case d: MultiDenotation => d.atSignature(sig, ename).isInstanceOf[MultiDenotation]
case _ => false
if isAmbiguous then
val selectFromQualifier =
name.isTypeName
|| qual.isInstanceOf[TreePickler.Hole] // holes have no symbol
|| sig == Signature.NotAMethod // no overload resolution necessary
|| !tree.denot.symbol.exists // polymorphic function type
|| tree.denot.asSingleDenotation.isRefinedMethod // refined methods have no defining class symbol
if selectFromQualifier then
writeByte(if name.isTypeName then SELECTtpt else SELECT)
pickleNameAndSig(name, sig, ename)
pickleTree(qual)
else // select from owner
writeByte(SELECTin)
withLength {
pickleNameAndSig(name, tree.symbol.signature, ename)
pickleTree(qual)
pickleType(tree.symbol.owner.typeRef)
}
else
writeByte(if (name.isTypeName) SELECTtpt else SELECT)
pickleNameAndSig(name, sig, ename)
pickleTree(qual)
}
case Apply(fun, args) =>
if (fun.symbol eq defn.throwMethod) {
Expand Down
39 changes: 29 additions & 10 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1186,15 +1186,34 @@ class TreeUnpickler(reader: TastyReader,
case SELECTin =>
var sname = readName()
val qual = readTerm()
val owner = readType()
def select(name: Name, denot: Denotation) =
val prefix = ctx.typeAssigner.maybeSkolemizePrefix(qual.tpe.widenIfUnstable, name)
makeSelect(qual, name, denot.asSeenFrom(prefix))
sname match
case SignedName(name, sig, target) =>
select(name, owner.decl(name).atSignature(sig, target))
case name =>
select(name, owner.decl(name))
val ownerTpe = readType()
val owner = ownerTpe.typeSymbol
val SignedName(name, sig, target) = sname: @unchecked // only methods with params use SELECTin
val qualType = qual.tpe.widenIfUnstable
val prefix = ctx.typeAssigner.maybeSkolemizePrefix(qualType, name)

/** Tasty should still be able to resolve a method from another root class,
* even if it has been moved to a super type,
* or an override has been removed.
*
* This is tested in
* - sbt-dotty/sbt-test/tasty-compat/remove-override
* - sbt-dotty/sbt-test/tasty-compat/move-method
*/
def lookupInSuper =
val cls = ownerTpe.classSymbol
if cls.exists then
cls.asClass.classDenot
.findMember(name, cls.thisType, EmptyFlags, excluded=Private)
.atSignature(sig, target)
else
NoDenotation

val denot =
val d = ownerTpe.decl(name).atSignature(sig, target)
(if !d.exists then lookupInSuper else d).asSeenFrom(prefix)

makeSelect(qual, name, denot)
case REPEATED =>
val elemtpt = readTpt()
SeqLiteral(until(end)(readTerm()), elemtpt)
Expand Down Expand Up @@ -1494,4 +1513,4 @@ object TreeUnpickler {
final val AllDefs = 2 // add everything

class TreeWithoutOwner extends Exception
}
}
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,13 @@ trait MessageRendering {
if (posString.nonEmpty) sb.append(posString).append(EOL)
if (pos.exists) {
val pos1 = pos.nonInlined
val (srcBefore, srcAfter, offset) = sourceLines(pos1, diagnosticLevel)
val marker = columnMarker(pos1, offset, diagnosticLevel)
val err = errorMsg(pos1, msg.message, offset)
sb.append((srcBefore ::: marker :: err :: outer(pos, " " * (offset - 1)) ::: srcAfter).mkString(EOL))
if (pos1.exists && pos1.source.file.exists) {
val (srcBefore, srcAfter, offset) = sourceLines(pos1, diagnosticLevel)
val marker = columnMarker(pos1, offset, diagnosticLevel)
val err = errorMsg(pos1, msg.message, offset)
sb.append((srcBefore ::: marker :: err :: outer(pos, " " * (offset - 1)) ::: srcAfter).mkString(EOL))
}
else sb.append(msg.message)
}
else sb.append(msg.message)
sb.toString
Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@ object Scala3:

/** Is symbol global? Non-global symbols get localN names */
def isGlobal(using Context): Boolean =
sym.is(Package)
|| !sym.isSelfSym && (sym.is(Param) || sym.owner.isClass) && sym.owner.isGlobal
sym.exists && (
sym.is(Package)
|| !sym.isSelfSym && (sym.is(Param) || sym.owner.isClass) && sym.owner.isGlobal
)

def isLocalWithinSameName(using Context): Boolean =
sym.exists && !sym.isGlobal && sym.name == sym.owner.name
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/util/SourceFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends
var idx = startOfLine(offset)
var col = 0
while (idx != offset) {
col += (if (idx < length && content()(idx) == '\t') (tabInc - col) % tabInc else 1)
col += (if (idx < content().length && content()(idx) == '\t') (tabInc - col) % tabInc else 1)
Copy link
Member

Choose a reason for hiding this comment

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

@nicolasstucki Why does length not just call content().length ?

idx += 1
}
col
Expand Down Expand Up @@ -285,4 +285,3 @@ object SourceFile {
override def exists: Boolean = false
override def atSpan(span: Span): SourcePosition = NoSourcePosition
}

10 changes: 10 additions & 0 deletions sbt-dotty/sbt-test/tasty-compat/add-overload/a-changes/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package a

object A {

class Buf[A] {
def append(a: A): this.type = this
def append(a: A*): this.type = this
}

}
9 changes: 9 additions & 0 deletions sbt-dotty/sbt-test/tasty-compat/add-overload/a/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package a

object A {

class Buf[A] {
def append(a: A): this.type = this
}

}
7 changes: 7 additions & 0 deletions sbt-dotty/sbt-test/tasty-compat/add-overload/b/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import a.*

object B {
val foo = new A.Buf[Seq[Double]]
val bar = Seq.empty[Double]
foo.append(bar)
}
23 changes: 23 additions & 0 deletions sbt-dotty/sbt-test/tasty-compat/add-overload/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
lazy val a = project.in(file("a"))
.settings(
Compile / classDirectory := (ThisBuild / baseDirectory).value / "b-input"
)

lazy val b = project.in(file("b"))
.settings(
Compile / unmanagedClasspath += (ThisBuild / baseDirectory).value / "b-input",
Compile / classDirectory := (ThisBuild / baseDirectory).value / "c-input"
)

lazy val `a-changes` = project.in(file("a-changes"))
.settings(
Compile / classDirectory := (ThisBuild / baseDirectory).value / "c-input"
)

lazy val c = project.in(file("."))
.settings(
scalacOptions ++= Seq("-from-tasty", "-Ycheck:readTasty"),
Compile / sources := Seq(new java.io.File("c-input/B.tasty")),
Compile / unmanagedClasspath += (ThisBuild / baseDirectory).value / "c-input",
Compile / classDirectory := (ThisBuild / baseDirectory).value / "c-output"
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import sbt._
import Keys._

object DottyInjectedPlugin extends AutoPlugin {
override def requires = plugins.JvmPlugin
override def trigger = allRequirements

override val projectSettings = Seq(
scalaVersion := sys.props("plugin.scalaVersion")
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version"))
8 changes: 8 additions & 0 deletions sbt-dotty/sbt-test/tasty-compat/add-overload/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# compile library A
> a/compile
# compile library B, from source, against A
> b/compile
# add a new overload to library A'
> a-changes/compile
# compile B, from tasty, against A', it should still compile: the overload does not conflict
> c/compile
15 changes: 15 additions & 0 deletions sbt-dotty/sbt-test/tasty-compat/add-override/a-changes/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package a

object A {

trait Box0[A] {
def append(a: A): this.type = this
}

trait BoxInt extends Box0[Int] {
override def append(a: Int): this.type = this
}

val box: BoxInt = new BoxInt {}

}
Loading