Skip to content

Refinements to auto-tupling #1459

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
Aug 21, 2016
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
7 changes: 0 additions & 7 deletions src/dotty/language.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,6 @@ object language {

class Feature

/** Allow higher-kinded type syntax (not yet checked) */
val higherKinds = new Feature

/** Keep union types */
val keepUnions = new Feature

/** No auto tupling */
val noAutoTupling = new Feature

}
4 changes: 2 additions & 2 deletions src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
*/
def featureEnabled(owner: ClassSymbol, feature: TermName): Boolean = {
def toPrefix(sym: Symbol): String =
if (!sym.exists || (sym eq defn.LanguageModuleClass) || (sym eq defn.Scala2LanguageModuleRef)) ""
if (!sym.exists || (sym eq defn.LanguageModuleClass) || (sym eq defn.Scala2LanguageModuleClass)) ""
else toPrefix(sym.owner) + sym.name + "."
def featureName = toPrefix(owner) + feature
def hasImport(implicit ctx: Context): Boolean = {
Expand All @@ -512,7 +512,7 @@ trait TypeOps { this: Context => // TODO: Make standalone object.

/** Is auto-tupling enabled? */
def canAutoTuple =
!featureEnabled(defn.LanguageModuleClass, nme.noAutoTupling)
!featureEnabled(defn.Scala2LanguageModuleClass, nme.noAutoTupling)

def scala2Mode =
featureEnabled(defn.LanguageModuleClass, nme.Scala2)
Expand Down
32 changes: 21 additions & 11 deletions src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -541,17 +541,29 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>

def typedApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = {

/** Try same application with an implicit inserted around the qualifier of the function
* part. Return an optional value to indicate success.
*/
def tryWithImplicitOnQualifier(fun1: Tree, proto: FunProto)(implicit ctx: Context): Option[Tree] =
tryInsertImplicitOnQualifier(fun1, proto) flatMap { fun2 =>
tryEither { implicit ctx =>
Some(typedApply(
cpy.Apply(tree)(untpd.TypedSplice(fun2), proto.typedArgs map untpd.TypedSplice),
pt)): Option[Tree]
} { (_, _) => None }
}

def realApply(implicit ctx: Context): Tree = track("realApply") {
var proto = new FunProto(tree.args, IgnoredProto(pt), this)(argCtx(tree))
val fun1 = typedExpr(tree.fun, proto)
val originalProto = new FunProto(tree.args, IgnoredProto(pt), this)(argCtx(tree))
val fun1 = typedExpr(tree.fun, originalProto)

// Warning: The following line is dirty and fragile. We record that auto-tupling was demanded as
// Warning: The following lines are dirty and fragile. We record that auto-tupling was demanded as
// a side effect in adapt. If it was, we assume the tupled proto-type in the rest of the application.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this comment be updated? It says "we assume the tupled proto-type in the rest of the application" but we use both proto and originalProto below

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, I'll take care of this in #1460.

// This crucially relies on he fact that `proto` is used only in a single call of `adapt`,
// otherwise we would get possible cross-talk between different `adapt` calls using the same
// prototype. A cleaner alternative would be to return a modified prototype from `adapt` together with
// a modified tree but this would be more convoluted and less efficient.
if (proto.isTupled) proto = proto.tupled
val proto = if (originalProto.isTupled) originalProto.tupled else originalProto

// If some of the application's arguments are function literals without explicitly declared
// parameter types, relate the normalized result type of the application with the
Expand Down Expand Up @@ -580,13 +592,11 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
val result = app.result
convertNewGenericArray(ConstFold(result))
} { (failedVal, failedState) =>
val fun2 = tryInsertImplicitOnQualifier(fun1, proto)
if (fun1 eq fun2) {
failedState.commit()
failedVal
} else typedApply(
cpy.Apply(tree)(untpd.TypedSplice(fun2), proto.typedArgs map untpd.TypedSplice), pt)
}
def fail = { failedState.commit(); failedVal }
Copy link
Member

Choose a reason for hiding this comment

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

Should be def fail() since it's not pure

tryWithImplicitOnQualifier(fun1, originalProto).getOrElse(
if (proto eq originalProto) fail
else tryWithImplicitOnQualifier(fun1, proto).getOrElse(fail))
}
case _ =>
handleUnexpectedFunType(tree, fun1)
}
Expand Down
19 changes: 10 additions & 9 deletions src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1445,26 +1445,27 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
val sel = typedSelect(untpd.Select(untpd.TypedSplice(tree), nme.apply), pt)
if (sel.tpe.isError) sel else adapt(sel, pt)
} { (failedTree, failedState) =>
val tree1 = tryInsertImplicitOnQualifier(tree, pt)
if (tree1 eq tree) fallBack(failedTree, failedState)
else adapt(tree1, pt)
}
tryInsertImplicitOnQualifier(tree, pt) match {
case Some(tree1) => adapt(tree1, pt)
case none => fallBack(failedTree, failedState)
}
}

