Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion library/src/scala/math/BigDecimal.scala
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,6 @@ extends ScalaNumber with ScalaNumericConversions with Serializable with Ordered[

/** Returns the decimal String representation of this BigDecimal.
*/
override def toString: String = this.bigDecimal.toString
override def toString(): String = this.bigDecimal.toString
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a fix without this change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this the main part, this fixes the issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes the issue because the issue uses BigDecimal. I'm curious if the same issue could arise when defining a custom class that overrides toString and define a ToExpr on it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if the same issue could arise when defining a custom class that overrides toString and define a ToExpr on it.

Yes, it can.

If we override methods from Java, we should require the override to be non-nullary, however, that wouldn't be backward-compatible. Alternatively, the typer could rewrite the override method from nullary to nilary.

What's the reason for the mismatch in the definition of the toString method in stdlib (e.g. BigInt vs BigDecimal)? I think that we should standardize this.

In any case, this change is invisible to developers, fixes the regression, and is consistent with the Java definition.

Copy link
Member

@hamzaremmal hamzaremmal Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if the same issue could arise when defining a custom class that overrides toString and define a ToExpr on it.
Yes, it can.

I'm sure it would. I don't remember why I wrote my comment with if.

What's the reason for the mismatch in the definition of the toString method in stdlib (e.g. BigInt vs BigDecimal)? I think that we should standardize this.

I agree with that and I tried to make the case a long time ago but I've been told it works as designed. This was the last time I brought this topic in a public comment. Regardless, we should acknowledge the bug with ToExpr and Java overrides in a separate issue and have it more minimized know that we fully understand the root of the issue. When that is done, I would be happy to approve this PR to at least close the regression.


}
3 changes: 1 addition & 2 deletions library/src/scala/quoted/ToExpr.scala
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,7 @@ object ToExpr {
/** Default implementation of `ToExpr[BigDecimal using the default MathContext]` */
given BigDecimalToExpr: ToExpr[BigDecimal] with {
def apply(x: BigDecimal)(using Quotes): Expr[BigDecimal] =
val bigDecimal: String = "" + x // workaround "method toString in class BigDecimal does not take parameters" in scaladoc/generateScalaDocumentation
'{ BigDecimal(${Expr(bigDecimal)}) }
'{ BigDecimal(${Expr(x.toString)}) }
}

/** Default implementation of `ToExpr[StringContext]` */
Expand Down
12 changes: 12 additions & 0 deletions tests/warn/i24093.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E197] Potential Issue Warning: tests/warn/i24093.scala:18:10 -------------------------------------------------------
18 | new StringConverter[T]: // warn
| ^
| New anonymous class definition will be duplicated at each inline site
|
| longer explanation available when compiling with `-explain`
-- [E197] Potential Issue Warning: tests/warn/i24093.scala:34:4 --------------------------------------------------------
34 | new QueryParamsConverter[T] { // warn
| ^
| New anonymous class definition will be duplicated at each inline site
|
| longer explanation available when compiling with `-explain`
54 changes: 54 additions & 0 deletions tests/warn/i24093.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import scala.deriving.*
import scala.compiletime.*

trait StringConverter[T]:
def to(t: T): String

trait QueryParams
trait QueryParamsConverter[T]:
def to(t: T): QueryParams

object QueryParamsConverter {
extension [T](t: T)
def toQueryParams(using converter: QueryParamsConverter[T]): QueryParams = converter.to(t)

inline private def summonStringConverter[T]: StringConverter[T] = summonFrom {
case converter: StringConverter[T] => converter
case _ =>
new StringConverter[T]: // warn
override def to(t: T): String = t.toString
}

inline def summonStringConverters[A <: Tuple]: List[StringConverter[?]] =
inline erasedValue[A] match
case _: EmptyTuple => Nil
case _: (t *: ts) => summonStringConverter[t] :: summonStringConverters[ts]

inline def summonQueryParamsConverters[A <: Tuple]: List[QueryParamsConverter[?]] =
inline erasedValue[A] match
case _: EmptyTuple => Nil
case _: (t *: ts) => summonInline[QueryParamsConverter[t]] :: summonQueryParamsConverters[ts]

inline given derived[T](using m: Mirror.Of[T]): QueryParamsConverter[T] = {
val stringConverters = summonStringConverters[m.MirroredElemTypes]
new QueryParamsConverter[T] { // warn
override def to(t: T): QueryParams =
inline m match
case p: Mirror.ProductOf[T] => ???
case s: Mirror.SumOf[T] =>
val _ = summonQueryParamsConverters[m.MirroredElemTypes]
???
}
}

}

object Test:
sealed trait FutureOrderCreateParams
object FutureOrderCreateParams:
case class LIMIT(quantity: BigDecimal) extends FutureOrderCreateParams

case class FutureOrderCreateResponse(clientOrderId: String)

import QueryParamsConverter.*
def createOrder(orderCreate: FutureOrderCreateParams) = orderCreate.toQueryParams
Loading