Permalink
Browse files

[nomaster] SI-5923 instantiates targs in deferred macro applications

Amazingly enough, the fix for the "macro not expanded" problem was super
easy. (And I remember spending a day or two trying to find a quick fix
somewhen around Scala Days 2012!)

The problem was in the implementation of the macro expansion trigger,
which was buried in a chain of if-elif-elif in `adapt`. This meant that macro
expansion was mutually exclusive with a lot of important adaptations, e.g.
with `instantiate`.

More precisely, if an expandee contains an undetparam, its expansion
should be delayed until all its undetparams are inferred and then retried
later. Sometimes such inference can only happen upon a call to instantiate
in one of the elif's coming after the macro expansion elif. However this
elif would never be called for expandees, because control flow would always
enter the macro expansion branch preceding the inference branch.

Therefore `macroExpand` now takes the matters in its own hands,
calling `instantiate` if the expansion has been delayed and we're not in
POLYmode (see a detailed explanation in a comment to `macroExpand`).

Consequences of this fix are vast. First of all, we can get rid of the
"type parameter must be specified" hack. Secondly and most importantly,
we can now remove the `materializeImplicit` method from Implicits and
rely on implicit macros to materialize tags for us. (This is a tricky
change, and I'll do it later after we merge as much of my pending work
as possible). Finally, we learn that the current scheme of interaction
between macros, type inference and implicits is, in principle, sound!

NOTE: This patch is a second take on fixing implicit macros, with the first
one being a backport from macro paradise merged into master in January 2013:
fe60284.

The original fix had an unfortunate error, as described on scala-internals:
https://groups.google.com/forum/#!msg/scala-internals/7pA9CiiD3u8, so I had
to refine the approach here.

This means that it's not possible to directly merge this commit into master,
so I'm marking it as [nomaster] and will submit a separate pull request
targetting master later on.
  • Loading branch information...
