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

Fix 13777 - refine check for caching companion in sum mirror #14035

Merged
merged 3 commits into from
Dec 13, 2021

Conversation

bishabosha
Copy link
Member

@bishabosha bishabosha commented Dec 3, 2021

This also exposes a check to see if a file is a "lateCompile" in the current run, as a late-compiled symbol's type info is derived from source - aka a companion that is late-compiled would not subtype Mirror.Sum.

Edit: the above is no longer a part of these changes

cache the mirror in the companion if:

  • the class is not scala 2 defined
    & the class has a companion
    & the class was either:
    • defined in source
    • from tasty and its companion is a subtype of Mirror.Sum

fixes #13777

@bishabosha
Copy link
Member Author

bishabosha commented Dec 6, 2021

I have no idea what's happening with the fs2 test in the community build - there are some comparison tests that are producing unexpected results - does not seem related to this

Edit- was fine after re-running CI

@bishabosha bishabosha assigned dwijnand and unassigned smarter Dec 6, 2021
cache the mirror in the companion if:
- the class is not scala 2 defined
  & the class has a companion
  & the class was either:
    - defined in source
    - from tasty and its companion is a subtype of Mirror.Sum
@bishabosha
Copy link
Member Author

@smarter I have implemented the simpler version of a from-source check

self.linkedClass.exists
&& !self.is(Scala2x)
&& (
// self is from source, or companion is a subtype of Sum
Copy link
Member

Choose a reason for hiding this comment

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

It'd be good to explain why for things which are not from source we need the more precise check (i.e., the difference in code generation between scala 3.0 and 3.1).

Copy link
Member

Choose a reason for hiding this comment

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

It's more about the other way round: ideally we can just check is subclass (which is only true in scala 3.1+). But we call this check before that parent is added, for code in compilation, so have to handle that specially.

Feels like the two new ones would subsume !self.is(Scala2x), right? (minor)

Copy link
Member Author

Choose a reason for hiding this comment

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

I have clarified the comment to explain why we check if its from source

@smarter smarter assigned bishabosha and unassigned smarter Dec 6, 2021
@dwijnand dwijnand added this to the 3.1.1 milestone Dec 12, 2021
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Thanks, Jamie, for picking this up. Good to see it didn't spin out from the fix we had theorycrafted. Good to go, for me. I would hope this could make the 3.1.1 patch release.

@smarter smarter added the backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" label Dec 13, 2021
@bishabosha bishabosha added backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Dec 13, 2021
@dwijnand dwijnand merged commit bc4d877 into scala:master Dec 13, 2021
@dwijnand dwijnand deleted the fix-13777 branch December 13, 2021 14:39
@bishabosha bishabosha added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClassCastException when summoning Mirror for hierarchical sum compiled by 3.0.x
3 participants