Add an -Yimplicit-to-given flag for rewrites to easily test changes in the ecosystem #22580
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jchyb
force-pushed
the
add-implicit-as-given-flag
branch
from
3a7cc12 to
f7dc85b
Compare
|
I think it would be a good idea to merge this. It can be helpful for migrating to givens. |
|
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
force-pushed
the
add-implicit-as-given-flag
branch
from
3f55011 to
357a2aa
Compare
jchyb
force-pushed
the
add-implicit-as-given-flag
branch
from
357a2aa to
ddbe8c5
Compare
jchyb
changed the title
jchyb
changed the title
jchyb
force-pushed
the
add-implicit-as-given-flag
branch
from
ddbe8c5 to
71a4ae8
Compare
jchyb
force-pushed
the
add-implicit-as-given-flag
branch
2 times, most recently
from
23646c2 to
2604288
Compare
jchyb
force-pushed
the
add-implicit-as-given-flag
branch
from
2604288 to
cb31976
Compare
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #22482
It replaces every implicit definition with
given(including implicit classes, which I was surprised to see working), and every implicit parameter clause withusing, 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.6context 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 togivenorusingmight not be available.usingkeywords 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).