Support Polymorphic Tags (Take 2)

[kitlangton]

Here is a work in progress, if I correctly understood @neko-kai's suggestions in #238, then I think this is much closer to what is actually needed. A lot more remains to be done, but I'm going to push what I have so far, just in case you (@neko-kai & @pshirshov) have any thoughts. Thanks for all the feedback :)

[pshirshov]

Yes, this approach seems to be lot better than the previous one. Though I won't say that your previous idea was totally wrong. And anyway, seems like you got a good exposure to the internals, which is a very good thing :)

[kitlangton]

Any thoughts on how I can create a Union/OrType constructor for LightTypeTag, similar to mkRefined?

      // TODO: This ain't right
       case orType: OrType => 
         val ltts0: List[Expr[LightTypeTag]] = flattenOr(orType).map(tt => mkLtt(tt))
         val ltts: Expr[List[LightTypeTag]] = Expr.ofList(ltts0)
         orType.asType match {
           case '[a] => 
             val struct = '{ Inspect.inspect[a] }
             '{ Tag.refinedTag(classOf[Any], $ltts, $struct, Map.empty)}
         }

[pshirshov]

I think you may need to add a method alike to LightTypeTag#combine directly into LTT, because you would want to merge all the component DBs.

[pshirshov]

You would need to construct a UnionReference in this line instead of calling .combine:

    LightTypeTag(ref.combine(argRefs), mergedBasesDB, mergedInheritanceDb)

Also you won't need to do argument application (appliedBases), you just need to construct a union reference and merge all the dbs into one.

[pshirshov]

Something alike to this:

  def asUnion(args: LightTypeTag*): LightTypeTag = {
    def mergedBasesDB = LightTypeTag.mergeIDBs(basesdb, args.iterator.map(_.basesdb))

    def mergedInheritanceDb = LightTypeTag.mergeIDBs(idb, args.iterator.map(_.idb))

    LightTypeTag(LightTypeTagRef.UnionReference(args.map(_.ref match {
      case reference: AbstractReference =>
        reference match {
          case Lambda(input, output) => ???
          case reference: AppliedReference => 
            reference
        }
    }).toSet), mergedBasesDB, mergedInheritanceDb)
  }

[pshirshov]

And yes, in case you meet a lambda there, probably you can't do anything sane with it, you just have to throw a useful exception.
In case, for some reason, you want to support unions with mixed kinds, (e.g. (T => F[T]) | String), you would need to change signature of UnionReference and allow any reference there, but that shouldn't be a sane usecase.

[kitlangton]

Woo! That seemed to work :)

[kitlangton]

Though, in testing: <:< did not seem to work:

object Testing extends App {
  trait Animal
  trait Mammal extends Animal

  def nestedTag[A: Tag, B: Tag] = TagMacro.tag[A | B]

  val animalStringTag = nestedTag[Animal, String]
  val mammalStringTag = nestedTag[Mammal, String]

  implicitly[(Mammal | String) <:< (Animal | String)] // compiles! :)

  println(mammalStringTag.tag <:< animalStringTag.tag) // returns false :(
}

Is <:< supported for Union types test?

[kitlangton]

Ah, doing it with a strong type also returns false:

object Testing extends App {
  println(Tag[Mammal | String].tag <:< Tag[Animal | String].tag)
}

So, at least I didn't break anything :D

[kitlangton]

So far, this seems to fix all of our open ZIO issues with dotty (testing against a snapshot). Is there anything else you think I should add to this to polish/clean/test? Can also extend it further in the future once (inevitably) more issues arise. Glad I sort of understand some of what's happening now (though not the DB/subtyping/pickling parts yet) 😄

[pshirshov]

Yes, from what I can remember <:< was supported for unions and I think I even added some tests for that. You may need to print inheritance dbs (there is a method for that), look at their content and then try to add some debug prints into inheritance calculator and see that's wrong there.

[pshirshov]

Is there anything else you think I should add to this to polish/clean/test?

