amnaredo / test

0 stars 0 forks source link

Macro implicits don't work outside package #86

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

Hey. Check this out: https://github.com/japgolly/upickle/commit/9a976af73ed46eeb7364829e457a973719ed9a7b

Eg2 compiles but Eg1 doesn't.

[error] upickle/jvm/shared/test/scala/upickle/blah/blah.scala:7: uPickle does not know how to read [upickle.elsewhere.YYY]s; define an implicit Reader[upickle.elsewhere.YYY] to teach it how
[error]   def blah(j: String): YYY = read[YYY](j)
[error]                                       ^
[warn] one warning found
[error] one error found
[error] upickle/js/shared/test/scala/upickle/blah/blah.scala:7: uPickle does not know how to read [upickle.elsewhere.YYY]s; define an implicit Reader[upickle.elsewhere.YYY] to teach it how
[error]   def blah(j: String): YYY = read[YYY](j)
[error]                                       ^
[warn] one warning found
[error] one error found

ID: 31 Original Author: japgolly

amnaredo commented 3 years ago

Looks like knownDirectSubclasses is not doing the right thing:

println("clsSymbol " + clsSymbol)
// clsSymbol trait YYY
println("clsSymbol.knownDirectSubclasses " + clsSymbol.knownDirectSubclasses)
// clsSymbol.knownDirectSubclasses Set()

Looks like we're bumping into https://issues.scala-lang.org/browse/SI-7046. It seems like calling clsSymbol.typeSignature before asking for the knownDirectSubclasses doesn't do anything, so we must be in the "workaround doesn't work" case. Fails in both 2.10.4 and 2.11.2.

One solution is to rename Eg1 to Eg3. That makes it compile and work great (lol)

I wonder of @xeno-by has any tips. That issue is still open, so there's still hope...

Original Author: lihaoyi

amnaredo commented 3 years ago

Ouch. Doesn't look like that's going to be fixed any time soon. :disappointed:

Ok now I'm on the hunt for reliable workarounds. Renaming Eg1 to Eg3 didn't work (lol were you joking about that?). I think I'll need to start writing util methods and combinators and creating Reader/Writers. Any other ideas?

Original Author: japgolly

amnaredo commented 3 years ago

It worked for me! Also, if I commented out eg1 and let eg2 compile first, eg1 would compile successfully happily ever after.

I honestly don't have any ideas on how to force a dependency here. Presumably there has to be some way of hinting to the compiler the dependency, but I got nothing... On Sep 29, 2014 3:10 PM, "David Barri" notifications@github.com wrote:

Ouch. Doesn't look like that's going to be fixed any time soon. [image: :disappointed:]

Ok now I'm on the hunt for reliable workarounds. Renaming Eg1 to Eg3 didn't work (lol were you joking about that?). I think I'll need to start writing util methods and combinators and creating Reader/Writers. Any other ideas?

— Reply to this email directly or view it on GitHub https://github.com/lihaoyi/upickle/issues/31#issuecomment-57238485.

Original Author: lihaoyi

amnaredo commented 3 years ago

Ah yeah I think that's due to incremental compilation. It consistently blows up on a clean compile for me.

Cool. I'll keep wrestling with it. If I discover any helpful tricks I'll share it here.

Original Author: japgolly

amnaredo commented 3 years ago

Can you provide any guidance on how to define the standard Reader/Writer? I've hit what looks to be a variant of this bug, related to my previous report, but separate -- in this case, Autowire is failing on the write side. In a nutshell, the relevant part of the API:

package querki.api
import models.Wikitext

trait ThingFunctions {
  def renderThing(thingId:String):Wikitext
}

This routes a given request like so (down in my server-side actors):

def write[Result: Writer](r: Result) = upickle.write(r)
def read[Result: Reader](p: String) = upickle.read[Result](p)

case ClientRequest(apiId, req, rc) => {
  // ...
  route[ThingFunctions](this)(req).foreach { result =>
    sender ! ClientResponse(result)                  
  }
}

This is failing with:

[error] ...\querki\scalajvm\app\querki\session\UserSpaceSession.scala:241: uPickle does not know how to write [models.Wikitext]s; define an implicit Writer[models.Wikitext] to teach it how
[error]                 route[ThingFunctions](this)(req).foreach { result =>

Based on the above issue report, I'm not 100% surprised, but I'm not quite sure how to work around it. The error implies that defining an implicit Writer should deal with it, and I'm willing to explore that -- but it's really unclear what the Writer should look like.

Wikitext is a sealed trait at the root of a mildly complex hierarchy; how do I define a Writer to do "the usual stuff" that would simply happen automatically if I was just saying write(myWikitextValue)? That is, how do I explicitly state the implicit val that is normally constructed automatically? (If this problem doesn't have an obvious fix in sight, and it sounds like it may not, this may be worth a FAQ.)

Original Author: jducoeur

amnaredo commented 3 years ago

@jducoeur take a look at the hand-made reader/writer for Either

https://github.com/lihaoyi/upickle/blob/master/shared/main/scala/upickle/Implicits.scala#L112-L129

That's basically what you have to do. It's kinda tedious, but that's what the macro was meant to fix =/ Writing it by hand is annoying but not the end of the world I guess...

Original Author: lihaoyi

amnaredo commented 3 years ago

Actually, I'm retracting and deleting my previous comment -- my problem may not be Autowire, it may be some other issue with upickle trying to understand how to write my class hierarchy. Researching that now...

Original Author: jducoeur

amnaredo commented 3 years ago

Grr -- yeah, wild goose chase. Sorry about the confusion. FYI, there seems to be an odd bug in the interaction of upickle and Eclipse, such that the "upickle doesn't know how to read/write" error shows up spuriously in certain circumstances in Eclipse, when actually compiling the code in Activator works just fine. This led me to draw some incorrect conclusions from my experiments.

Don't yet understand what causes these spurious errors to arise, but it seems to only be happening for this complex (and recursive) sealed trait hierarchy. If I manage to define the problem better, I may open an issue. (Certainly not the only spurious error I've found in the Eclipse IDE, though...)

Original Author: jducoeur

amnaredo commented 3 years ago

Btw I discovered a trick with this. Just force an implicit to be realised somewhere close to the data source, even if you don't use it. I think just having the macro evaluate at the right time puts scalac in the right state so that it has what it needs when you need to use the macro elsewhere in the codebase.

Original Author: japgolly

amnaredo commented 3 years ago

Yeah, that occurred to me shortly after I wrote my long (now-deleted) comment yesterday, and I experimentally tried it out. That also failed (that is, my explicit "write()" itself failed), which was what led me to the discovery that I'd been misunderstanding the problem from the beginning.

Good to know that the workaround works, though -- thanks...

On Sun, Oct 5, 2014 at 1:30 AM, David Barri notifications@github.com wrote:

Btw I discovered a trick with this. Just force an implicit to be realised somewhere close to the data source, even if you don't use it. I think just having the macro evaluate at the right time puts scalac in the right state so that it has what it needs when you need to use the macro elsewhere in the codebase.

— Reply to this email directly or view it on GitHub https://github.com/lihaoyi/upickle/issues/31#issuecomment-57927231.

Original Author: jducoeur

amnaredo commented 3 years ago

It's a shame that Implicit scope of type arguments doesn't work when you read/write a type A that has a parameter B that requires extra R/W definition... So that you would supply W[B] in A's companion object and it would work. However now W[B] just must be declared in the current scope for it to work or in B's companion object, but mostly B is a third party type so you don't have access to it's companion object.

So I default to either inheriting from Implicits trait or having these Type Class evidences in a package object.

It would be nice if one could supply all dependent R/W definitions into the A's companion object.

Original Author: l15k4

amnaredo commented 3 years ago

After I made changes in some unrelated areas of my code, this issue suddenly showed up.

@l15k4 Could you give an example how inheriting from Implicits fixed the problem? It contains unimplemented functions, so how would that work?

I also defined an implicit in my Protocol class which is in a different package. Then, I evaluate this implicit where upickle.write() is called, but to no avail.

Original Author: tindzk

amnaredo commented 3 years ago

@tindzk I can't figure out what I meant by "I default to either inheriting from Implicits trait". To my knowledge if you have custom R/W implicits in a different package than you need to directly import them to the scope where upickle.write() is called and it should work.

Original Author: l15k4

amnaredo commented 3 years ago

There must be something in Widok that collides with upickle. When I comment out import org.widok._, the errors vanish. My first idea was that this is related to the implicits that I define here, but even if I import these manually, I cannot reproduce this issue. I wish scalac would provide a bit more guidance. So a temporary solution for now is to encapsulate any upickle-related code in a separate file and ensure that no conflicting imports are made.

Original Author: tindzk

amnaredo commented 3 years ago

Guys this issue is still pretty nasty. I happened to remove an import statement of Reader & Writer evidence classes for pickling a custom datastructure, that Intellij idea considered unused when upgrading versions and later I spent 2 hours investigating why I'm getting really weird brand new scalac errors...Although I believe that recognizing this kind of problem must be pretty hard... Fingers crossed it won't happen again.

Original Author: l15k4

amnaredo commented 3 years ago

I knew that this issue made it so risky to choose upickle for a project and yet I did it again and here I am, chasing this issue for 3 hours... Even though my code is entirely within a single package and I know about a few other tricks that usually solved it like proper ordering and avoiding nested classes... And still god damn :

The referenced trait [[Aggregation]] does not have any sub-classes. This may happen due to a limitation of scalac (SI-7046) given that the trait is not in the same package. If this is the case, the hierarchy may be defined using integer constants.

The incremental compiler in Intellij is the biggest trap because it works works works until you do sbt clean and find it doesn't work god knows why after 2 hours of development...

Anyway, rules of thumb : 1) keep all components of model (trait, case classes) and serialization invocation within a single package 2) don't use nested classes 3) keep AST ordering of components in your scala files, ie. if some case class inherits from a sealed trait, have this trait declared above the case class, etc. etc. etc. 4) ???

Btw the @japgolly's trick with "implicit realization close to data source" doesn't work for me...

Original Author: l15k4

amnaredo commented 3 years ago

Moot with 0.5.1 Original Author: lihaoyi