Skip to content

Commit

Permalink
Surface (fix): __sXXX is a forward reference extending over the defin…
Browse files Browse the repository at this point in the history
…ition of __sYYY in methodsOf (#3451)

In some complex classes I get errors like "__s239 is a forward reference
extending over the definition of __s041" when using `methodsOf` in Scala
3 on them.

This is because `CompileTimeSurfaceFactory.methodsOf` lists the methods
in the `seen` order. Some of them are declared as `val`, some as `lazy
val`. While `lazy val` can reference each other freely, they cannot do
so across a barrier of `val`.

This change makes sure the `lazy val`s (non-lazy surfaces) are listed
first.

I have verified this fixes the issue for my complex class (about 400
members). I will try to create a standalone open-source repro later if
possible - this will probably need some mixture of methods with lazy and
non-lazy surfaces used for types.

I did not check Scala 2 implementation as I am not using Surface on
Scala 2 - it is possible it experiences the same issue. If there is a
test, this should show up.

---------

Co-authored-by: Taro L. Saito <[email protected]>
  • Loading branch information
OndrejSpanel and xerial authored Mar 18, 2024
1 parent 4dd6d2c commit b42a1a9
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,12 @@ private[surface] class CompileTimeSurfaceFactory[Q <: Quotes](using quotes: Q):
.toSeq
// Exclude primitive surfaces as it is already defined in Primitive object
.filterNot(x => primitiveTypeFactory.isDefinedAt(x._1))
.sortBy(_._2)
.map { (tpe, order) =>
// first list all lazy vals, otherwise there is a risk of forward reference error across strict vals
(tpe, (!lazySurface.contains(tpe), order))
}.sortBy(_._2)
.reverse
.map { case (tpe, order) =>
.foreach { (tpe, order) =>
// Update the cache so that the next call of surfaceOf method will use the local varaible reference
surfaceToVar += tpe -> Symbol.newVal(
Symbol.spliceOwner,
Expand Down
40 changes: 40 additions & 0 deletions airframe-surface/src/test/scala/wvlet/airframe/surface/i3451.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package wvlet.airframe.surface

import wvlet.airspec.AirSpec

object i3451 extends AirSpec {

case class Cons(head: String, tail: Cons)
case class TypedCons[A](head: Int, tail: TypedCons[A])

trait Recursive[T <: Recursive[T]]

case class E(x: Int) extends Recursive[E] {
def cons(c: Cons): Cons = c
def typedCons(c: TypedCons[_]): TypedCons[_] = c
def r3(r: Recursive[_]): Recursive[_] = r
}

test("Support methods with lazy and non-lazy types mixed in any order") {
val s = Surface.of[E]
debug(s.params)
val m = Surface.methodsOf[E]
val names = m.map(_.name)
names shouldContain "cons"
names shouldContain "typedCons"
names shouldContain "r3"
}
}

0 comments on commit b42a1a9

Please sign in to comment.