search

scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.83k stars 1.05k forks source link

Inline summon generated by macro doesn't consider expanded macro scope #12359

Open japgolly opened 3 years ago

japgolly commented 3 years ago

Compiler version

3.0.0-RC3

Attempt 1: summonInline

Minimized code

a:scala:

import scala.quoted.*

case class F[A]()

object F:
  implicit def option[A: F]: F[Option[A]] = F()

  inline def test[A]: F[Option[A]] =
    ${ _test[A] }

  def _test[A](using Quotes, Type[A]): Expr[F[Option[A]]] =
    import quotes.reflect.*

    // Dynamically created `implicit val`
    val faSym  = Symbol.newVal(Symbol.spliceOwner, "fa", TypeRepr.of[F[A]], Flags.Implicit, Symbol.noSymbol)
    val faBody = '{ F[Int]() }.asTerm
    val faDef  = ValDef(faSym, Some(faBody))

    val summonInline = '{ scala.compiletime.summonInline[F[Option[A]]] }.asTerm
    val result = Block(List(faDef), summonInline).asExprOf[F[Option[A]]]

    println(s"\n${result.show}\n")
    result

b:scala:

object Test:

  val ko = F.test[Int]

  // Copy-paste of the .show output of the generated Expr
  val ok =
    implicit val fa: F[scala.Int] = F.apply[scala.Int]()
    scala.compiletime.package$package.summonInline[F[scala.Option[scala.Int]]]

Output

{
  implicit val fa: F[scala.Int] = F.apply[scala.Int]()
  scala.compiletime.package$package.summonInline[F[scala.Option[scala.Int]]]
}

[error] -- Error: b.scala:3:17 ----------------
[error] 3 |  val ko = F.test[Int]
[error]   |           ^^^^^^^^^^^
[error]   |           cannot reduce summonFrom with
[error]   |            patterns :  case given t @ _:F[Option[Int]]
[error]   | This location contains code that was inlined from package.scala:140
[error]   | This location contains code that was inlined from a.scala:19
[error]   | This location contains code that was inlined from a.scala:19
[error] one error found

Attempt 2: inline call to Implicits.search

Minimized code

a:scala:

import scala.quoted.*

object Util:
  def summonLater[A: Type](using Quotes): Expr[A] =
    '{ inlineSummon[A] }

  inline def inlineSummon[A]: A =
    ${ _inlineSummon[A] }

  def _inlineSummon[A: Type](using Quotes): Expr[A] =
    summonOrError[A]

  def summonOrError[A](using Type[A])(using Quotes): Expr[A] =
    import quotes.reflect.*
    Implicits.search(TypeRepr.of[A]) match
      case iss: ImplicitSearchSuccess => iss.tree.asExpr.asInstanceOf[Expr[A]]
      case isf: ImplicitSearchFailure => report.throwError(isf.explanation)

case class F[A]()

object F:
  implicit def option[A: F]: F[Option[A]] = F()

  inline def test[A]: F[Option[A]] =
    ${ _test[A] }

  def _test[A](using Quotes, Type[A]): Expr[F[Option[A]]] =
    import quotes.reflect.*

    // Dynamically created `implicit val`
    val faSym  = Symbol.newVal(Symbol.spliceOwner, "fa", TypeRepr.of[F[A]], Flags.Implicit, Symbol.noSymbol)
    val faBody = '{ F[Int]() }.asTerm
    val faDef  = ValDef(faSym, Some(faBody))

    val summonInline = Util.summonLater[F[Option[A]]].asTerm
    val result = Block(List(faDef), summonInline).asExprOf[F[Option[A]]]

    println(s"\n${result.show}\n")
    result

b:scala:

object Test:

  val ko = F.test[Int]

  // Copy-paste of the .show output of the generated Expr
  val ok =
    implicit val fa: F[scala.Int] = F.apply[scala.Int]()
    Util.inlineSummon[F[scala.Option[scala.Int]]]

Output

{
  implicit val fa: F[scala.Int] = F.apply[scala.Int]()
  Util.inlineSummon[F[scala.Option[scala.Int]]]
}

[error] -- Error: b.scala:3:17 ----------------
[error] 3 |  val ko = F.test[Int]
[error]   |           ^^^^^^^^^^^
[error]   |           no implicit values were found that match type F[Int]
[error]   | This location contains code that was inlined from b.scala:3
[error]   | This location contains code that was inlined from a.scala:5
[error]   | This location contains code that was inlined from a.scala:5
[error] one error found

Expectation

Neither macro compiles, however in both cases if I copy-paste the output of .show, the code compiles ok.

Implicit search should consider sibling implicits in local scope, just like normal code.

som-snytt commented 3 years ago

In attempt 1, if it is inline def summonInline:

