report phases/units via xsbti.compile.CompileProgress.startUnit #13082 #13084
Conversation
unkarjedy
commented
Jul 15, 2021
unkarjedy
commented
- revive scala3-sbt-bridge-tests
There was a problem hiding this comment.
xsbti.compile.CompileProgress.advance is still not called
First, it's not obvious where exactly to calculate current / total
Second, there are some issues with the binary compatibility:
scala3-compiler depends on compiler-interface version 1.3.5
scala3-sbt-bridge depends on version 1.4.3
These two versions have different signature of advance.
1.3.5:
boolean advance(int current, int total);1.4.x and 1.5.x:
default boolean advance(int current, int total, String prevPhase, String nextPhase) {
return true;
}this will lead to runtime error NoSuchMethodError
As I understand form the code, the only reason scala3-compiler uses old interface is to avoid deprecation warnings:
Dependencies.oldCompilerInterface, // we stick to the old version to avoid deprecation warningsI tried to update it to the new version and suppress deprecated methods usages with @nowarn.
But it doesn't work ATM (see #8908 and #12857)
| ("Test-0-0.scala", "postInlining"), | ||
| ("Test-0-0.scala", "staging"), | ||
| ("Test-0-0.scala", "pickleQuotes"), | ||
| ("Test-0-0.scala", "MegaPhase{firstTransform, checkReentrant, elimPackagePrefixes, cookComments, checkStatic, betaReduce, inlineVals, expandSAMs}"), |
There was a problem hiding this comment.
These "MegaPhase" phase names are not ideal, but I don't know how to better report them ATM, taking into account that all subphases are reported for each file
There was a problem hiding this comment.
Maybe just the first name, e.g. "miniphases following firstTransform"?
There was a problem hiding this comment.
Hmm, not sure if there should be information loss.
This could probably be done on the client-side, by parsing.
It has quite a simple structure for that.
There was a problem hiding this comment.
Also, maybe we could create meaningful names for each MegaPhase, and report it?
I just do not have enough expertise in the compiler to give proper names to them.
There was a problem hiding this comment.
Also, maybe we could create meaningful names for each MegaPhase, and report it?
If we could, we would just put all mini-phases in a single MegaPhase (since that would result in a single pass over the AST), but because of implementation details that's not possible, so we split them into the minimum number of MegaPhases we can, but these MegaPhases are not meaningful on their own, so there's no useful name we can give them. In fact, I don't think the name of phases in general is useful information for end users, you could just as well use numbers ("Running phase 1/6 ...") or nothing at all ("Compiling ..."), since the number isn't very useful either.
There was a problem hiding this comment.
We do not print the information about the phases as text anywhere.
It's only reported in the Compilation Charts
https://blog.jetbrains.com/scala/2021/04/07/intellij-scala-plugin-2021-1/#phases-and-units-in-compilation-charts
Yes, most of the users are not interested in this, but there were some users (Scala 2), dealing with macro and compiler internals, which reported their interest in this kind of information.
There was a problem hiding this comment.
Anyway, until I can call advance method, this method call will tell IDE that the compilation process is not stuck.
(currently, compilation can take minutes without any indication of progress)
Any value will be fine.
…#13082 + revive scala3-sbt-bridge-tests
unkarjedy
force-pushed
the
branch-for-pr-13082
branch
from
1071d43 to
fd960b7
Compare
unkarjedy
force-pushed
the
branch-for-pr-13082
branch
2 times, most recently
from
9555513 to
5bf84e3
Compare
NOTE 1 (reminder): zinc also transitively includes compiler-interface NOTE 2: scala3-sbt-bridge 3.0.1 depends on compile-interface 1.4.3, scala3-compiler depends on 1.3.5 (which is partially binary compatible with 1.4.3) scala/scala3#13084 (review) NOTE 2: if this change breaks anything for some reason, try changing the version to 1.4.4 instead of reverting to 1.4.0-M12. I tried to compile some example project with Bloop and it at least didn't fail.
…iler` flag is specified
unkarjedy
force-pushed
the
branch-for-pr-13082
branch
from
5bf84e3 to
4bfa99e
Compare
| // Beware: scala3-compiler uses compiler-interface 1.3.5 which is binary incompatible | ||
| // with the one used un scala3-sbt-bridge (1.4.5) | ||
| // it has different signature def advance(current: Int, total: Int): Boolean |
There was a problem hiding this comment.
Yeah, trying to get to that, but haven't managed to do it yet. We should also upgrade zinc in Gradle after the Scala 3 support is merged.
|
@unkarjedy @smarter @tgodzik What's the status of this PR? Is it blocked on something or is it ready for review? |
|
See #13084 (comment) |
|
This is highly blocked on me 😓 |
|
Coming back to this after a long long time. What's the status of this? Is this something we still want to push forward, or should we close? |
|
This is blocked on #10816 which @bishabosha is working on right now :). |
|
I am working on a different PR to implement CompileProgress, will still reference this one for tests |
