Strip ascriptions originating from PrepareInlineable.wrapRHS or Typer.ensureNoLocalRefs during inlining

[mbovel]

Attempt to fix #24420 by removing ascriptions during inlining when they originate from PrepareInlineable.wrapRHS or Typer.ensureNoLocalRefs.

[sjrd]

Isn't that basically making all inlined def into transparent?

[mbovel]

Isn't that basically making all inline def behave like transparent?

Maybe, and that is probably not what we want.

So when should the result of calling a non-transparent inline method actually be visible?

[Comment extracted to #24431]

[mbovel]

Isn't that basically making all inlined def into transparent?

As discussed orally, no, assuming we put the stripping logic at the right place: in the Inliner, to make sure that we only do it during inlining.

[mbovel]

Edit: temporary fixed by 6ae39ef and 8b803e1.

Dropping ascriptions has strange effects, making YCheckPositions fail, and probably more.

Consider tests/pos/i21320b.scala, shortened to:

import scala.deriving.*
import scala.compiletime.*

trait ConfigMonoid[T]:
  def zero: T
  def orElse(main: T, defaults: T): T

object ConfigMonoid:
  given option: [T] => ConfigMonoid[Option[T]] = ???

  inline def zeroTuple[C <: Tuple]: Tuple =
    inline erasedValue[C] match
      case _: EmptyTuple => EmptyTuple
      case _: (t *: ts) =>
        summonInline[ConfigMonoid[t]].zero *: zeroTuple[ts]

  inline def valueTuple[C <: Tuple, T](index: Int, main: T, defaults: T): Tuple =
    inline erasedValue[C] match
      case _: EmptyTuple => EmptyTuple
      case _: (t *: ts) =>
        def get(v: T) = v.asInstanceOf[Product].productElement(index).asInstanceOf[t]
        summonInline[ConfigMonoid[t]].orElse(get(main), get(defaults)) *: valueTuple[ts, T](
          index + 1,
          main,
          defaults
        )

  inline given derive: [T] => (m: Mirror.ProductOf[T]) => ConfigMonoid[T] =
    new ConfigMonoid[T]:
      def zero: T = m.fromProduct(zeroTuple[m.MirroredElemTypes])
      def orElse(main: T, defaults: T): T = m.fromProduct(valueTuple[m.MirroredElemTypes, T](0, main, defaults))

final case class PublishContextualOptions(
  v1: Option[String] = None,
  v2: Option[String] = None,
  v3: Option[String] = None,
)
object PublishContextualOptions:
  implicit val monoid: ConfigMonoid[PublishContextualOptions] = ConfigMonoid.derive // was crash

We get the following error:

     while compiling: tests/pos/i21320b.scala
        during phase: Ycheck
                mode: Mode(ImplicitsEnabled)
     library version: version 3.8.0-RC1-bin-SNAPSHOT-nonbootstrapped
    compiler version: version 3.8.0-RC1-bin-SNAPSHOT-nonbootstrapped-git-96b1b83
            settings: -Vprint List(inlining) -Ycheck List(all) -classpath /localhome/bovel/scala3/library/target/scala-library-nonbootstrapped/scala-library-3.8.0-RC1-bin-SNAPSHOT-nonbootstrapped.jar -d /localhome/bovel/scala3/out/default-last-scalac-out.jar
Exception in thread "main" java.lang.AssertionError: assertion failed: wrong source set for x$1 # -1 of class dotty.tools.dotc.ast.Trees$Ident, set to tests/pos/i21320b.scala but context had library/src/scala/Tuple.scala
 <synthetic> <touched>
        at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:10)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:36)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.fold$1(Trees.scala:1681)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.apply(Trees.scala:1683)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1700)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1694)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1702)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1708)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
...

The zero method for Monoid[PublishContextualOptions] looks like this on main:

def zero: PublishContextualOptions =
              m$proxy1.fromProduct(
                {
                  {
                    val x$1: Option[String] = ConfigMonoid.option[String].zero
                    {
                      val Tuple_this: Tuple =
                        {
                          {
                            val x$1: Option[String] =
                              ConfigMonoid.option[String].zero
                            {
                              val Tuple_this: Tuple =
                                {
                                  {
                                    val x$1: Option[String] =
                                      ConfigMonoid.option[String].zero
                                    {
                                      val Tuple_this: Tuple =
                                        {
                                          EmptyTuple
                                        }:Tuple
                                      runtime.Tuples.cons(x$1, Tuple_this).
                                        asInstanceOf[*:[Option[String], Tuple]]:
                                        *:[Option[String], Tuple]
                                    }
                                  }
                                }:Tuple
                              runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                                *:[Option[String], Tuple]]:
                                *:[Option[String], Tuple]
                            }
                          }
                        }:Tuple
                      runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                        *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                    }
                  }
                }:Tuple

