Partial fix for inlining with opaque types

[odersky]

When compiling shapeless, we saw an error message

[error] -- [E007] Type Mismatch Error: /home/miles/projects/shapeless-ng/core/src/test/scala/shapeless/type-classes.scala:53:80
[error] 53 |  inline def derived[A] given (gen: K0.ProductGeneric[A]): Monoid[A] = monoidGen
[error]    |                                                                                ^
[error]    |Found:    shapeless.K0.ProductInstances[shapeless.Monoid, shapeless.ISB]
[error]    |Required: shapeless.K0.ProductInstances[shapeless.Monoid, shapeless.ISB]

With -Yprint-debug that became clearer:

|Found:    K0.this.ProductInstances[[A] => shapeless.this.Monoid[A], shapeless.this.ISB]
|Required: shapeless.this.K0.ProductInstances[[A] => shapeless.this.Monoid[A],

The problem is that one of the two types thinks it is inside K0 and dealiases to ErasedProductInstances while the other think it is outside and gets stuck. The weird thing
is that a type claiming to be inside an opaque scope shows up outside this scope.

This commit ensures that the type K0.this.ProductInstances does not show up outside of K0.
There were two places in the inliner where M.this for static M was kept as is. Normally
that's OK, but it does matter if M contains opaque types.

This is a partial fix only. Compared to the original shapeless code, test pos/shapeless.scala also
contains two casts (on lines 65 and 68), which would lead to inlining failures if missing.

Unfortunately, this second problem looks impossible to fix in general. We cannot simply move
code compiled inside an opaque alias scope to outside this scope. The compiler can try to insert
some casts to make it work, but I see no general proven way to do so, because opaque types are
explained through type abstraction instead of through implicit conversions.

[smarter]

There were two places in the inliner where M.this for static M was kept as is. Normally
that's OK, but it does matter if M contains opaque types.

Could we add a check to TreeChecker to make sure there's no other cases where this happens ?

[odersky]

Could we add a check to TreeChecker to make sure there's no other cases where this happens ?

It does happen for static this, and this is OK, as long as there are no opaque types. Right now, I am not sure we can combine opaque and inline. So maybe this whole PR will go away in the end, to be replaced by a check that an object does not have both opaque types and inline methods.

[odersky]

What happened to the build? Everything is sequential now where it used to be parallel.

[smarter]

Just merged #6845 which should restore parallelism on the CI

[smarter]

You may need to rebase on master to get parallelism working on the CI

[milessabin]

@odersky those additional casts are acceptable from my PoV: they're part and parcel of moving between the well-typed veneer that users see and the erased part which only I see ... I can live with that.

[milessabin]

Right now, I am not sure we can combine opaque and inline. So maybe this whole PR will go away in the end, to be replaced by a check that an object does not have both opaque types and inline methods.

I think this would be very unfortunate. Many of the people calling for opaque types for performance reasons want to be able to combine them with inline methods.

[milessabin]

LGTM.

[odersky]

@milessabin There are ways to get around the restriction. For instance, you could put the opaque types in a local object with wrap and unwrap methods. E.g. like this

  object KO {
    object Opaques {
      opaque type ProductInstances[F[_], T] = ErasedProductInstances[F[T]]
      def wrap[F[_], T](x: ErasedProductInstances[F[T]]): ProductInstances[F, T] = x
      def unwrap[F[_], T](x: ProductInstances[F, T]): ErasedProductInstances[F[T]] = x
   }
   type ProductInstances[F[_], T] = Opaques.ProductInstances[F, T]
   import Opaques.{wrap, unwrap}

   ... inline methods using `wrap`, `unwrap`.

That works, since the object containing opaque types is not the object containing inline methods. It sure is a bit clunky, but it's workable. If the alternative is inlined code failing in mysterious ways on expansion, I prefer this solution.

[milessabin]

@odersky unless I'm missing something, wrap and unwrap are identity functions which ideally you'd want to be inline?

[smarter]

I wouldn't worry too much about the compiler inlining identity functions, the JVM should have no trouble doing it on its own.

[odersky]

I wouldn't worry too much about the compiler inlining identity functions, the JVM should have no trouble doing it on its own.

Right. We generate a ton of small functions for accessors, all in the expectation that the JVM will inline them anyway. Identity functions are even simpler than that.