/** If this tree is a select node `qual.name`, try to insert an implicit conversion
* `c` around `qual` so that `c(qual).name` conforms to `pt`. If that fails
* return `tree` itself.
*/
def tryInsertImplicitOnQualifier(tree: Tree, pt: Type)(implicit ctx: Context): Tree = ctx.traceIndented(i"try insert impl on qualifier $tree $pt") {
def tryInsertImplicitOnQualifier(tree: Tree, pt: Type)(implicit ctx: Context): Option[Tree] = ctx.traceIndented(i"try insert impl on qualifier $tree $pt") {
tree match {
case Select(qual, name) =>
val qualProto = SelectionProto(name, pt, NoViewsAllowed)
tryEither { implicit ctx =>
val qual1 = adaptInterpolated(qual, qualProto, EmptyTree)
if ((qual eq qual1) || ctx.reporter.hasErrors) tree
else typedSelect(cpy.Select(tree)(untpd.TypedSplice(qual1), name), pt)
} { (_, _) => tree
if ((qual eq qual1) || ctx.reporter.hasErrors) None
else Some(typedSelect(cpy.Select(tree)(untpd.TypedSplice(qual1), name), pt))
} { (_, _) => None
}
case _ => tree
case _ => None
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/neg/autoTuplingTest.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import dotty.language.noAutoTupling
import language.noAutoTupling

object autoTuplingNeg2 {

Expand Down
5 changes: 5 additions & 0 deletions tests/pending/pos/t1756.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ with expected type A with Poly[A]. And no solution is found.
To solve this, I added a fallback scheme similar to implicit arguments:
When an implicit view that adds a method matching given arguments and result
type fails, try again without the result type.

However, troubles are not yet over. We now get an oprhan poly param C when pickling
and, if typr printer and -Ylog:front is on, an infinite type of the form

mu x. Ring[LazyRef(x) & A]
*/
trait Ring[T <: Ring[T]] {
def +(that: T): T
Expand Down
25 changes: 0 additions & 25 deletions tests/pending/pos/t2660.scala

This file was deleted.

29 changes: 25 additions & 4 deletions tests/pending/pos/t2913.scala → tests/pos/t2913.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import language.noAutoTupling // try with on and off

class A {
def foo(a: Int) = 0
Expand All @@ -10,15 +9,16 @@ class RichA {
def foo() = 0
}

object Test {
object TestNoAutoTupling {
import language.noAutoTupling // try with on and off

implicit def AToRichA(a: A): RichA = new RichA

val a = new A
a.foo()
a.foo(1)

a.foo("") // Without implicits, a type error regarding invalid argument types is generated at `""`. This is
a.foo("") // Without implicits, a type error regarding invalid argument types is generated at `""`. This is
// the same position as an argument, so the 'second try' typing with an Implicit View is tried,
// and AToRichA(a).foo("") is found.
//
Expand All @@ -29,7 +29,6 @@ object Test {

a.foo("a", "b") // Without implicits, a type error regarding invalid arity is generated at `foo(<error>"", "")`.
// Typers#tryTypedApply:3274 only checks if the error is as the same position as `foo`, `"a"`, or `"b"`.
// None of these po
}

// t0851 is essentially the same:
Expand All @@ -53,3 +52,25 @@ object Main {
()
}
}

object TestWithAutoTupling {

implicit def AToRichA(a: A): RichA = new RichA

val a = new A
a.foo()
a.foo(1)

a.foo("") // Without implicits, a type error regarding invalid argument types is generated at `""`. This is
// the same position as an argument, so the 'second try' typing with an Implicit View is tried,
// and AToRichA(a).foo("") is found.
//
// My reading of the spec "7.3 Views" is that `a.foo` denotes a member of `a`, so the view should
// not be triggered.
//
// But perhaps the implementation was changed to solve See https://lampsvn.epfl.ch/trac/scala/ticket/1756

a.foo("a", "b") // Without implicits, a type error regarding invalid arity is generated at `foo(<error>"", "")`.
// Typers#tryTypedApply:3274 only checks if the error is as the same position as `foo`, `"a"`, or `"b"`.
}