From 3ebc82b63396d829adc389adc7c7aa2314e61357 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Mon, 28 Apr 2025 16:19:56 +0000 Subject: [PATCH 1/3] Fix `Constant` tag of `Expr(null: String)` Co-authored-by: Hamza Remmal Co-authored-by: HarrisL2 Co-authored-by: Abdullah Arif Jafri [Cherry-picked bbe2dde1ee10adc05ceece96ab82af8a66a21880] --- .../src/scala/quoted/runtime/impl/QuotesImpl.scala | 2 +- tests/run-macros/i23008.check | 1 + tests/run-macros/i23008/Macros_1.scala | 12 ++++++++++++ tests/run-macros/i23008/Test_2.scala | 2 ++ 4 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 tests/run-macros/i23008.check create mode 100644 tests/run-macros/i23008/Macros_1.scala create mode 100644 tests/run-macros/i23008/Test_2.scala diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index b6d0eae2fbbb..8d58632bb79c 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -2419,7 +2419,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end StringConstantTypeTest object StringConstant extends StringConstantModule: - def apply(x: String): StringConstant = dotc.core.Constants.Constant(x) + def apply(x: String): StringConstant = dotc.core.Constants.Constant(x: Any) def unapply(constant: StringConstant): Some[String] = Some(constant.stringValue) end StringConstant diff --git a/tests/run-macros/i23008.check b/tests/run-macros/i23008.check new file mode 100644 index 000000000000..19765bd501b6 --- /dev/null +++ b/tests/run-macros/i23008.check @@ -0,0 +1 @@ +null diff --git a/tests/run-macros/i23008/Macros_1.scala b/tests/run-macros/i23008/Macros_1.scala new file mode 100644 index 000000000000..052760dbf833 --- /dev/null +++ b/tests/run-macros/i23008/Macros_1.scala @@ -0,0 +1,12 @@ +import scala.quoted.* + +object Macros { + inline def buildString = ${buildStringCode} + + def buildStringCode(using Quotes): Expr[String] = { + import quotes.reflect.* + val str: String = null + val exprString = Expr(str) + Expr(exprString.show) + } +} diff --git a/tests/run-macros/i23008/Test_2.scala b/tests/run-macros/i23008/Test_2.scala new file mode 100644 index 000000000000..cf3cfffb50ac --- /dev/null +++ b/tests/run-macros/i23008/Test_2.scala @@ -0,0 +1,2 @@ +@main def Test = + println(Macros.buildString) From 29d76b90cf953da04ec1a556cf222e3230e9bb5f Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Wed, 30 Apr 2025 15:13:38 +0000 Subject: [PATCH 2/3] Accept `null` in `StringToExpr` but not in `StringConstant` [Cherry-picked e0fbfde87a5faf9115db18dda4bfcda8fdbc21a5] --- compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala | 7 ++++++- library/src/scala/quoted/ToExpr.scala | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index 8d58632bb79c..b5b62816a448 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -2419,7 +2419,12 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end StringConstantTypeTest object StringConstant extends StringConstantModule: - def apply(x: String): StringConstant = dotc.core.Constants.Constant(x: Any) + def apply(x: String): StringConstant = + require(x != null, "StringConstant cannot be null") + // A `null` constant must be represented as a `NullConstant`, c.f. a + // constant with `tag == NullTag`, which is not a `StringConstant`. + // See issue 23008. + dotc.core.Constants.Constant(x) def unapply(constant: StringConstant): Some[String] = Some(constant.stringValue) end StringConstant diff --git a/library/src/scala/quoted/ToExpr.scala b/library/src/scala/quoted/ToExpr.scala index f9f3d00bcedf..e9cd06fd7ac9 100644 --- a/library/src/scala/quoted/ToExpr.scala +++ b/library/src/scala/quoted/ToExpr.scala @@ -77,7 +77,13 @@ object ToExpr { given StringToExpr[T <: String]: ToExpr[T] with { def apply(x: T)(using Quotes) = import quotes.reflect.* - Literal(StringConstant(x)).asExpr.asInstanceOf[Expr[T]] + val literal = + if (x: Any) == null then + // Can happen if called from code without `-Yexplicit-nulls`. + Literal(NullConstant()) + else + Literal(StringConstant(x)) + literal.asExpr.asInstanceOf[Expr[T]] } /** Default implementation of `ToExpr[Class[T]]` */ From e7aee4f2f7f4fbe5011d94e04f6d5c56ebf44805 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Thu, 8 May 2025 11:35:19 +0000 Subject: [PATCH 3/3] Do not handle `null` in `StringToExpr` [Cherry-picked 2e3c803340b8b170d144debeff02e811522d2717] --- .../quoted/runtime/impl/QuotesImpl.scala | 2 +- library/src/scala/quoted/Quotes.scala | 5 ++++- library/src/scala/quoted/ToExpr.scala | 8 +------ tests/neg-macros/i23008.check | 21 +++++++++++++++++++ .../i23008/Macro_1.scala} | 3 +-- tests/neg-macros/i23008/Test_2.scala | 1 + tests/run-macros/i23008.check | 1 - tests/run-macros/i23008/Test_2.scala | 2 -- 8 files changed, 29 insertions(+), 14 deletions(-) create mode 100644 tests/neg-macros/i23008.check rename tests/{run-macros/i23008/Macros_1.scala => neg-macros/i23008/Macro_1.scala} (77%) create mode 100644 tests/neg-macros/i23008/Test_2.scala delete mode 100644 tests/run-macros/i23008.check delete mode 100644 tests/run-macros/i23008/Test_2.scala diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index b5b62816a448..13f144d45122 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -2420,7 +2420,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler object StringConstant extends StringConstantModule: def apply(x: String): StringConstant = - require(x != null, "StringConstant cannot be null") + require(x != null, "value of StringConstant cannot be `null`") // A `null` constant must be represented as a `NullConstant`, c.f. a // constant with `tag == NullTag`, which is not a `StringConstant`. // See issue 23008. diff --git a/library/src/scala/quoted/Quotes.scala b/library/src/scala/quoted/Quotes.scala index 544e9119a770..70d09b7e4841 100644 --- a/library/src/scala/quoted/Quotes.scala +++ b/library/src/scala/quoted/Quotes.scala @@ -3556,7 +3556,10 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching => /** Methods of the module object `val StringConstant` */ trait StringConstantModule { this: StringConstant.type => - /** Create a constant String value */ + /** Create a constant String value + * + * @throw `IllegalArgumentException` if the argument is `null` + */ def apply(x: String): StringConstant /** Match String value constant and extract its value */ def unapply(constant: StringConstant): Some[String] diff --git a/library/src/scala/quoted/ToExpr.scala b/library/src/scala/quoted/ToExpr.scala index e9cd06fd7ac9..f9f3d00bcedf 100644 --- a/library/src/scala/quoted/ToExpr.scala +++ b/library/src/scala/quoted/ToExpr.scala @@ -77,13 +77,7 @@ object ToExpr { given StringToExpr[T <: String]: ToExpr[T] with { def apply(x: T)(using Quotes) = import quotes.reflect.* - val literal = - if (x: Any) == null then - // Can happen if called from code without `-Yexplicit-nulls`. - Literal(NullConstant()) - else - Literal(StringConstant(x)) - literal.asExpr.asInstanceOf[Expr[T]] + Literal(StringConstant(x)).asExpr.asInstanceOf[Expr[T]] } /** Default implementation of `ToExpr[Class[T]]` */ diff --git a/tests/neg-macros/i23008.check b/tests/neg-macros/i23008.check new file mode 100644 index 000000000000..c48fdb2fe6fc --- /dev/null +++ b/tests/neg-macros/i23008.check @@ -0,0 +1,21 @@ + +-- Error: tests/neg-macros/i23008/Test_2.scala:1:24 -------------------------------------------------------------------- +1 |@main def Test = Macros.buildString // error + | ^^^^^^^^^^^^^^^^^^ + | Exception occurred while executing macro expansion. + | java.lang.IllegalArgumentException: requirement failed: value of StringConstant cannot be `null` + | at scala.Predef$.require(Predef.scala:337) + | at scala.quoted.runtime.impl.QuotesImpl$reflect$StringConstant$.apply(QuotesImpl.scala:2533) + | at scala.quoted.runtime.impl.QuotesImpl$reflect$StringConstant$.apply(QuotesImpl.scala:2532) + | at scala.quoted.ToExpr$StringToExpr.apply(ToExpr.scala:80) + | at scala.quoted.ToExpr$StringToExpr.apply(ToExpr.scala:78) + | at scala.quoted.Expr$.apply(Expr.scala:70) + | at Macros$.buildStringCode(Macro_1.scala:9) + | + |--------------------------------------------------------------------------------------------------------------------- + |Inline stack trace + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + |This location contains code that was inlined from Macro_1.scala:4 +4 | inline def buildString = ${buildStringCode} + | ^^^^^^^^^^^^^^^^^^ + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/run-macros/i23008/Macros_1.scala b/tests/neg-macros/i23008/Macro_1.scala similarity index 77% rename from tests/run-macros/i23008/Macros_1.scala rename to tests/neg-macros/i23008/Macro_1.scala index 052760dbf833..67bd091cd9f3 100644 --- a/tests/run-macros/i23008/Macros_1.scala +++ b/tests/neg-macros/i23008/Macro_1.scala @@ -6,7 +6,6 @@ object Macros { def buildStringCode(using Quotes): Expr[String] = { import quotes.reflect.* val str: String = null - val exprString = Expr(str) - Expr(exprString.show) + Expr(str) } } diff --git a/tests/neg-macros/i23008/Test_2.scala b/tests/neg-macros/i23008/Test_2.scala new file mode 100644 index 000000000000..5ebf2715cf14 --- /dev/null +++ b/tests/neg-macros/i23008/Test_2.scala @@ -0,0 +1 @@ +@main def Test = Macros.buildString // error diff --git a/tests/run-macros/i23008.check b/tests/run-macros/i23008.check deleted file mode 100644 index 19765bd501b6..000000000000 --- a/tests/run-macros/i23008.check +++ /dev/null @@ -1 +0,0 @@ -null diff --git a/tests/run-macros/i23008/Test_2.scala b/tests/run-macros/i23008/Test_2.scala deleted file mode 100644 index cf3cfffb50ac..000000000000 --- a/tests/run-macros/i23008/Test_2.scala +++ /dev/null @@ -1,2 +0,0 @@ -@main def Test = - println(Macros.buildString)