Inline SAM conversions can generate lots of code

[odersky]

Compiler version

All 3.x versions

Minimized example

Here's some code that illustrates a problem found in Lichess:

class Item(x: String)

inline given Conversion[String, Item] = Item(_)

inline given Conversion[String, Int] with
  def apply(x: String) = Item(x)

Output

Here's the code generated for the first version:

    final inline given def given_Conversion_String_Item:
      Conversion[String, Item] =
      {
        def $anonfun(_$1: String): Item = new Item(_$1)
        closure($anonfun:Conversion[String, Item])
      }:Conversion[String, Item]

And here's the code generated for the 2nd version:

    given class given_Conversion_String_Int() extends Conversion[String, Int]()
       {
      def apply(x: String): Int = new Item(x)
    }
    final inline given def given_Conversion_String_Int:
      given_Conversion_String_Int =
      new given_Conversion_String_Int():given_Conversion_String_Int

The first version generates an anonymous function for the SAM-type Conversion, which will be expanded to an anonymous class later. Since the conversion is inline, this means that every one of its invocations will generate an anonymous class. This can lead to increased code pressure without the user noticing the problem, since all invocations are implicit. It's probably this what caused the increased code size in the Lichess port to Scala 3.

The second version does not have this problem. There's a single class generated and every inline conversion just creates a new object instance.

Question

Should we prevent the first version or at least warn about it? I see the following possibilities:

• Don't allow inline givens with a SAM closure as the right hand side. Suggest the rewriting to explicit apply.
• Warn about inline givens with a SAM closure as the right hand side, with the same suggestion.
• Silently rewrite the first version to the second.

What do people think? Which of these should be preferred?

[odersky]

Silent rewriting is problematic since the version with explicit apply creates a user-visible class name. So that leaves warning or error. I tend towards error, since it is clearer and the problem is easily avoided by a rewrite. EDIT: It seems there are some weird interactions with @compiletimeOnly. The error behavior of #16498 is different for test neg-macros/i11483. So maybe it's easier to just do a warning.

The error behavior is implemented in #16498 and the warning behavior is implemented in #16499.

[nicolasstucki]

To inline the conversion and the least amount of code we should use

given Conversion[String, Item] with
  inline def apply(x: String) = Item(x)

[nicolasstucki]

There are four combinations

class Item1(x: String)
inline given Conversion[String, Item1] = Item1(_)
def test1 = summon[Conversion[String, Item1]].apply("abc")

class Item2(x: String)
inline given Conversion[String, Item2] with
  def apply(x: String) = Item2(x)

class Item3(x: String)
given Conversion[String, Item3] with
  inline def apply(x: String) = Item3(x)

class Item4(x: String)
inline given Conversion[String, Item4] with
  inline def apply(x: String) = Item4(x)

def test1 = summon[Conversion[String, Item1]].apply("abc")
def test2 = summon[Conversion[String, Item2]].apply("abc")
def test3 = summon[Conversion[String, Item3]].apply("abc")
def test4 = summon[Conversion[String, Item4]].apply("abc")

which produce the following

def test1: Item1 = ((x: String) => (new Item1(x)): Conversion[String, Item1]).apply("abc") 
def test2: Item2 = (new given_Conversion_String_Item2(): given_Conversion_String_Item2).apply("abc")
def test3: Item3 = new Item3("abc"): Item3 // BEST
def test4: Item4 = 
  val given_Conversion_String_Item4_this: given_Conversion_String_Item4 =  new given_Conversion_String_Item4():given_Conversion_String_Item4
  new Item4("abc"):Item4

[smarter]

Is there any situation right now where inline given .. with is useful?

[odersky]

It just inlines the constructor call. Could be useful when passed to an inline using parameter that wants to inspect what was passed.

[nicolasstucki]

If the given has parameters we get

class Item1(x: String)(using DummyImplicit)
inline given (using d: DummyImplicit): Conversion[String, Item1] = Item1(_)

class Item2(x: String)(using DummyImplicit)
inline given (using d: DummyImplicit): Conversion[String, Item2] with
  def apply(x: String) = Item2(x)

class Item3(x: String)(using DummyImplicit)
given (using d: DummyImplicit): Conversion[String, Item3] with
  inline def apply(x: String) = Item3(x)

class Item4(x: String)(using DummyImplicit)
inline given (using d: DummyImplicit): Conversion[String, Item4] with
  inline def apply(x: String) = Item4(x)

def test1 = summon[Conversion[String, Item1]].apply("abc")
def test2 = summon[Conversion[String, Item2]].apply("abc")
def test3 = summon[Conversion[String, Item3]].apply("abc")
def test4 = summon[Conversion[String, Item4]].apply("abc")

we get

def test1: Item1 =
  (((x: String) => new Item1(x)(DummyImplicit.dummyImplicit)): Conversion[String, Item1]).apply("abc")
def test2: Item2 =
  (new given_Conversion_String_Item2(using DummyImplicit.dummyImplicit)(): given_Conversion_String_Item2).apply("abc")
def test3: Item3 =
  val given_Conversion_String_Item3_this: given_Conversion_String_Item3 = given_Conversion_String_Item3(DummyImplicit.dummyImplicit)
  new Item3("abc")(given_Conversion_String_Item3_this.Foo$package$given_Conversion_String_Item3$$inline$d):Item3
def test4: Item4 =
  val given_Conversion_String_Item4_this: given_Conversion_String_Item4 = new given_Conversion_String_Item4(using DummyImplicit.dummyImplicit)() :given_Conversion_String_Item4
  new Item4("abc")(given_Conversion_String_Item4_this.Foo$package$given_Conversion_String_Item4$$inline$d):Item4

in this case the inline given with inline def could be marginally better.