-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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 } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
tryWithImplicitOnQualifier(fun1, originalProto).getOrElse( | ||
if (proto eq originalProto) fail | ||
else tryWithImplicitOnQualifier(fun1, proto).getOrElse(fail)) | ||
} | ||
case _ => | ||
handleUnexpectedFunType(tree, fun1) | ||
} | ||
|
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 | ||
|
@@ -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 | ||
|
||
|
@@ -53,3 +53,26 @@ object Main { | |
() | ||
} | ||
} | ||
|
||
object TestWithAutoTuling { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is unfinished There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
|
There was a problem hiding this comment.
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