Monocle icon indicating copy to clipboard operation
Monocle copied to clipboard

Compilation error with context bounds

Open NTPape opened this issue 3 years ago • 8 comments

Hello there!

Compilation of the following code will fail

import monocle.Lens
import monocle.macros.GenLens

trait Foo[T]
trait Bar[T]

case class A[T: Foo](s: A.S[T]) {
  private val lens: Lens[A.S[T], Bar[T]] = GenLens[A.S[T]](_.bar)
}

object A {
  case class S[T: Foo](bar: Bar[T])
}

in Scala 3 with the following error

[error] 8 |  private val lens: Lens[A.S[T], Bar[T]] = GenLens[A.S[T]](_.bar)
[error]   |                                           ^^^^^^^^^^^^^^^^^^^^^^
[error]   |Exception occurred while executing macro expansion.
[error]   |java.lang.Exception: Expected an expression. This is a partially applied Term. Try eta-expanding the term first.

and in Scala 2.13 with

[error] could not find implicit value for evidence parameter of type Foo[T]
[error]   private val lens: Lens[A.S[T], Bar[T]] = GenLens[A.S[T]](_.bar)
[error]                                                           ^

Interestingly, the last version that appears to work is 1.5.1-cats on Scala 2.11 (but not on Scala 2.12.)

NTPape avatar Feb 05 '22 15:02 NTPape

Thanks for the bug report. I am not sure when we will have time to investigate.

julien-truffaut avatar Feb 14 '22 10:02 julien-truffaut

I had a look into the issue in Scala 2.13 and as far as I can see the issue is located in the lens's modifyF method because it has a context bound for cats.Functor. Due to macro magic, the Functor evidence is named like it were the first evidence in the source file, i.e. like the evidence for Foo of the enclosing class A, so the evidence for Foo is shadowed and hence unavailable in modifyF's function body where it is needed for copy.

That means one workaround for the issue in Scala 2.13 is putting another class with a context bound above the case class to rename the evidences below it, like this:

import monocle.Lens
import monocle.macros.GenLens

trait Foo[T]
trait Bar[T]

private class IncrementAllEvidencesBelow[T: Foo]()

case class A[T: Foo](s: A.S[T]) {
  private val lens: Lens[A.S[T], Bar[T]] = GenLens[A.S[T]](_.bar)
}

object A {
  case class S[T: Foo](bar: Bar[T])
}

or by explicitly naming the implicit parameter:

import monocle.Lens
import monocle.macros.GenLens

trait Foo[T]
trait Bar[T]

case class A[T](s: A.S[T])(implicit foo: Foo[T]) {
  private val lens: Lens[A.S[T], Bar[T]] = GenLens[A.S[T]](_.bar)
}

object A {
  case class S[T: Foo](bar: Bar[T])
}

NTPape avatar Feb 19 '22 20:02 NTPape

Thanks for investigating. Do you think it will solve the problem if we change https://github.com/optics-dev/Monocle/blob/master/macro/src/main/scala-2.x/monocle/macros/internal/Macro.scala#L100

from:

override def modifyF[$F[_]: _root_.cats.Functor](f: $aTpe => $F[$bTpe])(s: $sTpe): $F[$tTpe] =
          _root_.cats.Functor[$F].map(f(s.$fieldMethod))(a => s.copy($field = a))

to:

override def modifyF[$F[_[](f: $aTpe => $F[$bTpe])(s: $sTpe)(implicit evFunctor: _root_.cats.Functor[F]): $F[$tTpe] =
          _root_.cats.Functor[$F].map(f(s.$fieldMethod))(a => s.copy($field = a))

julien-truffaut avatar Feb 20 '22 20:02 julien-truffaut

I suggest using a freshName too, to make a shadowing conflict less likely. I created a (naive) PR accordingly.

NTPape avatar Feb 20 '22 23:02 NTPape

Good call, I completely forgot about freshName. Do you want me to cut a release with this fix?

julien-truffaut avatar Feb 21 '22 11:02 julien-truffaut

Thanks for the quick merge! A release for this fix is not needed from my end. I'm also trying to look into the issue in Scala 3.

NTPape avatar Feb 22 '22 23:02 NTPape

This use case is a bit unusual @NTPape - usually typeclass bounds are declared at the methods where they are required, rather that at the class constructor level.

YAGNI is one reason, there's no physical reason why the A.S object construction needs to be held up for the lack of a Foo[T].

Another reason is that creating an instance of A.S[T: Foo](bar: Bar[T]) captures the Foo[T] instance and stores it as a field in the object. The created A.S object can wander far away from the initial scope into other scopes where A.Ss are being created with entirely different Foo[T] instances, which is hard to reason about and could cause subtle bugs when they interact.

Are you sure really need us to support this use case?

kenbot avatar Mar 03 '22 12:03 kenbot

Oh, I didn't notice that the discussion also moved back here from the PR.

The minimal example was somewhat artificially constructed to produce the implicit parameter name shadowing of the context bounds that was observed in Scala 2.13. It just so happened that it also fails in Scala 3 but for totally different reasons as we know now.

For Scala 3 a minimal example would just be a lens to a case field of a case class with any (positive) number of implicit parameters (including context bounds.)

These will both fail to compile:

import monocle.macros.GenLens

case class Test1(i: Int)(implicit j: Int)
GenLens[Test1](_.i)

trait Foo[T]

case class Test2[T: Foo](t: T)
GenLens[Test2[Int]](_.t)

Is that really too unusual?

NTPape avatar Mar 15 '22 17:03 NTPape