And with my changes:

def zero: PublishContextualOptions =
              m$proxy1.fromProduct(
                {
                  {
                    val x$1: Option[String] = ConfigMonoid.option[String].zero
                    {
                      val x$1: Option[String] = ConfigMonoid.option[String].zero
                      val x$1: Option[String] = ConfigMonoid.option[String].zero
                      val Tuple_this: EmptyTuple.type = EmptyTuple
                      val Tuple_this: Option[String] *: Tuple =
                        runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                          *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                      {
                        val Tuple_this: Option[String] *: Tuple =
                          runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                            *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                        runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                          *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                      }
                    }
                  }
                }
              )

Which is incorrect.

My current guess is that dropping ascriptions enables eliding expressions, which in turns create other problems.

Need to further investigate…

Note: I tried to narrow the ascriptions stripping to only ascriptions created by PrepareInlinable.wrapRHS, but that doesn't solve the problem. I also tried to use a proper TreeMap to remove them, or to do it directly in Inliner.typedTyped.

[mbovel]

My current guess is that dropping ascriptions enables eliding expressions, which in turns create other problems.

Bingo, the culprit is liftBindings; if I disable it for blocks, then the tests pass again.

[mbovel]

Edit: fixed by 8b803e1. 6ae39ef was incomplete because it lost the capability to inline calls inside blocks, returning EmptyTrees instead.

The remaining failing test is tests/pos/i18123.scala. It fails because I disabled lifting blocks in liftBindings.

Minimal reproduction:

extension (x: String)
  transparent inline def rep(min: Int = 0): String = ???
def y: String = ???
def z = y.rep().toUpperCase

When lifting blocks, it fails (as expected?) with a type error:

-- [E007] Type Mismatch Error: tests/pos/i18123b.scala:6:8 ---------------------
6 |def z = y.rep().toUpperCase
  |        ^^^^^^^
  |Found:    (??? : => Nothing)
  |Required: ?{ toUpperCase: ? }
  |Note that implicit conversions were not tried because the result of an implicit conversion
  |must be more specific than ?{ toUpperCase: <?> }
  |
  | longer explanation available when compiling with `-explain`

But if we don't lift blocks, then we get:

-- Error: tests/pos/i18123b.scala:6:8 ------------------------------------------
6 |def z = y.rep().toUpperCase
  |        ^^^^^^^
  |        <notype> is illegal as a selection prefix

Which is due to the following adaptation:

adapt1(
  tree = {
    val x$1: String = y
    rep(x$1)(i18123b$package.rep$default$2(x$1))
  },
  pt = ?{ toUpperCase: ? }
) == <empty>

[mbovel]

I temporary solved the problems described in my 3 last comments by disabling lifting blocks in liftBindings. I suspect there is a problem when computing positions there. It's probably fixable, but I am not sure how.

Remaining, unrelated test failure:

[info] Test dotty.tools.dotc.semanticdb.SemanticdbTests.expectTests started
/home/runner/work/scala3/scala3/tests/semanticdb/javacp/com/javacp/Test.java:69: warning: [strictfp] as of release 17, all floating-point expressions are evaluated strictly and 'strictfp' is not required
    strictfp void strictfpMethod() {}
                  ^
1 warning
[error] check file /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect does not match generated.
If you meant to make a change, replace the expect file by:
  mv /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect.out /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect
inspect with:
  diff /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect.out
Or else update all expect files with
  sbt 'scala3-compiler-bootstrapped/Test/runMain dotty.tools.dotc.semanticdb.updateExpect'
Error:  Test dotty.tools.dotc.semanticdb.SemanticdbTests.expectTests failed: java.lang.AssertionError: 1 errors in expect test., took 1.729 sec
Error:      at dotty.tools.dotc.semanticdb.SemanticdbTests.runExpectTest(SemanticdbTests.scala:108)
Error:      at dotty.tools.dotc.semanticdb.SemanticdbTests.expectTests(SemanticdbTests.scala:65)
Error:      at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
Error:      at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
Error:      at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
Error:      at java.lang.reflect.Method.invoke(Method.java:569)
Error:      ...

To be investigated.