Nope, looks good. I'm happy to merge it as is, though it would be very nice if you may track the union subtyping problem, anyway you are somewhere around already :)

[kitlangton]

Some sub typing works. If I change it to:

  val animalStringTag = nestedTag[Any, String]
  val mammalStringTag = nestedTag[Mammal, String]

  implicitly[(Mammal | String) <:< (Animal | String)]

  println(mammalStringTag.tag <:< animalStringTag.tag) // true
  println(Tag[Mammal | String].tag <:< Tag[Any | String].tag) // true

[kitlangton]

I'll check what tests exist currently and see if I can come up with anything :)

Thanks for all the help! That was fun.

[pshirshov]

Use .render() on refs for debug printouts.

[neko-kai]

@kitlangton

So far, this seems to fix all of our open ZIO issues with dotty (testing against a snapshot)

I think these tests should be copied here if possible.

There's also a large test suite for TagMacro Scala 2 in TagTest.scala file, you could try copying tests one-by-one from there either to SharedTagTest.scala file or to Scala 3's TagTest.scala file (when it's impossible to write something with the same syntax as Scala 2 - e.g. higher-kinded kind-projector syntax)

A large portion of them may now pass or at least compile, whenever a test now compiles but doesn't pass, you can flip it to a progression test.

^ Actually LightTypeTag#debug() method will print more internal info than .render()

[kitlangton]

Hey, I found it!

      case (s: UnionReference, t: UnionReference) =>
        // yeah, this shit is quadratic
        s.refs.exists {
          c =>
            t.refs.exists { // <-- this was t.refs.forall
              p =>
                ctx.isChild(c, p)
            }
        }

Does that make sense?

Wait... no no. That makes everything return true. But I'll explore.

Using debug, things look correct:

⚙️ HI: {Testing$::Mammal | String}
⚡️bases: 
 - String ->   
   * CharSequence
   * Serializable
   * Comparable[=String]
   * Object
 - Comparable[=String] ->   
   * Matchable
   * Any
   * Object
 - Testing$::Mammal ->   
   * Object
   * Testing$::Animal
 - Matchable ->   
   * Any
 - Serializable ->   
   * Any
 - Testing$::Animal ->   
   * Object
 - Object ->   
   * Matchable
   * Any
 - CharSequence ->   
   * Object
⚡️inheritance: 
 - Testing$::Mammal ->   
   * Matchable
   * Any
   * Object
   * Testing$::Animal
 - String ->   
   * Any
   * Matchable
   * Serializable
   * Comparable
   * Object
   * CharSequence
⚙️ end HI

[kitlangton]

This is it! 🥳

      case (s: UnionReference, t: UnionReference) =>
        // yeah, this shit is quadratic
        s.refs.forall { // <- was exists
          c =>
            t.refs.exists { // <- was forall
              p =>
                ctx.isChild(c, p)
            }
        }

[kitlangton]

I'll add my tests from ZIO and these other union tests (I don't think it was tested before—as far as I could see) 🤗

[pshirshov]

Yes, that's seems to be correct. Sorry, my bad. Probably I just copied the logic from intersection but didn't change it properly and didn't add any tests.

[pshirshov]

case Lambda(input, output) => ???

Please throw a meaningful exception there, as in other cases

[pshirshov]

We need to throw something readable here.

[kitlangton]

Ah yeah, did that. Copying tests from the Scala 2 tag tests to Shared (that I can) now, then I'll push :)

[kitlangton]

Addressed @pshirshov's comment, added a spec, and moved some passing tests to the shared spec (as @neko-kai suggested). I think that's all for now. Though I'm sure I'll be back :)

[pshirshov]

Great job, thanks!

[kitlangton]

Thank you both!

[kitlangton]

@neko-kai @pshirshov Would it be possible to get another version bump (at your leisure of course)? 😄🙏

[neko-kai]

@kitlangton It's up https://github.com/zio/izumi-reflect/releases/tag/v2.0.5