scala3 icon indicating copy to clipboard operation
scala3 copied to clipboard

Fix quoted patterns with references to private fields

Open nicolasstucki opened this issue 3 years ago • 5 comments

Reverts 0b072d6b7df6d9c65d6ffce779c3417df42b47bd

Fixes #15676

nicolasstucki avatar Aug 09 '22 09:08 nicolasstucki

It seems that not making pattern quotes inlinable actually fixed quill, because the same part of CI failed in https://github.com/lampepfl/dotty/pull/13547 before https://github.com/lampepfl/dotty/commit/0b072d6b7df6d9c65d6ffce779c3417df42b47bd. :thinking:

[error] -- Error: /__w/dotty/dotty/community-build/community-projects/protoquill/quill-sql/src/main/scala/io/getquill/parser/Parser.scala:655:14 
[error] 655 |    case '{ ($i: InfixValue).generic.pure.as[t] } => genericInfix(i)(true, Quat.Generic)
[error]     |              ^
[error]     |              access to InfixParser.this from wrong staging level:
[error]     |               - the definition is at level 0,
[error]     |               - but the access is at level 1.
[error] one error found
[error] (quill-sql / Compile / compileIncremental) Compilation failed
[error] Total time: 24 s, completed Aug 9, 2022 11:57:12 AM

KacperFKorban avatar Aug 09 '22 11:08 KacperFKorban

To fix this we will need to minimize the failure of quill. I do not have time to do it now.

nicolasstucki avatar Aug 09 '22 11:08 nicolasstucki

OK, I'll try to minimize it then

KacperFKorban avatar Aug 09 '22 11:08 KacperFKorban

The minimized code looks like so:

package getquill

import scala.quoted.*

class InfixParser(using Quotes) {
  import quotes.reflect.*
  def attempt: Expr[Any] => Unit = {
    case '{ ($i: InfixValue).a } => ???
    case '{ ($i: InfixValue).b } => ??? // error
  }
}

private[getquill] trait InfixValue {
  def a: InfixValue
  private[getquill] def b: InfixValue
}

Error msg:

[error] ./quill-bug/_1.scala:9:15: access to InfixParser.this from wrong staging level:
[error]  - the definition is at level 0,
[error]  - but the access is at level 1.
[error]     case '{ ($i: InfixValue).b } => ???
[error]               ^

An accessor is generated for b, since it is qualified private and then it is accessed as this.inline$b$i1 in the case pattern.

KacperFKorban avatar Aug 09 '22 13:08 KacperFKorban

Minimized the quill example furder

package foo

class Test:
  import scala.quoted.*
  def test(i: Expr[A])(using Quotes): Expr[Unit] = '{ $i.b }

trait A:
  private[foo] def b: Unit
5 |  def test(i: Expr[A])(using Quotes): Expr[Unit] = '{ $i.b }
  |                                                      ^
  |                             access to Test.this from wrong staging level:
  |                              - the definition is at level 0,
  |                              - but the access is at level 1.

The code after typer is

package foo {
  class Test() extends Object() {
    import scala.quoted.*
    def test(i: quoted.Expr[foo.A])(using x$2: quoted.Quotes): quoted.Expr[Unit]
       = 
    '{
      this.inline$b$i1(
        ${
          {
            def $anonfun(using evidence$1: quoted.Quotes): quoted.Expr[foo.A] = 
              i
            closure($anonfun)
          }
        }
      )
    }.apply(x$2)
    def inline$b$i1(x$0: foo.A): Unit = x$0.b
  }
  trait A() extends Object {
    private[foo] def b: Unit
  }
}

Note that inline$b$i1 is added in the Test class. This is why we have the extra Test.this reference. This method should be added in some a static scope to work properly. This is related to #13215.

nicolasstucki avatar Aug 11 '22 11:08 nicolasstucki

Is this still being worked on, or are you waiting for another review @nicolasstucki? Just wanting to know if I should mark this as a draft or not.

ckipp01 avatar May 09 '23 18:05 ckipp01

We need #16992 to fix this issue

nicolasstucki avatar May 10 '23 18:05 nicolasstucki

What's the state of this PR? Can it be restored with added @publicInBinary or we're waiting until it leaves experimental stage?

WojciechMazur avatar Feb 06 '24 13:02 WojciechMazur

What's the state of this PR? Can it be restored with added @publicInBinary or we're waiting until it leaves experimental stage?

Not sure, I will need to reinvestagate the current interaction of @publicInBinary and this fix.

nicolasstucki avatar Feb 07 '24 14:02 nicolasstucki