Skip to content

Support Mirror synthesis for local and inner classes #15847

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

Merged
merged 2 commits into from
Aug 15, 2022

Conversation

bishabosha
Copy link
Member

@bishabosha bishabosha commented Aug 11, 2022

A new PR which takes a simpler approach - compute a prefix for the child and use asSeenFrom

  • update child accessibility check for anonymous mirrors in whyNotGenericSum. Now check the prefix at the callsite can access the child.
  • for sum mirrors, compute a new prefix for each child from the callsite prefix of the parent, see TypeOps.childPrefix. For each child, subsititute its prefix at definition with the childPrefix using asSeenFrom. For polymorphic classes, perform the subsitution on the constructor before inferring constraints.
  • add tests for issues 13332, 13935, 11174, 12328
  • add tests for local/inner classes taken from Shapeless for its Generic type, backed by mirrors

also added documentation changes

fixes #13332
fixes #12328
fixes #11174
fixes #13935
fixes #15122

- update child accessibility check for anonymous
  mirrors in whyNotGenericSum. Now check the
  prefix at the callsite can access the child.
- for sum mirrors, compute a new prefix for each
  child from the callsite prefix of the parent,
  see TypeOps.childPrefix. For each child,
  subsititute its prefix at definition with the
  childPrefix using asSeenFrom. For polymorphic
  classes, perform the subsitution on the
  constructor before inferring constraints.
- add tests for issues 13332, 13935, 11174, 12328
- add tests for local/inner classes taken from
  Shapeless for its Generic type, backed
  by mirrors
@bishabosha
Copy link
Member Author

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/15847/ to see the changes.

Benchmarks is based on merging with main (e560c2d)

@dwijnand dwijnand merged commit 802128e into scala:main Aug 15, 2022
@dwijnand dwijnand deleted the mirror-for-inner-classes branch August 15, 2022 20:37
@bishabosha bishabosha added the release-notes Should be mentioned in the release notes label Aug 16, 2022
@bishabosha
Copy link
Member Author

bishabosha commented Aug 16, 2022

should this be back ported to 3.2.0 - e.g. 3.2.0 won't be able to summon a mirror for a local class compiled by 3.2.1, so that makes it a forward source compatibility breaking change

@bishabosha bishabosha modified the milestones: 3.2.1-RC1, 3.2.0 backports Aug 17, 2022
@bishabosha bishabosha added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Aug 17, 2022
@bishabosha
Copy link
Member Author

doing the backport PR now

@Kordyjan Kordyjan added backport:done This PR was successfully backported. and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Aug 18, 2022
@Kordyjan Kordyjan modified the milestones: 3.2.0 backports, 3.2.1 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typeclass-derivation backport:done This PR was successfully backported. release-notes Should be mentioned in the release notes
Projects
None yet
4 participants