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

Fix issue #216 #285

Merged
merged 1 commit into from
Feb 20, 2019
Merged

Conversation

zarthross
Copy link
Member

Removing the Lazy on the ResultMatcher (which is only barely recursive on situations of F[ResultMatcher] seems to fix the issue in #216 with circe Encoder causing a compilation issue.

Reproduction requires this dependency:
"io.circe" %% "circe-core" % "0.10.0"

Minimum reproduction.

import cats.effect.Effect
import io.circe.Encoder
import org.http4s._
import org.http4s.rho.RhoRoutes

trait Auto

class Test[F[+_]: Effect] extends RhoRoutes[F] {
  implicit def autoEncoder[A <: Auto : Encoder]: EntityEncoder[F, A] = ???

  GET / "get" |>> ({ () => // Didn't work.  Used Lazy[ResultMatcher]
    Ok(())
  })

  GET / "get2" |>> ({ // Worked.  Does not use Lazy[ResultMatcher]
    Ok(())
  })
}

I'm still wondering how I can make a test, I haven't isolated what it is about the circe Encoder that causes the root issue.

@zarthross
Copy link
Member Author

Lazy was introduced here #93 and here #110, so summoning the original reviewer @bryce-anderson and author @shengc for comments.

@danxmoran
Copy link
Contributor

@zarthross is there a snapshot artifact of this branch which I could pull into my build?

@zarthross
Copy link
Member Author

zarthross commented Feb 20, 2019 via email

@zarthross zarthross merged commit 744065e into http4s:master Feb 20, 2019
@zarthross zarthross deleted the issue/216-diverging-implicit branch February 20, 2019 20:55
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

Successfully merging this pull request may close these issues.

3 participants