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

-Xsource:3 now respects refinements by whitebox macro overrides #10160

Merged
merged 1 commit into from Sep 30, 2022

Conversation

som-snytt
Copy link
Contributor

@scala-jenkins scala-jenkins added this to the 2.13.10 milestone Sep 25, 2022
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix seems a little arbitrary to me.

The way I understand whitebox macros is that the type of a macro invocation is the inferred type of the expansion. But this PR is changing the rules of type inference while type checking a macro expansion, which is something else.

On the other hand, since this is about macros which anyway don't cross-build with Scala 3, I guess it's benign. The underlying complication is the staging aspect, macro expansions are compiled with the user's compiler flags, and the quill macros require -Xsource:3 to be disabled to work as intended.

@som-snytt
Copy link
Contributor Author

som-snytt commented Sep 26, 2022

Edit: actually, maybe the only relevant part of my comment (I haven't had coffee yet) is

Possibly, the macro should be required to generate the signature if it wants it.

I think this is why whitebox magic such as https://docs.scala-lang.org/overviews/macros/typeproviders.html works.

I agree that once you're inside the expansion, it seems to weaken language guarantees.

The quill code emits def quoted = ast; def ast = $etwas; this is the required member refinement that is inferred. So it's turtles all the way down.

Possibly, the macro should be required to generate the signature if it wants it.

I couldn't find (quickly) a way to test if the current macro is whitebox, so it merely checks if any macros are getting expanded.

As a footnote, there is still a bit of old code in assignTypeToTree for macros that I think isn't used. Also noting that Scala 2 macros are still experimental.

@lrytz
Copy link
Member

lrytz commented Sep 26, 2022

Possibly, the macro should be required to generate the signature if it wants it.

I also think this would be the right thing to do. But more pragmatically, I think we should just merge this PR. As I said before, it seems benign, and it avoids causing problems for people that don't even know they are relying on our experiments.

@SethTisue SethTisue self-assigned this Sep 30, 2022
@SethTisue SethTisue merged commit 6966b64 into scala:2.13.x Sep 30, 2022
@SethTisue SethTisue removed their assignment Sep 30, 2022
@som-snytt som-snytt deleted the issue/12645-whitebox-inference branch September 30, 2022 02:57
@SethTisue SethTisue changed the title Xsource:3 respects refinements by whitebox overrides Xsource:3 now respects refinements by whitebox macro overrides Oct 8, 2022
@SethTisue SethTisue changed the title Xsource:3 now respects refinements by whitebox macro overrides -Xsource:3 now respects refinements by whitebox macro overrides Oct 8, 2022
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
4 participants