Skip to content
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

Remove tvars introduced while testing normalizedCompatible #21466

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TyperState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,15 @@ class TyperState() {
def uncommittedAncestor: TyperState =
if (isCommitted && previous != null) previous.uncheckedNN.uncommittedAncestor else this

/** Commit typer state so that its information is copied into current typer state
/** Commit `this` typer state by copying information into the current typer state,
* where "current" means contextual, so meaning `ctx.typerState`.
* In addition (1) the owning state of undetermined or temporarily instantiated
* type variables changes from this typer state to the current one. (2) Variables
* that were temporarily instantiated in the current typer state are permanently
* instantiated instead.
*
* A note on merging: An interesting test case is isApplicableSafe.scala. It turns out that this
* requires a context merge using the new `&' operator. Sequence of actions:
* requires a context merge using the new `&` operator. Sequence of actions:
* 1) Typecheck argument in typerstate 1.
* 2) Cache argument.
* 3) Evolve same typer state (to typecheck other arguments, say)
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4124,6 +4124,7 @@ object Types extends TypeUtils {
protected def prefixString: String = companion.prefixString
}

// Actually.. not cached. MethodOrPoly are `UncachedGroundType`s.
final class CachedMethodType(paramNames: List[TermName])(paramInfosExp: MethodType => List[Type], resultTypeExp: MethodType => Type, val companion: MethodTypeCompanion)
extends MethodType(paramNames)(paramInfosExp, resultTypeExp)

Expand Down Expand Up @@ -4884,7 +4885,7 @@ object Types extends TypeUtils {
def origin: TypeParamRef = currentOrigin

/** Set origin to new parameter. Called if we merge two conflicting constraints.
* See OrderingConstraint#merge, OrderingConstraint#rename
* See OrderingConstraint#merge
*/
def setOrigin(p: TypeParamRef) = currentOrigin = p

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/printing/Formatting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ object Formatting {
* The idea is to do this for known cases that are useful and then fall back
* on regular syntax highlighting for the cases which are unhandled.
*
* Please not that if used in combination with `disambiguateTypes` the
* Please note that if used in combination with `disambiguateTypes` the
* correct `Context` for printing should also be passed when calling the
* method.
*
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
toTextCapturing(parent, refsText, "") ~ Str("R").provided(printDebug)
else toText(parent)
case tp: PreviousErrorType if ctx.settings.XprintTypes.value =>
"<error>" // do not print previously reported error message because they may try to print this error type again recuresevely
"<error>" // do not print previously reported error message because they may try to print this error type again recursively
case tp: ErrorType =>
s"<error ${tp.msg.message}>"
case tp: WildcardType =>
Expand Down
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ object ProtoTypes {
|constraint was: ${ctx.typerState.constraint}
|constraint now: ${newctx.typerState.constraint}""")
if result && (ctx.typerState.constraint ne newctx.typerState.constraint) then
// Remove all type lambdas and tvars introduced by testCompat
for tvar <- newctx.typerState.ownedVars do
inContext(newctx):
if !tvar.isInstantiated then
tvar.instantiate(fromBelow = false) // any direction

// commit any remaining changes in typer state
newctx.typerState.commit()
result
case _ => testCompat
Expand Down
17 changes: 9 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -360,20 +360,21 @@ trait TypeAssigner {
resultType1)
}
}
else if !args.hasSameLengthAs(paramNames) then
wrongNumberOfTypeArgs(fn.tpe, pt.typeParams, args, tree.srcPos)
else {
// Make sure arguments don't contain the type `pt` itself.
// make a copy of the argument if that's the case.
// Make a copy of `pt` if that's the case.
// This is done to compensate for the fact that normally every
// reference to a polytype would have to be a fresh copy of that type,
// but we want to avoid that because it would increase compilation cost.
// See pos/i6682a.scala for a test case where the defensive copying matters.
val ensureFresh = new TypeMap with CaptureSet.IdempotentCaptRefMap:
Copy link
Member

Choose a reason for hiding this comment

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

How are the changes in this file and Typer related to the change in ProtoTypes?

Copy link
Member Author

Choose a reason for hiding this comment

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

So the ones in Typer, i.e. when adapting by applying fresh type vars, is necessary for the recursive programs like i6682a (and in the comment). I then randomly grepped the reference i6682a and found the code in TypeAssigner, so I started expanding my Typer comment. But I noticed that the way its done in TypeAssigner is in cloning the lambda in the args. So I thought I could hit two birds with one stone by (1) do it consistently in both (making it easier to document) and (2) do it more efficiently in TypeAssigner by doing it once rather than for each argument.

def apply(tp: Type) = mapOver(
if tp eq pt then pt.newLikeThis(pt.paramNames, pt.paramInfos, pt.resType)
else tp)
val argTypes = args.tpes.mapConserve(ensureFresh)
if (argTypes.hasSameLengthAs(paramNames)) pt.instantiate(argTypes)
else wrongNumberOfTypeArgs(fn.tpe, pt.typeParams, args, tree.srcPos)
val needsFresh = new ExistsAccumulator(_ eq pt, StopAt.None, forceLazy = false)
val argTypes = args.tpes
val pt1 = if argTypes.exists(needsFresh(false, _)) then
pt.newLikeThis(pt.paramNames, pt.paramInfos, pt.resType)
else pt
pt1.instantiate(argTypes)
}
}
case err: ErrorType =>
Expand Down
24 changes: 23 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4754,7 +4754,29 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
var typeArgs = tree match
case Select(qual, nme.CONSTRUCTOR) => qual.tpe.widenDealias.argTypesLo.map(TypeTree(_))
case _ => Nil
if typeArgs.isEmpty then typeArgs = constrained(poly, tree)._2.map(_.wrapInTypeTree(tree))
if typeArgs.isEmpty then
val poly1 = tree match
case Select(qual, nme.apply) => qual.tpe.widen match
case defn.PolyFunctionOf(_) =>
// Given a poly function, like the one in i6682a:
// val v = [T] => (y:T) => (x:y.type) => 3
// It's possible to apply `v(v)` which extends to:
// v.apply[?T](v)
// Requiring the circular constraint `v <: ?T`,
// (because type parameter T occurs in v's type).
// So we create a fresh copy of the outer
// poly method type, so we now extend to:
// v.apply[?T'](v)
// Where `?T'` is a type var for a T' type parameter,
// leading to the non-circular `v <: ?T'` constraint.
//
// This also happens in `assignType(tree: untpd.TypeApply, ..)`
// to avoid any type arguments, containing the type lambda,
// being applied to the very same type lambda.
poly.newLikeThis(poly.paramNames, poly.paramInfos, poly.resType)
case _ => poly
case _ => poly
typeArgs = constrained(poly1, tree)._2.map(_.wrapInTypeTree(tree))
convertNewGenericArray(readapt(tree.appliedToTypeTrees(typeArgs)))
case wtp =>
val isStructuralCall = wtp.isValueType && isStructuralTermSelectOrApply(tree)
Expand Down
9 changes: 9 additions & 0 deletions tests/pos/interleaving-overload.cleanup.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// A minimisation of interleaving-overload
// Used while developing the tvar/tl clearnup in normalizedCompatible
class B[U]
class Test():
def fn[T]: [U] => Int => B[U] = [U] => (x: Int) => new B[U]()
def test(): Unit =
fn(1)
fn(2)
()
15 changes: 15 additions & 0 deletions tests/pos/zipped.min.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Justifies the need for TypeApply in tryInsertImplicitOnQualifier
// after failing ys.map[?B, C] using Zipped2's map
// we want to try ys.map[?B] using Coll's map, after toColl
final class Coll[+A]:
def map[B](f: A => B): Coll[B] = new Coll[B]
def lazyZip[B](that: Coll[B]): Zipped2[A, B] = new Zipped2[A, B](this, that)
final class Zipped2[+X, +Y](xs: Coll[X], ys: Coll[Y]):
def map[B, C](f: (X, Y) => B): Coll[C] = new Coll[C]
object Zipped2:
import scala.language.implicitConversions
implicit def toColl[X, Y](zipped2: Zipped2[X, Y]): Coll[(X, Y)] = new Coll[(X, Y)]
class Test:
def test(xs: Coll[Int]): Unit =
val ys = xs.lazyZip(xs)
ys.map((x: (Int, Int)) => x._1 + x._2)
Loading