scala/scala3

Unable to match a lower-case constructor in quoted pattern

Closed this issue · 15 comments

Compiler version

3.6.3

Minimized code

final case class caseName(name: String) extends scala.annotation.Annotation
object caseName {

  given FromExpr[caseName] =
    new FromExpr[caseName] {
      override def unapply(x: Expr[caseName])(using Quotes): Option[caseName] =
        x match {
          case '{ caseName(${ Expr(name) }) }     => caseName(name).some
          // with/without the following line...
          case '{ new caseName(${ Expr(name) }) } => caseName(name).some
          case _                                  => println(x.show); None
        }
    }

}
sealed trait SealedTrait3[+A, +B] derives Show
object SealedTrait3 {
  final case class AB1[+B, +A](a: B, b: A) extends SealedTrait3[B, A]
  final case class AB2[+C, +D](a: C, b: D) extends SealedTrait3[D, C]
  final case class A[+A](a: A) extends SealedTrait3[A, Nothing]
  @caseName("_B_") final case class B[+B](b: B) extends SealedTrait3[Nothing, B]
  case object Neither extends SealedTrait3[Nothing, Nothing]
}
    def optionalAnnotation[Annot: Type]: Option[Expr[Annot]] = {
      val annotTpe = TypeRepr.of[Annot]
      val annotFlags = annotTpe.typeSymbol.flags

      if (annotFlags.is(Flags.Abstract) || annotFlags.is(Flags.Trait))
        report.errorAndAbort(s"Bad annotation type ${annotTpe.show} is abstract")

      this.getAnnotation(annotTpe.typeSymbol) match
        case Some(tree) if tree.tpe <:< annotTpe => tree.asExprOf[Annot].some
        case _                                   => None
    }

    def requiredAnnotation[Annot: Type]: Expr[Annot] =
      optionalAnnotation[Annot].getOrElse(report.errorAndAbort(s"Missing required annotation `${TypeRepr.of[Annot].show}` for `$this`"))

    def optionalAnnotationValue[Annot: {Type, FromExpr}]: Option[Annot] =
      optionalAnnotation[Annot].map { expr =>
        expr.value.getOrElse(report.errorAndAbort(s"Found annotation `${TypeRepr.of[Annot].show}` for `$this`, but are unable to extract Expr.value\n${expr.show}"))
      }

    def requiredAnnotationValue[Annot: {Type, FromExpr}]: Annot = {
      val expr = requiredAnnotation[Annot]
      expr.value.getOrElse(report.errorAndAbort(s"Found annotation `${TypeRepr.of[Annot].show}` for `$this`, but are unable to extract Expr.value\n${expr.show}"))
    }

Output

[error]    |Found annotation `oxygen.meta.example.caseName` for `oxygen.meta.example.SealedTrait3.B[B]`, but are unable to extract Expr.value
[error]    |new oxygen.meta.example.caseName("_B_")

then add a case for that in the FromExpr, and...

[error] 28 |          case '{ new caseName(${ Expr(name) }) } => caseName(name).some
[error]    |                      ^^^^^^^^
[error]    |                      caseName does not have a constructor

Expectation

That either:

  1. case '{ caseName(${ Expr(name) }) } => caseName(name).some matches
  2. case '{ new caseName(${ Expr(name) }) } => caseName(name).some is allowed, and matches

Quite difficult to handle when you see "we found this, but cant match on it", and then when you try to match on the exact expr.show, it says that expr is not valid?

I don't know the details, but a recent conversation about annotation includes #22553

I can have a look; @Gedochao can we assign this to me for this cycle?

Thanks @mbovel!
Also, reading the description for scala.annotation.ConstantAnnotation, it really seems like for any A <: ConstantAnnotation, the compiler should be providing a given FromExpr[A]. Im curious your thoughts on this, and whether that might be doable here as well?

I guess, generally it would be nice if the compiler provided a built-in mechanism for deriving FromExpr[A] for any product/sum type, but I believe it would be correct to say that a correctly written auto-derive for a ConstantAnnotation should always return Some[A]?

The problem does not seem linked to annotations or classes extending annotations. It seems to be linked to classes with lower-case names.

Compiles successfully:

import scala.quoted.{Expr, Quotes}

class Foo(name: String) extends scala.annotation.StaticAnnotation

