Skip to content

Commit

Permalink
Add a check for correct Array shape in quotes.reflect.ClassOfConstant (
Browse files Browse the repository at this point in the history
…#22033)

Closes #21916
I tried to supply the ClassOfConstant with multiple other broken Types,
but I was unable to break it beyond the linked issue, so I ended up
adding the check for only that one case. This makes sense - the backend
(and thus erasure) needs to know if the Array type parameter is a
primitive type, but in other cases the erasure phase needs to know only
the class, without the type parameters.

It's impossible to call classOf through the quoted code (`'{classOf[t]}`
with a boundless t will error out), so we don't need that additional
check there.

There does appear to be an issue with being able to set `'{List[Array]}`
resulting in a crash, but that is beyond the scope of this fix - I will
prepare a separate issue for that (edit: reported
[here](#22034)).
  • Loading branch information
hamzaremmal authored Feb 21, 2025
2 parents d4421d0 + 80ae408 commit 595c5d7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 1 deletion.
12 changes: 11 additions & 1 deletion compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2530,7 +2530,17 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler

object ClassOfConstant extends ClassOfConstantModule:
def apply(x: TypeRepr): ClassOfConstant =
// TODO check that the type is a valid class when creating this constant or let Ycheck do it?
// We only check if the supplied TypeRepr is valid if it contains an Array,
// as so far only that Array could cause issues
def correctTypeApplicationForArray(typeRepr: TypeRepr): Boolean =
val isArray = typeRepr.typeSymbol != dotc.core.Symbols.defn.ArrayClass
typeRepr match
case AppliedType(_, targs) if !targs.isEmpty => true
case _ => isArray
xCheckMacroAssert(
correctTypeApplicationForArray(x),
"Illegal empty Array type constructor. Please supply a type parameter."
)
dotc.core.Constants.Constant(x)
def unapply(constant: ClassOfConstant): Some[TypeRepr] = Some(constant.typeValue)
end ClassOfConstant
Expand Down
15 changes: 15 additions & 0 deletions tests/neg-macros/i21916.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

-- Error: tests/neg-macros/i21916/Test_2.scala:3:27 --------------------------------------------------------------------
3 |@main def Test = Macro.test() // error
| ^^^^^^^^^^^^
| Exception occurred while executing macro expansion.
| java.lang.AssertionError: Illegal empty Array type constructor. Please supply a type parameter.
| at Macro$.testImpl(Macro_1.scala:8)
|
|---------------------------------------------------------------------------------------------------------------------
|Inline stack trace
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
|This location contains code that was inlined from Macro_1.scala:3
3 | inline def test() = ${testImpl}
| ^^^^^^^^^^^
---------------------------------------------------------------------------------------------------------------------
9 changes: 9 additions & 0 deletions tests/neg-macros/i21916/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import scala.quoted._
object Macro:
inline def test() = ${testImpl}
def testImpl(using Quotes): Expr[Any] = {
import quotes.reflect._
val tpe = TypeRepr.of[Array[Byte]] match
case AppliedType(tycons, _) => tycons
Literal(ClassOfConstant(tpe)).asExpr
}
3 changes: 3 additions & 0 deletions tests/neg-macros/i21916/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// lack of type ascription is on purpose,
// as that was part of what would cause the crash before.
@main def Test = Macro.test() // error

0 comments on commit 595c5d7

Please sign in to comment.