Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange Error while emitting error when adding a new member to Surface #20072

Closed
OndrejSpanel opened this issue Apr 2, 2024 · 7 comments
Closed
Labels
area:metaprogramming:quotes Issues related to quotes and splices itype:bug stat:needs minimization Needs a self contained minimization

Comments

@OndrejSpanel
Copy link
Member

OndrejSpanel commented Apr 2, 2024

Compiler version

3.3.3
3.4.1
3.5.0-RC1-bin-20240331-cc55381-NIGHTLY

Minimized code

Output

java.util.NoSuchElementException: val rawType$2
        at scala.collection.mutable.AnyRefMap$ExceptionDefault.apply(AnyRefMap.scala:508)
        at scala.collection.mutable.AnyRefMap$ExceptionDefault.apply(AnyRefMap.scala:507)
        at scala.collection.mutable.AnyRefMap.apply(AnyRefMap.scala:207)
        at dotty.tools.backend.jvm.BCodeSkelBuilder$PlainSkelBuilder$locals$.load(BCodeSkelBuilder.scala:583)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genLoadTo(BCodeBodyBuilder.scala:440)

See https://github.com/OpenGrabeso/light-surface/actions/runs/8525399157/job/23352226318

Expectation

No error. The code compiles fine when you comment out override val docString: Option[String] = None, and there is no reason why it should not compile with it.

This is driving me crazy. I have made many changes to this class before and I never had any issues with it, it is based on https://wvlet.org/airframe/docs/airframe-surface

@OndrejSpanel OndrejSpanel added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 2, 2024
@OndrejSpanel
Copy link
Member Author

OndrejSpanel commented Apr 2, 2024

Another repro of the same issue, might be a better starting point for possible minification, is branch docstring-def in the same repo.

The code compiles fine in the commit before the last one, it crashes as soon as you comment typeArgs = Seq.empty as seen in the last comment.

The critical code fragment is:

      '{
        new org.opengrabeso.airframe.surface.GenericSurface(
          ${ clsOf(t) },
          //typeArgs = Seq.empty,
          params = Seq.empty,
          docString = ${ doc }
        )
      }

It seems you need to provide all arguments to GenericSurface in the quoted expression in spite of them having defaults, otherwise the compiler crashes. This also provides me with a workaround - always assign all parameters of GenericSurface explicitly.

Rant: I have worked with Scala 2 macros before, and while design of Scala 3 macros looks elegant and powerful, the implementation is ridden with so many issues that doing anything with them is very frustrating. Esp. the error messages are most often not helpful, crashes happen somewhere inside of the compiler, callstacks do not include the inlining chains. Following quote sounds more like a dream:

Scala 3 introduces a new principled approach of metaprogramming that is designed for stability

@Gedochao Gedochao added stat:needs minimization Needs a self contained minimization area:metaprogramming:quotes Issues related to quotes and splices and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 3, 2024
@OndrejSpanel
Copy link
Member Author

OndrejSpanel commented Apr 3, 2024

I am trying to reduce the repro. Creating a small repro from scratch did not work, therefore I remove parts of code from the original repro and I am gradually reducing its size.

@OndrejSpanel
Copy link
Member Author

This is how the generated expression looks like with the explicit argument present. The value is obtained by logging println(s"[${t.show}] ${surface.show}") inside of surfaceOf function:

new org.opengrabeso.airframe.surface.GenericSurface(classOf[X], typeArgs = scala.Seq.empty[scala.Nothing], docString = scala.None)

and this is how it looks without it, using the default value:

{
  val rawType$1: java.lang.Class[_ >: scala.Nothing <: scala.Any] = classOf[X]
  val docString$1: scala.None.type = scala.None
  val typeArgs$1: scala.collection.immutable.Seq[org.opengrabeso.airframe.surface.Surface] @scala.annotation.unchecked.uncheckedVariance = org.opengrabeso.airframe.surface.GenericSurface.$lessinit$greater$default$2
  new org.opengrabeso.airframe.surface.GenericSurface(rawType$1, typeArgs$1, docString = docString$1)
}

In the second case the compiler seems to insert some helper variables which then fail for some obscure reason.

@OndrejSpanel
Copy link
Member Author

The repro is reduced to a single file, 160 lines, and a test file. I doubt I can reduce it much more, in particular I cannot remove seen. and memo caching mechanism in surfaceOf.

@OndrejSpanel
Copy link
Member Author

OndrejSpanel commented Apr 3, 2024

I can create the error even without using default values. This gives the error:

      val surface = '{
        val none = None
        new org.opengrabeso.airframe.surface.Surface(none)
      }

This does not:

      val surface = '{
        val none = None
        new org.opengrabeso.airframe.surface.Surface(None)
      }

Also no error happens when I replace the Flags.Lazy with Flags.EmptyFlags. The complete expression which causes the failure is:

{
  lazy val __A_X_s_A_000: org.opengrabeso.airframe.surface.Surface = {
    val none: scala.None.type = scala.None
    new org.opengrabeso.airframe.surface.Surface(none)
  }
  Seq(__A_X_s_A_000)
}

When I provide the same expression explicitly, no error happens.

When I use -Xcheck-macros, I get following assertion:

[error]    |java.lang.AssertionError: assertion failed: Tree had an unexpected owner for val none
[error]    |Expected: val __A_X_s_A_000 (org.opengrabeso.airframe.surface.MethodSurfaceTest._$_$_$__A_X_s_A_000)
[error]    |But was: val macro (org.opengrabeso.airframe.surface.MethodSurfaceTest._$_$macro)

I am not sure what this all means.

@OndrejSpanel
Copy link
Member Author

The issue can be fixed by using changeOwner:

ValDef(sym, Some(surfaceOf(tpe, useVarRef = false).asTerm.changeOwner(sym)))

xerial pushed a commit to wvlet/airframe that referenced this issue Apr 6, 2024
Fixes issue investigated as scala/scala3#20072

Unfortunately there are still other issues preventing compiling with
`-Xcheck-macros`
(https://stackoverflow.com/questions/78193025/how-to-extract-type-parameter-from-typelambda-correctly),
but this fix is an important step in that direction.
@OndrejSpanel
Copy link
Member Author

Caused by a bad owner on the macro side, not a compiler issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metaprogramming:quotes Issues related to quotes and splices itype:bug stat:needs minimization Needs a self contained minimization
Projects
None yet
Development

No branches or pull requests

2 participants