wvlet / airframe

Essential Building Blocks for Scala
https://wvlet.org/airframe
Apache License 2.0
631 stars 65 forks source link

Surface: __sXXX is a forward reference extending over the definition of __sYYY in methodsOf #3451

Closed OndrejSpanel closed 5 months ago

OndrejSpanel commented 5 months ago

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 vals (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.

OndrejSpanel commented 5 months ago

I will try to create a standalone open-source repro

Surface on Scala 2 - it is possible it experiences the same issue. If there is a test, this should show up.

The repro is done. It fails before the fix - and it fails on Scala 2.13. I will try to implement a Scala 2 fix as well.

OndrejSpanel commented 5 months ago

Scala 2 failure was different, it was because its methods contain productElementNames, which I did not expect. Other than that, it seems to work fine (at least with this repro). I have relaxed the test and it is passing now.

xerial commented 5 months ago

Thanks for the fix! Merged