Skip to content

Add an -Yimplicit-to-given flag for rewrites to easily test changes in the ecosystem #22580

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Feb 11, 2025

closes #22482
It replaces every implicit definition with given (including implicit classes, which I was surprised to see working), and every implicit parameter clause with using, all in the parser phase.
Symbols and trees unpickled from tasty are unaffected by this flag. Trees generated using the reflection api are unaffected as well.
Pre -source:3.6 context bounds are left as implicits.
All of the above should be easy enough to change, but I didn't really see the purpose for it.

As I run into several issues with the previous method (where we would try to interpret implicit keywords as givens, with no changes to source code, which could lead to unclear error messages, and different behavior in dependent modules), I ended up redoing this to be rewrite-based. Now, the added flag should be used in conjunction with --rewrite, adding rewrite patches to the source code. This will omit implicit classes and old-style implicit conversions, where a simple rewrite to given or using might not be available. using keywords are also added to converted previously-implicit-now-using parameter applications, but only if both the method definition and application are in the same compilation run (in other cases they would have to be added manually).

@odersky
Copy link
Contributor

odersky commented Apr 8, 2025

I think it would be a good idea to merge this. It can be helpful for migrating to givens.

@jchyb
Copy link
Contributor Author

jchyb commented Apr 8, 2025

We probably should not merge this yet, as last time @WojciechMazur tested this with the cats repository, he detected some more differences beyond the given/implicit resolution unnaccaunted for here (e.g. implicits allow an additional empty argument list, but when replacing that with using, we get an error unrelated to what we want to be tested here).

I'll try to fix those issues and will prioritize this PR over the coming days, so that this can be merged (also since we want this merged in the compiler, I'll probably need to replace the added symbol flag with something else)

@jchyb jchyb force-pushed the add-implicit-as-given-flag branch from 3f55011 to 357a2aa Compare April 11, 2025 08:31
@jchyb jchyb force-pushed the add-implicit-as-given-flag branch from 357a2aa to ddbe8c5 Compare May 6, 2025 09:43
@jchyb jchyb changed the title Add an -Yimplicit-as-given flag to easily test changes in the ecosystem Add an -Yimplicit-to-given flag to easily test changes in the ecosystem May 6, 2025
@jchyb jchyb changed the title Add an -Yimplicit-to-given flag to easily test changes in the ecosystem Add an -Yimplicit-to-given flag for rewrites to easily test changes in the ecosystem May 6, 2025
@jchyb jchyb force-pushed the add-implicit-as-given-flag branch from ddbe8c5 to 71a4ae8 Compare May 6, 2025 10:10
@jchyb jchyb force-pushed the add-implicit-as-given-flag branch 2 times, most recently from 23646c2 to 2604288 Compare May 6, 2025 22:22
@jchyb jchyb force-pushed the add-implicit-as-given-flag branch from 2604288 to cb31976 Compare May 7, 2025 09:02
@jchyb jchyb marked this pull request as ready for review May 8, 2025 09:02
@jchyb jchyb requested a review from odersky May 8, 2025 09:03
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.

Compiler flag to interpret implicit definitions as givens
2 participants