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 1 commit
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
6 changes: 1 addition & 5 deletions src/dotty/language.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@ 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
val f = "".contains("", (_: Int))
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that was an intentional change


}
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: 0 additions & 2 deletions tests/neg/autoTuplingTest.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import dotty.language.noAutoTupling

object autoTuplingNeg2 {

val x = Some(1, 2) // error: too many arguments for method apply: (x: A)Some[A]
Expand Down
27 changes: 25 additions & 2 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,7 +9,8 @@ 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

Expand Down Expand Up @@ -53,3 +53,26 @@ object Main {
()
}
}

object TestWithAutoTuling {
Copy link

Choose a reason for hiding this comment

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

You forgot a "p" here


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"`.
// None of these po
Copy link
Member

Choose a reason for hiding this comment

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

This comment is unfinished

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think it came from scalac that way, but I might be wrong.

}