exception occurred while compiling a.scala
java.lang.AssertionError: assertion failed: unresolved symbols: type A (line 12) #9326 when pickling a.scala while compiling a.scala
Exception in thread "main" java.lang.AssertionError: assertion failed: unresolved symbols: type A (line 12) #9326 when pickling a.scala
        at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
        at dotty.tools.dotc.core.tasty.TreePickler.pickle(TreePickler.scala:788)
        at dotty.tools.dotc.quoted.PickledQuotes$.pickle(PickledQuotes.scala:168)

If it is transparent inline def summonInline then it just works.

Then I quit while I was ahead.

japgolly commented 3 years ago

@som-snytt If I had your consent, I would kiss you!! :heart: Thank you so much!

import scala.quoted.*

case class F[A](i: Int)

object F:
  implicit def option[A](implicit f: F[A]): F[Option[A]] = F(f.i * 10)

  inline def test[A]: F[Option[A]] =
    ${ _test[A] }

  def _test[A](using Quotes, Type[A]): Expr[F[Option[A]]] =
    import quotes.reflect.*
    transparent inline def summonInline = '{ scala.compiletime.summonInline[F[Option[A]]] }
    val result: Expr[F[Option[A]]] = '{
      implicit val fa: F[A] = F(3)
      $summonInline
    }
    // println(s"\n${result.show}\n")
    result
@main def run =
  println("Expecting to see F(30)")
  println(F.test[Int])

which does indeed print:

Expecting to see F(30)
F(30)
japgolly commented 3 years ago

Sorry @som-snytt but I need to take that potential kiss back. It doesn't actually work. It only appeared to work because it was statically able to derive an instance before macro-expansion time and only because this is a static example for minification purposes. I'll clarify with a new snippet....

japgolly commented 3 years ago

I've updated the examples in the main issue description. I minimised a little too much, in reality I'm working with dynamically-created implicits in scope.

som-snytt commented 3 years ago

Sorry to break your heart like that 💔 but I just started reading the docs.

I'm pretty sure this was the tag line for the movie "Highlander 4: The Inlining":

The summoning is delayed until the call has been fully inlined.
nicolasstucki commented 3 years ago

The correct way to summon this value is using Expr.summon. But still there is some bug.

- val summonInline = '{ scala.compiletime.summonInline[F[Option[A]]] }.asTerm
+ val summonInline = Expr.summon[F[Option[A]]].get.asTerm
nicolasstucki commented 3 years ago

Oh, there is no implicit F[scala.Int] at the expansion site. The following fixes the issue.

object Test:
+  implicit val fa: F[scala.Int] = F.apply[scala.Int]()
  val ko = F.test[Int]
nicolasstucki commented 3 years ago

The here is if the following summonInline can see the given definition that got inlined

inline def f() =
  given Int = 3
  compiletime.summonInline[Int]

def test() = 
  f()
  // given given_Int: Int = 3
  // compiletime.summonInline[Int](using ???)

The real question is if this would count as a re-elaboration of the code or not.

We designed the inline summoning assuming that we would find the implicit at call site. But this shows that the call site might also contain inlined given definitions.

@odersky, @sjrd WDYT?

nicolasstucki commented 3 years ago

Note that even if we can see the previously given definition we would not be able to see this one as f is inlined after g is expanded.

inline def f(inline thunk: Unit) =
  given Int = 3
  thunk

def g = summonInline[Int]

def test() = f { g }
nicolasstucki commented 3 years ago

Also, summonInline should behave the same way as Expr.summon which seems to indicate that we cannot rely on the given definitions that are inlined.

nicolasstucki commented 3 years ago

It seems that the current semantics are correct and we cannot change them without breaking something.

summonInline, Expr.summon and quotes.refelct.Implicits.search see the context of the expansion site at the time of exapansion.

nicolasstucki commented 3 years ago

Note that we can inject a given definition if we combine transparent and non-transparent definitions

transparent inline def f(inline thunk: Unit) =
  given Int = 3
  thunk

inline def g = compiletime.summonInline[Int]

def test() = f { g }
nicolasstucki commented 3 years ago

This means that we can, with some ugly hacks get some kind of given injection using 

import scala.quoted.*

case class F[A]()

object F:
  implicit def option[A: F]: F[Option[A]] = F()

  transparent inline def test[A]: F[Option[A]] =
    ${ _test[A] }

+  inline def delayedSummonInline[T] = scala.compiletime.summonInline[T]

  def _test[A](using Quotes, Type[A]): Expr[F[Option[A]]] =
    import quotes.reflect.*

    // Dynamically created `implicit val`
    val faSym  = Symbol.newVal(Symbol.spliceOwner, "fa", TypeRepr.of[F[A]], Flags.Implicit, Symbol.noSymbol)
    val faBody = '{ F[Int]() }.asTerm
    val faDef  = ValDef(faSym, Some(faBody))

-   val summonInline = '{ scala.compiletime.summonInline[F[Option[A]]] }.asTerm
+   val summonInline = '{ delayedSummonInline[F[Option[A]]] }.asTerm
    val result = Block(List(faDef), summonInline).asExprOf[F[Option[A]]]

    println(s"\n${result.show}\n")
    result

