Skip to content
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

Type classes for parameter are not lazy #466

Open
steinybot opened this issue May 22, 2023 · 3 comments
Open

Type classes for parameter are not lazy #466

steinybot opened this issue May 22, 2023 · 3 comments

Comments

@steinybot
Copy link

steinybot commented May 22, 2023

I'm not sure why this is happening since they ought to be CallByNeed but it looks as though type classes are being summoned for case class parameters even if they are not used:

import magnolia1.{CaseClass, Derivation, SealedTrait}

trait Thing[A]

object Thing extends Derivation[Thing]:
  override def split[T](sealedTrait: SealedTrait[Thing, T]): Thing[T] = ???
  override def join[T](caseClass: CaseClass[Thing, T]): Thing[T] = ???

case class Foo(bar: String)

object Main extends App:
  Thing.derived[Foo]

Fails to compile with:

[error] -- Error: /Users/jason/src/bug-reports/src/main/scala/Main.scala:12:2 ----------
[error]  12 |  Thing.derived[Foo]
[error]     |  ^^^^^^^^^^^^^^^^^^
[error]     |  No given instance of type Thing[String] was found
[error]     |---------------------------------------------------------------------------
[error]     |Inline stack trace
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from impl.scala:86
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from impl.scala:86
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from impl.scala:86
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from impl.scala:86
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from impl.scala:86
[error]      ---------------------------------------------------------------------------
[error] one error found

Now obviously I wouldn't expect this to work since Derivation is not implemented. In my real use case I am only looking at certain fields within join and was surprised to get this error when I wasn't using the type class for a String parameter.

Reproduction: https://github.com/steinybot/bug-reports/tree/magnolia/typeclasses-too-eager

@joroKr21
Copy link
Contributor

That's pretty much a limitation of the library. It's assumed that the typeclasses are "regular" in the sense that when deriving for a case class, we need them for all fields and when deriving for a sealed trait we need them for all child classes. The CallByNeed is there really to support recursion rather than to avoid summoning a typeclass. E.g. for things like case class Nel[A](head: A, tail: Option[Nel[A]]). But the instances still need to be there for it to compile.

There is one workaround you could use to convert this from a compile time error to a runtime error. You could derive a wrapper typeclass instead:

trait OptionalThing[A] {
  def thing: Option[Thing[A]]
}

but then when you're using it, you would have to throw some kind of error when it's missing and you actually need it.

@steinybot
Copy link
Author

That makes sense now that I think about how it is implemented. You would need to be able to filter the params before that code gets inlined but that isn't possible. The only thing I can think of is if it were implemented as a macro instead then perhaps we could override a method that returns an Expr for the params.

@joroKr21
Copy link
Contributor

I think that could work theoretically in Scala 3 with inline - but I think it's a totally new design of the library. Perhaps worth a try but it would be a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants