Skip to content

Make compilable with -Xcheck-macros #278

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

OndrejSpanel
Copy link
Contributor

At this moment the PR only enables -Xcheck-macros. The library does not compile, there are 5 errors.

Once there is:

Malformed tree was found while expanding macro with -Xcheck-macros.
               |The tree does not conform to the compiler's tree invariants.
               |
               |Macro was:
               |scala.quoted.runtime.Expr.splice[com.softwaremill.quicklens.PathModify[com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1, scala.Int]](((evidence$1: scala.quoted.Quotes) ?=> com.softwaremill.quicklens.QuicklensMacros.toPathModifyFromFocus[com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1, scala.Int](scala.quoted.runtime.Expr.quote[com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1](ab).apply(using evidence$1), scala.quoted.runtime.Expr.quote[scala.Function1[com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1, scala.Int]](((_$3: com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1) => _$3.a)).apply(using evidence$1))(scala.quoted.Type.of[com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1](evidence$1), scala.quoted.Type.of[scala.Int](evidence$1), evidence$1)))
               |
               |The macro returned:
               |com.softwaremill.quicklens.PathModify.apply[com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1, scala.Int](ab, ((x: scala.Function1[scala.Int, scala.Int]) => (ab.copy(a = x.apply(ab.a)): com.softwaremill.quicklens.ModifyAndTypeTest.B & com.softwaremill.quicklens.ModifyAndTypeTest.A1)))
               |
               |Error:
               |assertion failed: Found:    com.softwaremill.quicklens.ModifyAndTypeTest.A1
Required: com.softwaremill.quicklens.ModifyAndTypeTest.B &
  com.softwaremill.quicklens.ModifyAndTypeTest.A1
found: class A1 in object ModifyAndTypeTest with class A1, flags = case <noinits> <touched>, underlying = com.softwaremill.quicklens.ModifyAndTypeTest.A1, Object with Product with Serializable {...}
expected: ??
tree = ab.copy(a = x.apply(ab.a))

And four times there are variants of:

Exception occurred while executing macro expansion.
java.lang.AssertionError: Reference to a method must be eta-expanded before it is used as an expression: com.softwaremill.quicklens.TestData.aPoly.copy$default$2
	at scala.collection.immutable.List.foreach(List.scala:334)
	at scala.quoted.runtime.impl.QuotesImpl$reflect$.scala$quoted$runtime$impl$QuotesImpl$reflect$$$xCheckMacroValidExprs(QuotesImpl.scala:3111)
	at scala.quoted.runtime.impl.QuotesImpl$reflect$Apply$.apply(QuotesImpl.scala:645)
	at scala.quoted.runtime.impl.QuotesImpl$reflect$Apply$.apply(QuotesImpl.scala:643)
	at com.softwaremill.quicklens.QuicklensMacros$.callMethod$1$$anonfun$3(QuicklensMacros.scala:266)
	at scala.collection.LinearSeqOps.foldLeft(LinearSeq.scala:183)
	at scala.collection.LinearSeqOps.foldLeft$(LinearSeq.scala:179)
	at scala.collection.immutable.List.foldLeft(List.scala:79)
	at com.softwaremill.quicklens.QuicklensMacros$.callMethod$1(QuicklensMacros.scala:266)
...
    aPoly.modify(_.poly).using(duplicate) should be(aPolyDup)

Hopefully solving those should not be very difficult?

@KacperFKorban
Copy link
Collaborator

@OndrejSpanel Do you want to try fixing those or should I take a look at it?

@OndrejSpanel
Copy link
Contributor Author

I was trying to fix the second error (Reference to a method must be eta-expanded before it is used as an expression) fixed for default values, but I was not successful so far. I was trying to add the expansion by adding .etaExpand(Symbol.spliceOwner) conditionally before obj.select(methodSymbol).appliedToIfNeeded(defaultMethodArgs) and argsHandled.foldLeft(methodApplied)(Apply(_, _)), but no luck so far. If you can take over, I will be very glad.

@adamw
Copy link
Member

adamw commented Apr 12, 2025

Out of curiosity - are these bugs in quicklens, or how are the macros checked?

@KacperFKorban
Copy link
Collaborator

They are macro related. In practice this means, that they break some invariants about the generated code, to it could potentially cause bugs for some edge cases. Though, IMHO it is probably very unlikely for the second one to cause any bugs.

@OndrejSpanel
Copy link
Contributor Author

Though, IMHO it is probably very unlikely for the second one to cause any bugs.

The trouble is once any macro library used in the project causes -Xcheck-macros failures, it is quite difficult to check the rest of the project. I was motivated to open this by the fact my project is crashing with 3.7.0 because of some macro bug in https://github.com/OpenGrabeso/light-surface/.

@KacperFKorban
Copy link
Collaborator

The trouble is once any macro library used in the project causes -Xcheck-macros failures, it is quite difficult to check the rest of the project.
100% agree, I think that utest might still fail with "-Xcheck-macros"

@adamw
Copy link
Member

adamw commented Apr 13, 2025

I guess there's also the non-zero chance that since we are producing working code, the invariants are somehow too strict? :)

@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented Apr 13, 2025

since we are producing working code, the invariants are somehow too strict?

Maybe, or maybe you are in the Undefined Behaviour territory and anything can happen (including working code). This seems to be the case with the crash caused by the https://github.com/OpenGrabeso/light-surface/ - it was also producing a working code (with plenty of -Xcheck-macros violations), but the code is not working anymore with 3.7 compiler.

I have tried to understand what exactly is violated in quicklens and how to work around that, but after spending about an hour I did not achieve much.

@adamw
Copy link
Member

adamw commented Apr 13, 2025

Ah yes, undefined behavior makes more sense:)

@KacperFKorban
Copy link
Collaborator

Looks like one of the errors was actually an obvious unsoundness 🙃

@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented Apr 14, 2025

You can use following to add settings for Scala3 only:

  scalacOptions ++= if (CrossVersion.partialVersion(scalaVersion.value).exists(_ >= (3, 0)) Seq("-Xcheck-macros") else Seq()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants