-
Notifications
You must be signed in to change notification settings - Fork 44
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
cannot bind or create instance of scalaz reader (due to miss of Manifest) #42
Comments
What versions of scala, guice, and scala-guice are you using? |
scala 2.10.4, 4.0-beta5 for google guice, and 4.0.0-beta5 for scala-guice |
The underlying issue is that Here's a shorter example that does not use scalaz or guice: class A[T[_]]
manifest[A[List]] // No Manifest available This leads me to this stackoverflow on the issue: http://stackoverflow.com/questions/26010800/why-does-getting-a-manifest-of-a-trait-with-a-higher-kinded-type-does-not-work It seems like we won't be able to support this until we move to TypeTags. I'm going to try to squeeze it in as soon as I can. |
That made a lot of sense. Looking forward to it. |
I've banged out the required changes to migrate from Manifest to TypeTag and they can be previewed here: There is a pretty big drawback to TypeTags, and that's that they are not thread safe in Scala 2.10. I've, personally, coded around this for scala-guice library. However, it is basically impossible to coordinate across other libraries that might also be using TypeTags. That might be a big negative to other users of scala 2.10. I still need to add a specific test case for your use, but I believe that this works for your use case. @tsuckow, I could use your input here. Should we push forward with this change and begin to "deprecate" Scala 2.10? |
Thanks a lot @nbauernfeind for working on |
You're welcome @vkostyukov. I wrote those tests (haven't pushed them to my branch yet) and found a couple of edge-cases that are wrong related to Arrays. Arrays are tricky since there isn't a real class that supports them. JVM has a flag on the class instance to say whether or not the type is an array. With this particular use case the type tag doesn't conform to |
I'll pull the sonatype stats and see what usage looks like. I would hate to maintain a separate branch, but that may be the best if we have to hack 2.10 or leave it broken. |
Looks like 2.10 is curbing off. But still hundreds of users and 2.11 only has about half as many. |
Guys found another case that is similar to this.... trait A { def a {} } type AB = A with B class Model extends AbstractModule with ScalaModule { Guice.createInjector(new Model()) // This fails at run time. Guice thinks binds are dups. |
@tsuckow I think that's probably fine. If someone has to care or runs into that problem we just direct them to the earlier versions of 2.10. Most of the time people don't upgrade to 2.11 because of other third party dependencies. Twitter is usually one of the last companies to upgrade and I see most of their popular projects have already upgraded. Let's just deprecate 2.10 and move on. @shengc I've filed a separate issue for your second example since I'd prefer to deal with them separately. I haven't had a chance to find out why or if TypeTags fix them, yet. #43 |
Also, it appears I didn't finish my changes related to Arrays. I'll add that to my todo list this weekend and send a proper pull-request. |
So 4.2.1 has a conversion of typeLiteral[T] to use TypeTag. I started looking at replacing occurrences of Manifest with TypeTag. I'll see about adding this as a test case and see what happens. I should also look into the compilation assertions, I would like to add some expected failures on some of the tickets with corner cases like this. But maybe with Typetags they will all be fixed. ¯_(ツ)_/¯ |
Now that 2.10 support is gone, it's worth noting that Dotty (soon to become Scala 3) will no longer synthesize Manifest instances, so there are two reasons to move to type tags. Is there still a branch around somewhere? |
Much has already been converted to type tags. Though it isn't as rainbows
and unicorns as I'd hoped.
|
import com.google.inject._
import net.codingwell.scalaguice._
import scalaz.Reader
class AModule extends AbstractModule with ScalaModule {
def configure() {
bind[Reader[Int, Int]].toInstance(Reader { (_: Int) * 2 })
}
}
gives me error: "Manifest of Reader[Int, Int] is not found"
so I have to resort to the legacy way of creating guice module
class AnotherModule extends AbstractModule {
def configure() {
bind(classOf[Reader[Int, Int]]).toInstance(Reader { (_: Int) * 2 })
}
}
but then when I tried to create instance using scalaguice, got the same error again
val injector = Guice.createInjector(new AnotherModule())
injector.instance[Reader[Int, Int]] // same error as above
The text was updated successfully, but these errors were encountered: