Implement inline override

[odersky]

The idea is to support inline override methods that can be both inlined and called dynamically. The old restriction that an inline method can only override concrete methods will go away. The implementation idea is as follows:

1. A definition

   inline override def f(x: T) = body
   

   is desugared to two methods:

   inline override def f(x: T) = body
   private def f$retainedBody(x: T) = f(x)
   

2. At erasure, assume the two methods look like this:

   inline override def f(x: T) = body1
   private def f$retainedBody(x: T) = body2
   

   They are then rewritten to

    override def f(x: T) = body2
   

Here, the owner of body2 is changed to f and all references to parameters of f$retainedBody are
changed to references of corresponding parameters in f.

An open question is whether we should go further and apply this scheme also if there is no override present. This could be useful e.g. for being able to inline unapply methods, since unapply methods have to be kept around at runtime as well. A possible syntax for this would be a soft modifier retained (the opposite of erased). I.e

inline retained unapply(x: T) = ...

This would give an inline unapply that persists after erasure, just like the f above.

[smarter]

since unapply methods have to be kept around at runtime as well.

Calls to unapply are generated by the typer, so when would we need them to be kept at runtime? In fact, #8542 adds support for inline def unapply

[nicolasstucki]

Note that this could also allow us to support the implementation of an abstract method with an inline method

trait Hello:
  def hello: String

object App extends Hello:
  inline def hello: String = "hi"

[odersky]

I am not sure whether we need retained inline unapplys and whether they have to be user-level. @nicolasstucki do you have a typical usecase?

When trying to implement this PR, I noted a potential problem. It's in typeclass-derivation2b.scala, lines 52ff:

    inline override def alternative(inline n: Int) <: GenericProduct[_ <: Lst[T]] =
      inline n match {
        case 0 => Cons.GenericCons[T]
        case 1 => Nil.GenericNil

If we translate that with the described scheme, we get:

53 |      inline n match {
   |             ^
   |             cannot reduce inline match with
   |              scrutinee:  {
   |               n
   |             } : (n : Int)
   |              patterns :  case 0
   |                          case 1

The problem is that the retained version of alternative does not have parameters that are constant, so the inline match fails.

I see two feasible ways around this

• Make inline override methods that have inline parameters not retained, i.e. use the old scheme for them.
  This works, but feels very subtle and implicit.

• Invent some more explicit scheme for inline methods to simply shadow other methods, but not override them at runtime. One such possibility would be to bring back erased. E.g. we could reformulate alternative like this:

    inline erased override def alternative(inline n: Int) <: GenericProduct[_ <: Lst[T]] = ...

An erased override is not an override at runtime, only at compile time. So no runtime instance of alternative needs to be created.

Opinions?

[sjrd]

@odersky The problematic example has a very specific property: the overridden method is never actually correct to begin with. It has = ??? as its rhs, which turns out fine because it's never called at runtime. It should be abstract to begin with, and would certainly be anyway if that had been allowed like we suggest.

Moreover, the example clearly shows that it can only work if that method is overridden in the "virtual" dispatch when it is called. It's just that this always happens statically. Again, this highlights that the true overriding semantics is what this example relies on.

What we need is therefore not a way to bypass overriding semantics with an erased at overriding site. Instead we need a way to mark a method as available for a call, but at compile time it must be checked that it is statically overridden at every call site. That would be the meaning of an abstract inline def, from what I can see.

I would therefore like to declare GenericSum.alternative as an abstract inline def, and GenericLst.alternative as a normal concrete inline def (which would not get a runtime bridge because it only implements an abstract inline method; not because itself is marked as a semantically wrong inline erased thing).

[smarter]

I see two feasible ways around this

I'm not a fan of either way, seems like too much complexity for too little gains. I think it's OK to have stricter requirements for the bodies of inline override def than inline def.

[nicolasstucki]

I am not sure if the inline override def unapply is needed. I will investigate further.

The code for inline override def alternative does not look problematic as one should just change the implementation to

 inline override def alternative(inline n: Int) <: GenericProduct[_ <: Lst[T]] =
     n match { // remove the inline here
        case 0 => Cons.GenericCons[T]
        case 1 => Nil.GenericNil

This means that if the code is not inlined, the match will be retained. If the code is inlined, it will reduce the match.

Alternatively, we could say the inline match does not check the body of the override inline.

Another alternative is to write the code as follows

 inline override def alternative(inline n: Int) <: GenericProduct[_ <: Lst[T]] =
     inline n match
        case 0 => Cons.GenericCons[T]
        case 1 => Nil.GenericNil
        case _ =>
            // non-specialized implementation (not necessarily the same as the inlined code)
            n match
              case 0 => Cons.GenericCons[T]
              case 1 => Nil.GenericNil

[odersky]

@sjrd Good analysis. I think abstract inline could be the way to go here. I'll try that next.

[soronpo]

Will this have any effect over the ability to combine inline def with opaque type?
lampepfl/dotty-feature-requests#82

[sjrd]

@soronpo No it won't, neither positively nor negatively.

[sjrd]

Hum, I don't think we should allow concrete inline methods to be overridden, even by other inline methods. This can break the semantics of polymorphic dispatch, because we might not flag a call to a method of the superclass on an instance that, at runtime, will be of a subclass.

It works with abstract inline methods because attempts at calling those will be flagged if they cannot be resolved to an overriding concrete inline method.

[odersky]

I tried with the last commit but it breaks scalatest. We have two inline versions of assert; one in Assertions the other in Diagrams. What scalatest does does not look unreasonable. It's a pity that this means we get static behavior which is not backed by runtime behavior. But it does not look easy to find workarounds in existing software.

So I think we will have to revert this.

[smarter]

(for reference the definitions in scalatest are https://github.com/scalatest/scalatest/blob/4e1e38e2372ddada9037d442aa7efed1418119d9/scalatest.dotty/src/main/scala/org/scalatest/Assertions.scala#L470 and https://github.com/scalatest/scalatest/blob/4e1e38e2372ddada9037d442aa7efed1418119d9/scalatest.dotty/src/main/scala/org/scalatest/diagrams/Diagrams.scala#L180).

I'm still strongly in favor of disallowing this. As @sjrd said, it just has the wrong semantics. Scalatest will have to be slightly refactored to avoid the override: put the code for the regular assert and diagram assert in the same macro, and pass a boolean to the macro to decide which branch to use, this boolean could come from a def in the trait that can be overridden in Diagrams to preserve the current API. This does not allow users to overrides assert themselves like the previous version could, but we can't do that without breaking semantics so that's not a bad thing.

[odersky]

I am not sure. What you propose is a breach of modularity. Assertions now has to know that there is an override in Diagrams. This might work in this concrete example, but I am not sure where else this pattern is used.

So, in the abstract it's better to disallow but in the concrete I personally do not want to deal with the breakage this causes. I'll revert for now and open an issue. If someone wants to take this up, fine.

[smarter]

What you propose is a breach of modularity.

inline defs inherently break modularity, better make that obvious than have surprising semantics IMO. But we can indeed discuss this separately.

[smarter]

Thinking about it, an abstract inline def assert in a base trait would probably suite scalacheck well, but it might require a bigger refactoring.

[nicolasstucki]

For scalatest the refactoring seems simple. We can have a single macro in Assertions that takes this as an argument. If this is statically known to be Diagrams we call DiagrammedAssertionsMacro.assert otherwise we call AssertionsMacro.assert. I will try it out.

[nicolasstucki]

Indeed it was simple #8601

[nicolasstucki]

The implementation of the retained methods logic LGTM.

We still need to decide on #8564. I strongly suggest making them final.
Also not sure about the usefulness of the abstract inline.

All these concerns can be handled in followup PRs.

[dwijnand]

This is missing from the documentation of Name above (line 40)