But this is a quite bad pattern as it is just forcing a new implicit search for something that we already know how to build. If we created fa then we should just use it where we need to find fa.

japgolly commented 3 years ago

Oh wow! Thanks so much @nicolasstucki ! You accidentally missed another small change that was required in that test needs to be transparent but yeah, I can confirm that those 3 small changes make all the difference! Seriously thanks so much!!

import scala.quoted.*

case class F[A]()

object F:
  implicit def option[A: F]: F[Option[A]] = F()

-  inline def test[A]: F[Option[A]] =
+  transparent inline def test[A]: F[Option[A]] =
    ${ _test[A] }

+  inline def delayedSummonInline[T] = scala.compiletime.summonInline[T]

  def _test[A](using Quotes, Type[A]): Expr[F[Option[A]]] =
    import quotes.reflect.*

    // Dynamically created `implicit val`
    val faSym  = Symbol.newVal(Symbol.spliceOwner, "fa", TypeRepr.of[F[A]], Flags.Implicit, Symbol.noSymbol)
    val faBody = '{ F[Int]() }.asTerm
    val faDef  = ValDef(faSym, Some(faBody))

-   val summonInline = '{ scala.compiletime.summonInline[F[Option[A]]] }.asTerm
+   val summonInline = '{ delayedSummonInline[F[Option[A]]] }.asTerm
    val result = Block(List(faDef), summonInline).asExprOf[F[Option[A]]]

    println(s"\n${result.show}\n")
    result
japgolly commented 3 years ago

But this is a quite bad pattern as it is just forcing a new implicit search for something that we already know how to build. If we created fa then we should just use it where we need to find fa.

If you know exactly what you need, then yes 100% this is a bad pattern, I agree with you. The problem is that there are cases where you not only do you know what what's required, but the effort to work out if something is required and then to try to recursively populate it with it's required implicits, especially when you're generating implicits that can (according to user-space rules and not the rules of the macro) have mutual/recursive implicit dependencies, trying to calculate all of that effort 100% statically is equivalent to just implementing the implicit search algorithm. Using this pattern I can just generate all of the expected implicits and if the user has done everything right on their side, then everything should work out at expansion time. If they haven't, then they'll get a nice implicit-not-found error after my macro expands which is perfect for both users, and me as the macro author.

goshacodes commented 2 months ago

@nicolasstucki Hi, I want to vote for this issue. In 3.3.3 Expr.summon still not sees givens put in a block statements from block expr. I can use summonInline instead, but it is quite annoying since I can't see what is actually summoned. This is what generated (same with summonInline).

{
  val given_JsonWriter_Option[String]: tethys.JsonWriter[scala.Option[scala.Predef.String]] = tethys.JsonWriter.optionalWriter[java.lang.String](tethys.JsonWriter.stringWriter)
  final class $anon() extends tethys.JsonObjectWriter[Person] {
    override def writeValues(value: Person, tokenWriter: tethys.writers.tokens.TokenWriter): scala.Unit = {
      tethys.derivation.ConfigurationMacroUtils$package.delayedSummonInline[tethys.JsonWriter[scala.Int]].write("id", value.id, tokenWriter)
      tethys.derivation.ConfigurationMacroUtils$package.delayedSummonInline[tethys.JsonWriter[java.lang.String]].write("name", value.name, tokenWriter)
      tethys.derivation.ConfigurationMacroUtils$package.delayedSummonInline[tethys.JsonWriter[scala.Option[scala.Predef.String]]].write("phone", value.phone, tokenWriter)
      tethys.derivation.ConfigurationMacroUtils$package.delayedSummonInline[tethys.JsonWriter[java.lang.String]].write("default", value.default, tokenWriter)
      ()
    }
  }

  (new $anon(): tethys.JsonObjectWriter[Person])
}

So I can't actually see, is given_JsonWriter_Option[String] used or it summons tethys.JsonWriter.optionalWriter[java.lang.String](tethys.JsonWriter.stringWriter) again

And I want to get this, but Expr.summon can't find given_JsonWriter_Option[String]:

{
  val given_JsonWriter_Option[String]: tethys.JsonWriter[scala.Option[scala.Predef.String]] = tethys.JsonWriter.optionalWriter[java.lang.String](tethys.JsonWriter.stringWriter)
  final class $anon() extends tethys.JsonObjectWriter[Person] {
    override def writeValues(value: Person, tokenWriter: tethys.writers.tokens.TokenWriter): scala.Unit = {
      tethys.JsonWriter.intWriter.write("id", value.id, tokenWriter)
      tethys.JsonWriter.stringWriter.write("name", value.name, tokenWriter)
      given_JsonWriter_Option[String].write("phone", value.phone, tokenWriter)
      tethys.JsonWriter.stringWriter.write("default", value.default, tokenWriter)
      ()
    }
  }

  (new $anon(): tethys.JsonObjectWriter[Person])
}
nicolasstucki commented 2 months ago

Hi @goshacodes, I am not actively working on Scala anymore. @jchyb took over this part of the language. Please follow up with him.