Skip to content

Spurious "Cyclic macro dependencies" error with pipelining turned on #20119

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.

Sign up for GitHub

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

Jump to bottom
Closed
bishabosha opened this issue Apr 8, 2024 · 2 comments · Fixed by #20139
Closed

Spurious "Cyclic macro dependencies" error with pipelining turned on #20119

bishabosha opened this issue Apr 8, 2024 · 2 comments · Fixed by #20139
Assignees
@bishabosha
Labels
area:tooling better-errors Issues concerned with improving confusing/unhelpful diagnostic messages itype:bug
Milestone
3.5.0

Comments

@bishabosha
Copy link
Member

bishabosha commented Apr 8, 2024 •
edited
Loading

Compiler version

3.5.0-RC1-bin-20240404-a7f00e2-NIGHTLY

Minimized code

both these projects exhibit the behaviour:

https://github.com/DFiantHDL/DFiant/tree/pipeline
https://github.com/atry/html.scala/tree/v3.0.3

Add these lines to build.sbt

ThisBuild / usePipelining := true
ThisBuild / scalaVersion := "3.5.0-RC1-bin-20240404-a7f00e2-NIGHTLY"

Output

from command html/Test/compile in atry/html.scala

[error] Cyclic macro dependencies in /Users/jamie/workspace/html.scala/html/target/scala-3.5.0-RC1-bin-20240404-a7f00e2-NIGHTLY/src_managed/test/sbt-example-generated.scala.
[error] Compilation stopped since no further progress can be made.
[error]
[error] To fix this, place macros in one set of files and their callers in another.
[error]
[error] Compile with -Xprint-suspension for information.
[warn] one warning found
[error] one error found
[error] Total time: 3 s, completed Apr 8, 2024, 9:11:58 AM

from command core/compile in DFiantHDL/DFiant

[error] Cyclic macro dependencies among
[error]
[error]   /Users/jamie/workspace/DFiant/core/src/main/scala/dfhdl/core/DFBits.scala, /Users/jamie/workspace/DFiant/core/src/main/scala/dfhdl/core/DFVector.scala, /Users/jamie/workspace/DFiant/core/src/main/scala/dfhdl/core/SameElementsVector.scala
[error]
[error] Compilation stopped since no further progress can be made.
[error]
[error] To fix this, place macros in one set of files and their callers in another.
[error]
[error] Compile with -Xprint-suspension for information.
[error] one error found
[error] (core / Compile / compileIncremental) Compilation failed
[error] Total time: 28 s, completed 8 Apr 2024, 09:19:48

both do not appear if pipelining is turned off.

Expectation

compile as normal
@bishabosha
Copy link
Member Author

bishabosha commented Apr 8, 2024 •
edited
Loading

from debugging so far it seems like a problem in macro splicing, suddenly there are far more "missing class" suspensions that do not appear without pipelining.

@Gedochao Gedochao removed their assignment Apr 8, 2024
@bishabosha
Copy link
Member Author

bishabosha commented Apr 8, 2024 •
edited
Loading

This is looking closer to a problem of build configuration - i.e. if a macro in project b uses a class in project a, then pipelining will need to be disabled on project a, but the error message is obscure.

E.g. in DFiant it looks like there is a class from the compiler_ir module being used in a macro in core, causing missing class error when invoking the macro. I'm working on improving reporting here.

Edit: it is still classed as a bug, because the error being reported is incorrect, it should be another error instead

@bishabosha bishabosha mentioned this issue Apr 9, 2024
Merged
@bishabosha bishabosha added better-errors Issues concerned with improving confusing/unhelpful diagnostic messages area:tooling and removed area:tooling labels Apr 9, 2024
bishabosha added a commit that referenced this issue Apr 10, 2024
@bishabosha
Detect macro dependencies that are missing from the classloader (#20139)
2b09680 
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 a more actionable error.

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
@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
Assignees

@bishabosha bishabosha

Labels
area:tooling better-errors Issues concerned with improving confusing/unhelpful diagnostic messages itype:bug
Projects
None yet
Milestone
3.5.0
Development

Successfully merging a pull request may close this issue.

Detect macro dependencies that are missing from the classloader dotty-staging/dotty
3 participants
@Kordyjan @bishabosha @Gedochao