1 parent 0c6927b commit 90ac5c4e1350493f6f47c674572c2dcb97b2b9eb @xeno-by xeno-by committed Dec 31, 2012
@@ -69,11 +69,7 @@ abstract class Pickler extends SubComponent {
}
if (!t.isDef && t.hasSymbol && t.symbol.isTermMacro) {
- unit.error(t.pos, t.symbol.typeParams.length match {
- case 0 => "macro has not been expanded"
- case 1 => "this type parameter must be specified"
- case _ => "these type parameters must be specified"
- })
+ unit.error(t.pos, "macro has not been expanded")
return
}
}
@@ -388,6 +388,8 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces {
} finally {
popMacroContext()
}
+ case Delay(delayed) =>
+ typer.instantiate(delayed, EXPRmode, WildcardType)
case Fallback(fallback) =>
typer.typed1(fallback, EXPRmode, WildcardType)
case Other(result) =>
@@ -652,9 +654,9 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces {
private sealed abstract class MacroExpansionResult
private case class Success(expanded: Tree) extends MacroExpansionResult
+ private case class Delay(delayed: Tree) extends MacroExpansionResult
private case class Fallback(fallback: Tree) extends MacroExpansionResult { currentRun.seenMacroExpansionsFallingBack = true }
private case class Other(result: Tree) extends MacroExpansionResult
- private def Delay(expanded: Tree) = Other(expanded)
private def Skip(expanded: Tree) = Other(expanded)
private def Cancel(expandee: Tree) = Other(expandee)
private def Failure(expandee: Tree) = Other(expandee)
@@ -706,6 +708,54 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces {
} finally {
popMacroContext()
}
+ case Delay(delayed) =>
+ // =========== THE SITUATION ===========
+ //
+ // If we've been delayed (i.e. bailed out of the expansion because of undetermined type params present in the expandee),
+ // then there are two possible situations we're in:
+ //
+ // 1) We're in POLYmode, when the typer tests the waters wrt type inference
+ // (e.g. as in typedArgToPoly in doTypedApply).
+ //
+ // 2) We're out of POLYmode, which means that the typer is out of tricks to infer our type
+ // (e.g. if we're an argument to a function call, then this means that no previous argument lists
+ // can determine our type variables for us).
+ //
+ // Situation #1 is okay for us, since there's no pressure. In POLYmode we're just verifying that
+ // there's nothing outrageously wrong with our undetermined type params (from what I understand!).
+ //
+ // Situation #2 requires measures to be taken. If we're in it, then noone's going to help us infer
+ // the undetermined type params. Therefore we need to do something ourselves or otherwise this
+ // expandee will forever remaing not expanded (see SI-5692).
+ //
+ // A traditional way out of this conundrum is to call `instantiate` and let the inferencer
+ // try to find the way out. It works for simple cases, but sometimes, if the inferencer lacks
+ // information, it will be forced to approximate.
+ //
+ // =========== THE PROBLEM ===========
+ //
+ // Consider the following example (thanks, Miles!):
+ //
+ // // Iso represents an isomorphism between two datatypes:
+ // // 1) An arbitrary one (e.g. a random case class)
+ // // 2) A uniform representation for all datatypes (e.g. an HList)
+ // trait Iso[T, U] {
+ // def to(t : T) : U
+ // def from(u : U) : T
+ // }
+ // implicit def materializeIso[T, U]: Iso[T, U] = macro ???
+ //
+ // case class Foo(i: Int, s: String, b: Boolean)
+ // def foo[C, L](c: C)(implicit iso: Iso[C, L]): L = iso.to(c)
+ // foo(Foo(23, "foo", true))
+ //
+ // In the snippet above, even though we know that there's a fundep going from T to U
+ // (in a sense that a datatype's uniform representation is unambiguously determined by the datatype,
+ // e.g. for Foo it will be Int :: String :: Boolean :: HNil), there's no way to convey this information
+ // to the typechecker. Therefore the typechecker will infer Nothing for L, which is hardly what we want.
+ val shouldInstantiate = typer.context.undetparams.nonEmpty && !inPolyMode(mode)
+ if (shouldInstantiate) typer.instantiatePossiblyExpectingUnit(delayed, mode, pt)
+ else delayed
case Fallback(fallback) =>
typer.context.withImplicitsEnabled(typer.typed(fallback, EXPRmode, pt))
case Other(result) =>
@@ -1149,12 +1149,9 @@ trait Typers extends Modes with Adaptations with Tags {
adaptConstrPattern()
else if (shouldInsertApply(tree))
insertApply()
- else if (!context.undetparams.isEmpty && !inPolyMode(mode)) { // (9)
+ else if (context.undetparams.nonEmpty && !inPolyMode(mode)) { // (9)
assert(!inHKMode(mode), modeString(mode)) //@M
- if (inExprModeButNot(mode, FUNmode) && pt.typeSymbol == UnitClass)
- instantiateExpectingUnit(tree, mode)
- else
- instantiate(tree, mode, pt)
+ instantiatePossiblyExpectingUnit(tree, mode, pt)
} else if (tree.tpe <:< pt) {
tree
} else {
@@ -1302,6 +1299,13 @@ trait Typers extends Modes with Adaptations with Tags {
}
}
+ def instantiatePossiblyExpectingUnit(tree: Tree, mode: Int, pt: Type): Tree = {
+ if (inExprModeButNot(mode, FUNmode) && pt.typeSymbol == UnitClass)
+ instantiateExpectingUnit(tree, mode)
+ else
+ instantiate(tree, mode, pt)
+ }
+
private def isAdaptableWithView(qual: Tree) = {
val qtpe = qual.tpe.widen
( !isPastTyper
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
@@ -0,0 +1,3 @@
+C(Int)
+C(String)
+C(Nothing)
@@ -0,0 +1,14 @@
+import scala.reflect.macros.Context
+import language.experimental.macros
+
+case class C[T](t: String)
+object C {
+ implicit def foo[T]: C[T] = macro Macros.impl[T]
+}
+
+object Macros {
+ def impl[T: c.WeakTypeTag](c: Context) = {
+ import c.universe._
+ reify(C[T](c.literal(weakTypeOf[T].toString).splice))
+ }
+}
@@ -0,0 +1,5 @@
+object Test extends App {
+ println(implicitly[C[Int]])
+ println(implicitly[C[String]])
+ println(implicitly[C[Nothing]])
+}
@@ -0,0 +1,3 @@
+class [Ljava.lang.Object;
+class [Ljava.lang.Object;
+class [Ljava.lang.Object;
@@ -0,0 +1,7 @@
+object Test extends App {
+ import scala.collection.generic.CanBuildFrom
+ val cbf = implicitly[CanBuildFrom[Nothing, Nothing, Array[Nothing]]]
+ println(cbf().result.getClass)
+ println(new Array[Nothing](0).getClass)
+ println(Array[Nothing]().getClass)
+}

0 comments on commit 90ac5c4

Please sign in to comment.