object Macro:
  inline def myMacro3(): Unit =
    ${ myMacroImpl3('{new Foo("hello")}) }

  def myMacroImpl3(x: Expr[Foo])(using q: Quotes): Expr[Unit] =
    x match
      case '{new Foo(${Expr(name)})} =>
        println(name)
      case _ =>
        println("not a Foo")

    '{()}
$ scala compile --server=false -S 3.6.3 22616
hello

Errors out:

import scala.quoted.{FromExpr, Expr, Quotes}

class foo(name: String)

object Macro:
  inline def myMacro3(): Unit =
    ${ myMacroImpl3('{new foo("hello")}) }

  def myMacroImpl3(x: Expr[foo])(using q: Quotes): Expr[Unit] =
    x match
      case '{new foo(${Expr(name)})} =>
        println(name)
      case _ =>
        println("not a foo")

    '{()}
$ scala compile --server=false -S 3.6.3 22616
-- Error: /localhome/bovel/scala3/22616/defs.scala:11:17 -------------------------------------------------------------------------------------------------------------------------
11 |      case '{new foo(${Expr(name)})} =>
   |                 ^^^
   |                 foo does not have a constructor
-- Error: /localhome/bovel/scala3/22616/defs.scala:11:27 -------------------------------------------------------------------------------------------------------------------------
11 |      case '{new foo(${Expr(name)})} =>
   |                       ^^^^^^^^^^
   |                       Type must be fully defined.
   |                       Consider annotating the splice using a type ascription:
   |                         (${Expr(name)}: XYZ).
-- [E006] Not Found Error: /localhome/bovel/scala3/22616/defs.scala:12:16 --------------------------------------------------------------------------------------------------------
12 |        println(name)
   |                ^^^^
   |                Not found: name - did you mean Some?
   |
   | longer explanation available when compiling with `-explain`
3 errors found
Compilation failed
$ scala compile -Ydebug-error --server=false -S 3.6.3 22616
-- Error: /localhome/bovel/scala3/22616/defs.scala:11:17 -------------------------------------------------------------------------------------------------------------------------
11 |      case '{new foo(${Expr(name)})} =>
   |                 ^^^
   |                 foo does not have a constructor
java.lang.Exception: Stack trace
        at java.base/java.lang.Thread.dumpStack(Thread.java:1389)
        at dotty.tools.dotc.report$.error(report.scala:70)
        at dotty.tools.dotc.typer.ErrorReporting$.errorType(ErrorReporting.scala:32)
        at dotty.tools.dotc.typer.TypeAssigner.notAMemberErrorType(TypeAssigner.scala:186)
        at dotty.tools.dotc.typer.TypeAssigner.notAMemberErrorType$(TypeAssigner.scala:16)
        at dotty.tools.dotc.typer.Typer.notAMemberErrorType(Typer.scala:148)
        at dotty.tools.dotc.typer.Typer.reportAnError$1(Typer.scala:897)
        at dotty.tools.dotc.typer.Typer.typedSelectWithAdapt(Typer.scala:911)
        at dotty.tools.dotc.typer.Typer.typeSelectOnTerm$1(Typer.scala:996)
        at dotty.tools.dotc.typer.Typer.typedSelect(Typer.scala:1034)
        at dotty.tools.dotc.typer.Typer.typedNamed$1(Typer.scala:3475)
        at dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3584)
        at dotty.tools.dotc.typer.Typer.typed(Typer.scala:3662)
        at dotty.tools.dotc.typer.Typer.typed(Typer.scala:3666)
        at dotty.tools.dotc.typer.Typer.typedExpr(Typer.scala:3777)
        at dotty.tools.dotc.typer.Applications.realApply$1(Applications.scala:1051)
        at dotty.tools.dotc.typer.Applications.typedApply(Applications.scala:1244)
        at dotty.tools.dotc.typer.Applications.typedApply$(Applications.scala:434)
        at dotty.tools.dotc.typer.Typer.typedApply(Typer.scala:148)
        at dotty.tools.dotc.typer.Typer.typedUnnamed$1(Typer.scala:3500)
        at dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3585)
        at dotty.tools.dotc.typer.Typer.typed(Typer.scala:3662)
        at dotty.tools.dotc.typer.Typer.typed(Typer.scala:3666)
        at dotty.tools.dotc.typer.Typer.typedExpr(Typer.scala:3777)
        at dotty.tools.dotc.typer.QuotesAndSplices.typedQuotePattern(QuotesAndSplices.scala:286)
        at dotty.tools.dotc.typer.QuotesAndSplices.typedQuote(QuotesAndSplices.scala:56)
        at dotty.tools.dotc.typer.QuotesAndSplices.typedQuote$(QuotesAndSplices.scala:32)

Adding backticks enables the lower case version to compile as expected:

  def myMacroImpl3(x: Expr[foo])(using q: Quotes): Expr[Unit] =
    x match
      case '{new `foo`($name)} =>
        println(name)
      case _ =>
        println("not a foo")

I am not sure if the current behavior is expected or is a bug; I can't find a definitive answer in the documentation. In general, what do lower case identifiers in term positions in quoted patterns mean?

I see I can match case '{true} or case '{bar($name)}. So maybe I should also be able to match new foo($name)?

Somehow related: this page says that lower case identifiers in type positions are binders: “Just as in a normal pattern, the type variables are written using lower case names.”

cc @hamzaremmal @jchyb @nicolasstucki

Further thinking about it, I think this is a bug, and that the culprit is this condition in typedIdent:

if untpd.isVarPattern(tree) && name.isTypeName then

The condition is true when typing foo in new foo, while it probably shouldn't.

I agree it looks like a bug (especially with that error message). I also believe the type positions part in lower case identifiers in type positions are binders only relates to Typed() and TypeApply() trees (but I can't say that for sure, as I have not really delved to that part of the code yet).

I also believe the type positions part in lower case identifiers in type positions are binders only relates to Typed() and TypeApply() trees

That would make sense to me. Unfortunately, I don't think we can easily know if we are in one of these nodes when typing the nested identifier. At least, mode and prototype do not help:

ctx.mode: Mode(Type,ImplicitsEnabled,QuotedExprPattern)
pt: WildcardType(NoType)

I also believe the type positions part in lower case identifiers in type positions are binders only relates to Typed() and TypeApply() trees (but I can't say that for sure, as I have not really delved to that part of the code yet).

It might actually only be inside Typed trees, given what I observe in practice, and this sentence from the documentation

The pattern $x: t will match an expression of any type and t will be bound to the type of the pattern.

And that even seems to be too general; for example, this doesn't compile:

import scala.quoted.{FromExpr, Expr, Quotes}

case class Foo[T](x: String)

object Macro:
  inline def myMacro3(): Unit =
    ${ myMacroImpl3('{Foo[Any]("hello")}) }

  def myMacroImpl3(x: Expr[Foo[Any]])(using Quotes): Expr[Unit] =
    x match
      case '{ Foo($y: t) } =>
        '{Foo[t]($y); ()}
      case _ =>
        println("not a foo")

    '{()}
-- [E007] Type Mismatch Error: test.scala:11:18 --------------------------------
11 |      case '{ Foo($y: t) } =>
   |                  ^^^^^
   |                  Found:    t
   |                  Required: String
   |
   | longer explanation available when compiling with `-explain`
Set()
-- [E006] Not Found Error: test.scala:12:14 ------------------------------------
12 |        '{Foo[t]($y); ()}
   |              ^
   |              Not found: type t
   |
   | longer explanation available when compiling with `-explain`
-- [E006] Not Found Error: test.scala:12:18 ------------------------------------
12 |        '{Foo[t]($y); ()}
   |                  ^
   |                  Not found: y
   |
   | longer explanation available when compiling with `-explain`
3 errors found

But it does if the field x of the class Foo has type T: case class Foo[T](x: T) 🤔

It might actually only be inside Typed trees, given what I observe in practice, and this sentence from the documentation

The pattern $x: t will match an expression of any type and t will be bound to the type of the pattern.

No, it is more general than that, and must work for type arguments anywhere, including ones that are not at the top-level. See for example this test:

case '{ Field.const[a, b, tpe]($selector) } =>

The problem is really the type we give to the quoted type variable.

When we write new foo(...) as in this comment, foo becomes an abstract type type foo which doesn't have the required constructor.

The same happens when we write takeString(... : t) as in this comment: we make an abstract type t which is not a string.

So how does this work for normal patterns? The logic that deals with it is in typedBind: it forces pt to be fully defined and then create a symbol that has (hopefully) the right type.

We could do the same for quoted pattern variables. I tried something like:

isFullyDefined(pt, ForceDegree.all) 
typedQuotedTypeVar(..., TypeBounds.upper(pt))

For the second example (takeString(... : t)), that works: we create a type type t <: String instead of just type t, allowing it to compile.

However, we can't do that for the first example (new foo(...)) because in that case 1. we have pt == WildcardType, and 2. even if we had a prototype, I can't think of a fully defined upper bound that would work.

For the second example (takeString(... : t)), that works: we create a type type t <: String instead of just type t, allowing it to compile.

But for that to works, when typing e: t with an expected type pt, we'd need to type t with the same expected type, which is probably not we want in general.

We discussed this during today's compiler meeting and concluded that solving the problem in general would require significant effort, which we don't have the bandwidth for right now.

Instead, we'll:

  1. Try to emit an error for a quoted type variable after new (and suggest either using backticks or the lower-level API),
  2. Do nothing for the takeString(... : t) case.