Port sourcecode to Dotty #80
Conversation
In the dotty community build, the version is forced to something other than 0.10 (e.g. 0.12.…), so scala-2.10 isn't taken into account without this change.
Symbol literals are dropped: scala/scala3#5681
|
Thanks! I'll take a look. @alexarchambault @olafurpg as committers your review would also be very welcome |
There was a problem hiding this comment.
Left some comments. Looks good!
Args not working is probably fine I think. I never found it work reliably even in Scala 2.x, and it seemed to have a propensity for making the compiler think there's an invalid recursive reference.
I don't think I'm qualified to review sourcecode/src-0/sourcecode/Macros.scala; from a cursory view, it looks like a mostly syntactic rewrite from the original scala-reflect implementation. Hopefully someone more familiar with dotty-reflect can sign off on it!
The overall project layout changes look fine to me.
Only thing I want is getting FullName and Enclosing to match their output in Scala 2; I assume Dotty's macro API provides all the functionality necessary, and you just need to wrestle/mangle it to give the same result. After that I'll be happy to merge and keep this running in CI
| import ctx.tasty.given | ||
| val owner = actualOwner(ctx.tasty)(ctx.tasty.rootContext.owner) | ||
| val simpleName = Util.getName(ctx.tasty)(owner) | ||
| '{Name(${simpleName.toExpr})} |
There was a problem hiding this comment.
Is the reference to free terms like Name in these splices hygienic? We use the ${c.prefix} incantation in the Scala 2.x implementation to ensure that, not sure if somethign similar is required in Dotty
There was a problem hiding this comment.
As far as my understanding of hygiene in Scala 2 macros goes, in Scala 3 it is fine. Scala 3 quotes are compiled on definition site wheres Scala 2 quasiquotes are compiled on expansion site. So in Scala 2, if e.g. you use Name instead of ${c.prefix} and comment out package sourcecode in Apply.scala tests, you'll get a bunch of errors saying that Name is not found. Whereas it works with the package sourcecode uncommented. In Scala 3, the context of expansion is irrelevant since the quote is already compiled. @nicolasstucki maybe you can provide more precise info on this.
@nicolasstucki @liufengyun if you find the time for this one, your opinion would be much appreciated! |
The conventions to distinguish static vs instance members
anatoliykmetyuk
force-pushed
the
upstream-tracking
branch
from
5bfbe16 to
6ecc0c5
Compare
|
I've addressed the changes mentioned – WDYT @lihaoyi? |
|
@anatoliykmetyuk I think it looks good. How would you like to proceed from here: would you like to merge this into master and keep it running in uTest's CI? Or would you like to port a few more libraries to Dotty and get them all working together before committing? Or do you want to wait for the next Dotty stable version before publishing these libraries? I'm fine with whatever, so it would be up to you how you'd like to do this |
|
I think it's best to keep things as decoupled as possible while aiming at making them available to the end-user fast. What I have in mind is the following.
This should make sure the changes induced by the Dotty development propagate to the main repo as fast as possible. Also, this ensures that the libraries become available to the community at the same time with the new Dotty RC versions. The relatively frequent changes forced by the Dotty development (especially macros) can be included in the main repos without the need for the library version change since the language version (nightly and RC) will be different for these changes. The only possible drawback here is the point of releasing the libraries to Maven on each Dotty RC build and when a library depends on a nightly version of another library. We release every 6 weeks, and if your CI doesn't automatically publish to Maven, this scenario may tax you with the work needed to publishing the libraries every 6 weeks. What do you think? |
|
@anatoliykmetyuk that plan sounds reasonable to me. One caveat is that since Dotty releases relatively frequently, we should publish versions of these libraries for new versions of Dotty without incrementing the library's version number. That should be doable simple by selecting the dotty cross builds when we run I can give you my publishing credentials, that way you'll have full flexibility to publish any and all of these libraries for Dotty as and when you see fit: whether every 6 weeks, or even more frequently. I can also give you commit bit on these repos, so for minor changes that do not require a code review (e.g. tweaks to the Dotty-specific code) you can simply make them without needing to go the latency of a PR review & merge (though for any large changes it's still best to get reviewed) If we're OK with that, I suppose the next step would be merging this PR, and giving you publishing/commit access. Let me know and i'll send you the invite/creds |
|
Sounds good to me – thanks for making this happen!
Yes, the Dotty-induced changes will be tagged with the language version, so we don't need to change the library version. |
|
Thanks @anatoliykmetyuk, I have sent you an invite for this repo and emailed you the publishing creds. You may merge when ready |
|
Merged it. The next Dotty release is the following week, so I'll try to publish the Dotty version to Maven then. |
Here's a short summary of the changes:
src-0folder and Scala 2 sources – insrc-2. Scala 2 sources are moved without changes.Argsdoes not work so far – see Update Sourcecode to the latest Upstream and Dotty versions scala/scala3#7553 (comment)