Predef.implicitly TODO comment

[xuwei-k]

https://github.com/scala/scala/blob/v2.13.0-M5/src/library/scala/Predef.scala#L214

// TODO: when dependent method types are on by default, give this result type e.type, so that inliner has better chance of knowing which method to inline in calls like implicitly[MatchingStrategy[Option]].zero

Should we just add e.type? or remove TODO comment if incorrect?

[Jasper-M]

I am very much in favor of this change. It also prevents loss of type information.

[xuwei-k]

🤔

object A {
  def x1[A](implicit a: A): A = a
  @inline def x2[A](implicit a: A): A = a
  def x3[A](implicit a: A): a.type = a
  @inline def x4[A](implicit a: A): a.type = a

  implicit val i = 42

  def main(): Unit = {
    x1[Int]

    x2[Int]

    x3[Int]

    x4[Int]
  }
}

$ scala -version
Scala code runner version 2.13.0-M5 -- Copyright 2002-2018, LAMP/EPFL and Lightbend, Inc.
$ java -version
java version "1.8.0_181"
Java(TM) SE Runtime Environment (build 1.8.0_181-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.181-b13, mixed mode)
$ scalac -opt:l:inline -opt-inline-from:** A.scala 

javap -v A\$.class

  public void main();
    descriptor: ()V
    flags: ACC_PUBLIC
    Code:
      stack=2, locals=1, args_size=1
         0: aload_0
         1: aload_0
         2: invokevirtual #29                 // Method i:()I
         5: invokestatic  #35                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
         8: invokevirtual #37                 // Method x1:(Ljava/lang/Object;)Ljava/lang/Object;
        11: pop
        12: aload_0
        13: invokevirtual #29                 // Method i:()I
        16: pop
        17: aload_0
        18: aload_0
        19: invokevirtual #29                 // Method i:()I
        22: invokestatic  #35                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
        25: invokevirtual #39                 // Method x3:(Ljava/lang/Object;)Ljava/lang/Object;
        28: pop
        29: aload_0
        30: invokevirtual #29                 // Method i:()I
        33: pop
        34: return
      LineNumberTable:
        line 10: 0
        line 12: 12
        line 14: 17
        line 16: 29
        line 16: 34
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      35     0  this   LA$;
}

[lrytz]

Maybe your shell expanded the **, try with '-opt-inline-from:**'?

[smarter]

Would be interesting to do this change and see what happens to the community build. Same thing with identity and locally .

[lrytz]

(Wait, I misunderstood something)

[lrytz]

What is the problem with the bytecode you posted above?

[milessabin]

FYI shapeless has a the method with these semantics, backed by a macro to ensure inlining, and there is also @non's Imp which is basically the same.

I'm sure Erik won't mind me saying that we'd both like to see this change so we can retire our workarounds :-)

[xuwei-k]

The @inline annotation provide meaningful effects regardless of whether add : a.type = or not.

[lrytz]

The inliner works on the bytecode level, on erased types, so the change doesn't matter there.

It can make a difference in constant folding, maybe elsewhere

scala> def id[T](a: T): T = a
id: [T](a: T)T

scala> def ida[T](a: T): a.type = a
ida: [T](a: T)a.type

scala> id(1) + 2 : 3
             ^
       error: type mismatch;
        found   : Int
        required: 3

scala> ida(1) + 2 : 3
res5: Int = 3

[xuwei-k]

Maybe we should also change following similar methods.

scala/math/Equiv.scala:  @inline def apply[T: Equiv]: Equiv[T] = implicitly[Equiv[T]]
scala/math/Fractional.scala:  @inline def apply[T](implicit frac: Fractional[T]): Fractional[T] = frac
scala/math/Integral.scala:  @inline def apply[T](implicit int: Integral[T]): Integral[T] = int
scala/math/Numeric.scala:  @inline def apply[T](implicit num: Numeric[T]): Numeric[T] = num
scala/math/Ordering.scala:  @inline def apply[T](implicit ord: Ordering[T]) = ord

[milessabin]

@xuwei-k what would that gain us in those cases?

The rationale for making the change in the case of implicitly is that, depending on the type argument, there are refinements which are lost. But in the case of those type classes there aren't any useful refinements to lose.

[smarter]

Implementing this in Scala 2 will require fixing the compiler to be a bit more tolerant when using a dependent method as a function (it should just widen the dependent part), from https://github.com/smarter/scala/tree/precise-id:

[error] /home/smarter/opt/scala/src/library/scala/StringContext.scala:114:54: polymorphic expression cannot be instantiated to expected type;
[error]  found   : [A](x: A)x.type (with underlying type [A](x: A)x.type)
[error]  required: String => String
[error]   def raw(args: Any*): String = standardInterpolator(identity, args, parts)
[error]                                                      ^
[error] /home/smarter/opt/scala/src/library/scala/collection/ArrayOps.scala:1209:39: method with dependent type (x: A)x.type cannot be converted to function value
[error]   def distinct: Array[A] = distinctBy(identity)
[error]                                       ^

[adriaanm]

Let's consider this during 2.14 dev, since it has knock-on effects as mentioned by smarter.

[xuwei-k]

https://github.com/scala/scala/pull/7874

[adriaanm]

Here's what I tried to improve type inference, but this broke a massive number of quasiquote-related tests, and we're out of time for RC1:

  object ApproximateDependentMap extends TypeMap {
    def apply(tp: Type): Type =
      if (tp.isImmediatelyDependent) {
        // Don't give up this easily -- we may be able to provide a bit more information by using the widened type.
        // This pattern is used by Predef.implicitly to propagate the type var in the argument type to the result,
        // while also being as precise as possible in the result type.
        val w = tp.widen
        if (tp ne w) apply(w) else WildcardType
      }
      else tp.mapOver(this)
  }

I also tried not changing the compiler, but a slightly more sneaky signature for implicitly:

  @inline def implicitly[T](implicit e: T): e.type with T = e

this runs into other compiler bugs (the evidence parameter may be private, and the result type is not properly widened to avoid depending on it) 😓

[adriaanm]

For the record, I tried one more (perhaps too hacky) compromise, but still too many macro tests break (and I have no idea how to even approach fixing them)

  object ApproximateDependentMap extends TypeMap {
    def apply(tp: Type): Type =
      if (tp.isImmediatelyDependent) {
        // Don't give up just yet -- we may be able to propagate a bit more information for type inference if we can widen to a type parameter.
        // This pattern is used by Predef.implicitly to propagate the type var in the argument type to the result,
        // while also being as precise as possible in the result type.
        val w = tp.widen
        if ((tp ne w) && w.typeSymbol.isTypeParameterOrSkolem) apply(w) else WildcardType
      }
      else tp.mapOver(this)
  }

Test failures:

• run/macro-reify-chained1
• run/macro-reify-chained2
• run/macro-reify-nested-a1
• run/macro-reify-nested-a2
• run/macro-reify-nested-b1
• run/macro-reify-nested-b2

wip: https://github.com/scala/scala/compare/2.13.x...adriaanm:implicitly-precise