Make singleton operations covariant

[mbovel]

Currently, this does not compile:

def mult(x: Int, y: Int): x.type * y.type = (x * y).asInstanceOf
val y: x.type * x.type * x.type = mult(mult(x, x), x)
|                                 ^^^^^^^^^^^^^^^^^^^
|Found:    (?1 : (Test.x : Int) * (Test.x : Int)) * (Test.x : Int)
|Required: (Test.x : Int) * (Test.x : Int) * (Test.x : Int)
because (x: X) * Y is not a subtype of X * Y.

because (x: X) * Y is not a subtype of X * Y.

Making the compiletime operation types covariant in their arguments would enable this.

[soronpo]

The problem is that covariance and contravariance encourage the compiler to widen. I wonder if it has any unwanted side effect.

[mbovel]

I think covariance allows the compiler to widen when it is actually useful. In my experiments with singleton ops, the impossibility to compare operations containing skolems or term references is a big blocker.

We definitely need a way to preserve precise types and I am experimenting with different possibilities (such as an exact or precise type modifier as you suggested), but that seems orthogonal to this covariance problem to me. What do you think?

[soronpo]

I'll try this branch on my code-base to see if it has any unwanted effect. I'll reply when I'm done.

[MaximeKjaer]

This looks very reasonable to me. Seems intuitive that 1 + 2 should be a subtype of 1 + Int or Int + Int.

I don't feel comfortable marking the PR as approved myself, because I don't actively contribute to the repo, and I might not have the full picture. But I do otherwise approve of the change!

Unless we have a specific counter-example of how covariance might result in unexpected widenings and break things right now, then I think that as long as tests pass, we can merge this.

One thing you could try to do to make sure that this works is to compile the community build, or maybe just a few projects that make heavy use of singleton ops (e.g. tf-dotty -- you can ping me if you need any help with this).

[soronpo]

I checked this branch effect on my library, and nothing "bad" happened.
The implementation LGTM.

[mbovel]

One thing you could try to do to make sure that this works is to compile the community build, or maybe just a few projects that make heavy use of singleton ops (e.g. tf-dotty -- you can ping me if you need any help with this).

tf-dotty is included in the CI tests as part of community_build_c 😄 onnx-scala is included as part of community_build_c. It uses io.kjaer.compiletime like tf-dotty so it should be a good test already. I'll test with tf-dotty as well.

[som-snytt]

The appropriate approval pun would be +1!

[mbovel]

I'll test with tf-dotty as well.

@MaximeKjaer IndicesSuite and ShapeSuite pass successfully with the changes in this PR ✔️

[MaximeKjaer]

Should we consider leaving this one invariant? IsConst[1] is true, but IsConst[Int] is false