Skip to content

Commit

Permalink
Add warning for anonymous inline classes (#16723)
Browse files Browse the repository at this point in the history
  • Loading branch information
aherlihy committed May 3, 2024
1 parent e2c456f commit 1f14b35
Show file tree
Hide file tree
Showing 17 changed files with 87 additions and 16 deletions.
2 changes: 1 addition & 1 deletion community-build/community-projects/scodec
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/Formatting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ object Formatting {
trait CtxShow:
def run(using Context): Shown

private inline def CtxShow(inline x: Context ?=> Shown) = new CtxShow { def run(using Context) = x(using ctx) }
private inline def CtxShow(inline x: Context ?=> Shown) =
class InlinedCtxShow extends CtxShow { def run(using Context) = x(using ctx) }
new InlinedCtxShow
private def toStr[A: Show](x: A)(using Context): String = Shown.toStr(toShown(x))
private def toShown[A: Show](x: A)(using Context): Shown = Show[A].show(x).runCtxShow

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case UnstableInlineAccessorID // errorNumber: 192
case VolatileOnValID // errorNumber: 193
case ExtensionNullifiedByMemberID // errorNumber: 194
case InlinedAnonClassWarningID // errorNumber: 195

def errorNumber = ordinal - 1

Expand Down
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3104,6 +3104,15 @@ extends SyntaxMsg(InlineGivenShouldNotBeFunctionID):
| inline def apply(x: A) = x.toB
"""

class InlinedAnonClassWarning()(using Context)
extends Message(InlinedAnonClassWarningID):
def kind = MessageKind.PotentialIssue
def msg(using Context) = "New anonymous class definition will be duplicated at each inline site"
def explain(using Context) =
i"""Anonymous class will be defined at each use site, which may lead to a larger number of classfiles.
|
|To inline class definitions, you may provide an explicit class name to avoid this warning."""

class ValueDiscarding(tp: Type)(using Context)
extends Message(ValueDiscardingID):
def kind = MessageKind.PotentialIssue
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Symbols.*
import typer.RefChecks
import MegaPhase.MiniPhase
import ast.tpd
import reporting.InlinedAnonClassWarning

import config.Feature
import Decorators.*
Expand Down Expand Up @@ -51,6 +52,7 @@ class PruneErasedDefs extends MiniPhase with SymTransformer { thisTransform =>
else cpy.ValDef(tree)(rhs = trivialErasedTree(tree.rhs))

override def transformDefDef(tree: DefDef)(using Context): Tree =
RefChecks.checkNoInlineAnnoClasses(tree)
checkErasedInExperimental(tree.symbol)
if !tree.symbol.isEffectivelyErased || tree.rhs.isEmpty then tree
else cpy.DefDef(tree)(rhs = trivialErasedTree(tree.rhs))
Expand Down
10 changes: 10 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ object RefChecks {
def isStable = true
}

def checkNoInlineAnnoClasses(tree: DefDef)(using Context): Unit =
if tree.symbol.is(Inline) then
new TreeTraverser {
def traverse(tree: Tree)(using Context): Unit =
tree match
case tree: TypeDef if tree.symbol.isAnonymousClass =>
report.warning(new InlinedAnonClassWarning(), tree.symbol.sourcePos)
case _ => traverseChildren(tree)
}.traverse(tree)

/** Only one overloaded alternative is allowed to define default arguments */
private def checkOverloadedRestrictions(clazz: Symbol)(using Context): Unit = {
// Using the default getters (such as methodName$default$1) as a cheap way of
Expand Down
12 changes: 12 additions & 0 deletions tests/neg/i13044.check
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
-- [E195] Potential Issue Warning: tests/neg/i13044.scala:26:8 ---------------------------------------------------------
26 | new Schema[A] {
| ^
| New anonymous class definition will be duplicated at each inline site
|
| longer explanation available when compiling with `-explain`
-- [E195] Potential Issue Warning: tests/neg/i13044.scala:32:8 ---------------------------------------------------------
32 | new Schema[A] {
| ^
| New anonymous class definition will be duplicated at each inline site
|
| longer explanation available when compiling with `-explain`
-- Error: tests/neg/i13044.scala:65:40 ---------------------------------------------------------------------------------
65 | implicit def typeSchema: Schema[A] = Schema.gen // error // error
| ^^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion tests/pos/i17314.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ object circelike {
inline final def derived[A](using conf: Configuration)(using
inline mirror: Mirror.Of[A]
): ConfiguredCodec[A] =
new ConfiguredCodec[A]:
class InlinedConfiguredCodec extends ConfiguredCodec[A]:
val codec = summonInline[Codec[URI]] // simplification
new InlinedConfiguredCodec
}

object foo {
Expand Down
4 changes: 2 additions & 2 deletions tests/pos/not-looping-implicit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ object Schema {
inline summonInline[Mirror.Of[A]] match {
case m: Mirror.SumOf[A] =>
lazy val members = recurse[m.MirroredElemLabels, m.MirroredElemTypes]()
new Schema[A] {}
???
case m: Mirror.ProductOf[A] =>
lazy val fields = recurse[m.MirroredElemLabels, m.MirroredElemTypes]()
new Schema[A] {}
???
}

inline given gen[A]: Schema[A] = derived[A]
Expand Down
12 changes: 7 additions & 5 deletions tests/run/i11050.scala
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,14 @@ object Show:

inline def show[T](x: T): String = summonInline[Show[T]].show(x)

transparent inline def derived[T](implicit ev: Mirror.Of[T]): Show[T] = new {
def show(x: T): String = inline ev match {
case m: Mirror.ProductOf[T] => showProduct(x.asInstanceOf[Product], m)
case m: Mirror.SumOf[T] => showCases[m.MirroredElemTypes](0)(x, m.ordinal(x))
transparent inline def derived[T](implicit ev: Mirror.Of[T]): Show[T] =
class InlinedShow extends Show[T] { // provide name to anonymous class
def show(x: T): String = inline ev match {
case m: Mirror.ProductOf[T] => showProduct(x.asInstanceOf[Product], m)
case m: Mirror.SumOf[T] => showCases[m.MirroredElemTypes](0)(x, m.ordinal(x))
}
}
}
new InlinedShow

transparent inline def showProduct[T](x: Product, m: Mirror.ProductOf[T]): String =
constValue[m.MirroredLabel] + showElems[m.MirroredElemTypes, m.MirroredElemLabels](0, Nil)(x)
Expand Down
6 changes: 3 additions & 3 deletions tests/warn/i15503i.scala
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ package foo.test.i16679a:
import scala.deriving.Mirror
object CaseClassByStringName:
inline final def derived[A](using inline A: Mirror.Of[A]): CaseClassByStringName[A] =
new CaseClassByStringName[A]:
new CaseClassByStringName[A]: // warn
def name: String = A.toString

object secondPackage:
Expand All @@ -263,7 +263,7 @@ package foo.test.i16679b:
object CaseClassName:
import scala.deriving.Mirror
inline final def derived[A](using inline A: Mirror.Of[A]): CaseClassName[A] =
new CaseClassName[A]:
new CaseClassName[A]: // warn
def name: String = A.toString

object Foo:
Expand All @@ -279,7 +279,7 @@ package foo.test.i17156:
package a:
trait Foo[A]
object Foo:
inline def derived[T]: Foo[T] = new Foo{}
inline def derived[T]: Foo[T] = new Foo{} // warn

package b:
import a.Foo
Expand Down
4 changes: 2 additions & 2 deletions tests/warn/i15503j.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ package foo.unused.summon.inlines:

transparent inline given conflictInside: C =
summonInline[A]
new {}
???

transparent inline given potentialConflict: C =
summonInline[B]
new {}
???

val b: B = summon[B]
val c: C = summon[C]
6 changes: 6 additions & 0 deletions tests/warn/i16723.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- [E195] Potential Issue Warning: tests/warn/i16723.scala:3:2 ---------------------------------------------------------
3 | new Object {} // warn
| ^
| New anonymous class definition will be duplicated at each inline site
|
| longer explanation available when compiling with `-explain`
3 changes: 3 additions & 0 deletions tests/warn/i16723.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
inline def foo =
class NotAnon
new Object {} // warn
6 changes: 6 additions & 0 deletions tests/warn/i16723a.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- [E195] Potential Issue Warning: tests/warn/i16723a.scala:5:38 -------------------------------------------------------
5 |inline given Converter[Int, String] = new Converter { // warn
| ^
| New anonymous class definition will be duplicated at each inline site
|
| longer explanation available when compiling with `-explain`
17 changes: 17 additions & 0 deletions tests/warn/i16723a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
trait Converter[A, B] {
def convert: A => B
}

inline given Converter[Int, String] = new Converter { // warn
def convert = _.toString()
}

def foo(using bar: Converter[Int, String]) =
"foo"

@main
def main =
foo
foo
foo
foo

0 comments on commit 1f14b35

Please sign in to comment.