From f7495619b41af7d29466e30f53236e702d31ba7a Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 10 Dec 2022 15:16:28 +0100 Subject: [PATCH 1/2] Warn on inline given aliases with functions as RHS ```scala inline given a: Conversion[String, Item] = Item(_) ``` will now produce this warning: ``` 5 | inline given a: Conversion[String, Item] = Item(_) | ^^^^^^^ |An inline given alias with a function value as right-hand side can significantly increase |generated code size. You should either drop the `inline` or rewrite the given with an |explicit `apply` method. |---------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | A function value on the right-hand side of an inline given alias expands to | an anonymous class. Each application of the inline given will then create a | fresh copy of that class, which can increase code size in surprising ways. | For that reason, functions are discouraged as right hand sides of inline given aliases. | You should either drop `inline` or rewrite to an explicit `apply` method. E.g. | | inline given Conversion[A, B] = x => x.toB | | should be re-formulated as | | inline given Conversion[A, B] with | def apply(x: A) = x.toB | ``` Fixes #16497 --- .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 20 +++++++++++++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 4 ++++ .../fatal-warnings/inline-givens.scala | 15 ++++++++++++++ 4 files changed, 40 insertions(+) create mode 100644 tests/neg-custom-args/fatal-warnings/inline-givens.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index a7bc7f027517..926533b01f1b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -187,6 +187,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case MissingArgumentID // errorNumer 171 case MissingImplicitArgumentID // errorNumber 172 case CannotBeAccessedID // errorNumber 173 + case InlineGivenShouldNotBeFunctionID // errorNumber 174 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index a3356c1f7021..885481c4f2de 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2769,4 +2769,24 @@ extends ReferenceMsg(CannotBeAccessedID): i"$whatCanNot be accessed as a member of $pre$where.$whyNot" def explain(using Context) = "" +class InlineGivenShouldNotBeFunction()(using Context) +extends SyntaxMsg(InlineGivenShouldNotBeFunctionID): + def msg(using Context) = + i"""An inline given alias with a function value as right-hand side can significantly increase + |generated code size. You should either drop the `inline` or rewrite the given with an + |explicit `apply` method.""" + def explain(using Context) = + i"""A function value on the right-hand side of an inline given alias expands to + |an anonymous class. Each application of the inline given will then create a + |fresh copy of that class, which can increase code size in surprising ways. + |For that reason, functions are discouraged as right hand sides of inline given aliases. + |You should either drop `inline` or rewrite to an explicit `apply` method. E.g. + | + | inline given Conversion[A, B] = x => x.toB + | + |should be re-formulated as + | + | inline given Conversion[A, B] with + | def apply(x: A) = x.toB + """ diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1f6516cdfb1c..0e59174844bb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2374,6 +2374,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if sym.isInlineMethod then if StagingContext.level > 0 then report.error("inline def cannot be within quotes", sym.sourcePos) + if sym.is(Given) + && untpd.stripBlock(untpd.unsplice(ddef.rhs)).isInstanceOf[untpd.Function] + then + report.warning(InlineGivenShouldNotBeFunction(), ddef.rhs.srcPos) val rhsToInline = PrepareInlineable.wrapRHS(ddef, tpt1, rhs1) PrepareInlineable.registerInlineInfo(sym, rhsToInline) diff --git a/tests/neg-custom-args/fatal-warnings/inline-givens.scala b/tests/neg-custom-args/fatal-warnings/inline-givens.scala new file mode 100644 index 000000000000..eae50bca45cf --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/inline-givens.scala @@ -0,0 +1,15 @@ + +class Item(x: String) + +inline given a: Conversion[String, Item] = + Item(_) // error + +inline given b: Conversion[String, Item] = + (x => Item(x)) // error + +inline given c: Conversion[String, Item] = + { x => Item(x) } // error + +inline given d: Conversion[String, Item] with + def apply(x: String) = Item(x) // ok + From 8274b10af8887608c25a8371836e2e6e91ccdf2f Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 14 Dec 2022 18:54:41 +0100 Subject: [PATCH 2/2] Update compiler/src/dotty/tools/dotc/reporting/messages.scala Co-authored-by: Guillaume Martres --- compiler/src/dotty/tools/dotc/reporting/messages.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 885481c4f2de..1300afc9413c 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2786,7 +2786,7 @@ extends SyntaxMsg(InlineGivenShouldNotBeFunctionID): | |should be re-formulated as | - | inline given Conversion[A, B] with - | def apply(x: A) = x.toB + | given Conversion[A, B] with + | inline def apply(x: A) = x.toB """