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

Detect macro dependencies that are missing from the classloader #20139

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

bishabosha
Copy link
Member

@bishabosha bishabosha commented Apr 9, 2024

So the situation is basically that DFiant and html.scala projects do not work "out of the box" with pipelining, and will need to tune their builds if they want some pipelining.

However, the compiler reports an error that is not helpful to the user, so in this PR we report a better one.

Previously, it was assumed that a missing class (that is valid in current run)
during macro evaluation was due to the symbol being defined in the same project.
If this condition is met, then compilation is suspended.

This assumption breaks when the symbol comes from the classpath, but without a
corresponding class file, leading a situation where the same file is always suspended,
until it is the only one left, leading to the "cyclic macro dependencies" error.
In this case we should assume that the class file will never become available because class path entries
are supposed to be immutable. Therefore we should not suspend in this case.

This commit therefore detects this situation. Instead of suspending the unit,
the compiler aborts the macro expansion, reporting an error that
the user will have to deal with - likely by changing the build definition/

In the end, users will see something like:

[error] -- Error: /Users/jamie/workspace/DFiant/core/src/main/scala/dfhdl/core/DFVector.scala:163:25
[error] 163 |            val idxVal = d"$i".asConstOf[DFUInt[Int]]
[error]     |                         ^^^^^
[error]     |Macro code depends on trait DFType in package dfhdl.compiler.ir found on the classpath, but could not be loaded while evaluating the macro.
[error]     |  This is likely because class files could not be found in the classpath entry for the symbol.
[error]     |
[error]     |  A possible cause is if the origin of this symbol was built with pipelined compilation;
[error]     |  in which case, this problem may go away by disabling pipelining for that origin.
[error]     |
[error]     |  trait DFType is defined in file /Users/jamie/workspace/DFiant/compiler/ir/target/scala-3.5.0-RC1-bin-SNAPSHOT/early/dfhdl-compiler-ir_3-0.3.7+15-05c25215+20240409-1626-SNAPSHOT.jar(dfhdl/compiler/ir/DFType.tasty)
[error]     |---------------------------------------------------------------------------
[error]     |Inline stack trace
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from DFDecimal.scala:411
[error] 411 |          ${ applyMacro('sc, 'args) }
[error]     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]      ---------------------------------------------------------------------------

and

[error] -- Error: /Users/jamie/workspace/html.scala/html/target/scala-3.5.0-RC1-bin-SNAPSHOT/src_managed/test/sbt-example-generated.scala:15:52
[error]  15 |        val span: Binding.Stable[HTMLSpanElement] = html"<span>The quick ${color.bind} fox jumps&nbsp;over the lazy $animal</span>"
[error]     |                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |Macro code depends on missing object Definitions in package com.yang_bo.html found on the classpath, but could not be loaded while evaluating the macro.
[error]     |  This is likely because class files could not be found in the classpath entry for the symbol.
[error]     |
[error]     |  A possible cause is if the origin of this symbol was built with pipelined compilation;
[error]     |  in which case, this problem may go away by disabling pipelining for that origin.
[error]     |
[error]     |object Definitions is defined in file /Users/jamie/workspace/html.scala/html-Definitions/target/scala-3.5.0-RC1-bin-SNAPSHOT/early/html-definitions_sjs1_3-3.0.3+0-d627afe0+20240409-1532.jar(com/yang_bo/html/Definitions.tasty)
[error]     |---------------------------------------------------------------------------
[error]     |Inline stack trace
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from package.scala:712
[error] 712 |      ${
[error]     |      ^
[error] 713 |        Macros.html('stringContext, 'args)
[error] 714 |      }
[error]      ---------------------------------------------------------------------------

which is more actionable.

Note that sbt already automatically disables pipelining on projects that define macros, but this is not useful if the macro itself depends on upstream projects that do not define macros. This is probably a hard problem to detect automatically - so this is good compromise.

We also fix -Xprint-suspension, which appeared to swallow a lot of diagnostic information.
Also make -Yno-suspended-units behave better.

fixes #20119

@bishabosha bishabosha requested a review from sjrd April 9, 2024 15:05
Previously, it was assumed that a missing class (that is valid in current run)
during macro evaluation was due to the symbol being defined in the same project.
If this condition is met, then compilation is suspended.

This assumption breaks when the symbol comes from the classpath, but without a
corresponding class file, leading a situation where the same file is always suspended,
until it is the only one left, leading to the "cyclic macro dependencies" error.
In this case we should assume that the class file will never become available because class path entries
are supposed to be immutable. Therefore we should not suspend in this case.

This commit therefore detects this situation. Instead of suspending the unit,
the compiler aborts the macro expansion, reporting an error that
the user will have to deal with - likely by changing the build definition
@bishabosha bishabosha force-pushed the fix-pipelining-cyclic-macros branch from 1af8a12 to ab91dfe Compare April 10, 2024 09:08
@bishabosha bishabosha enabled auto-merge April 10, 2024 09:09
@bishabosha bishabosha merged commit 2b09680 into scala:main Apr 10, 2024
16 checks passed
@bishabosha bishabosha deleted the fix-pipelining-cyclic-macros branch April 10, 2024 10:33
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
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.

Spurious "Cyclic macro dependencies" error with pipelining turned on
3 participants