Skip to content

Refactor class parents handling #11475

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 1 commit into from
Feb 22, 2021
Merged

Conversation

smarter
Copy link
Member

@smarter smarter commented Feb 19, 2021

Before this commit we had:

  • Type#parents: for a ClassInfo, this is the parents of the class as seen from
    the ClassInfo prefix
  • ClassInfo#classParents: this is also the parents of the class but
    not as seen from the ClassInfo prefix
  • ClassDenotation#classParents: this delegates to ClassInfo#parents and
    not to ClassInfo#classParents as one might think.

To clear things up this commit:

  • Renames ClassInfo#classParents to initParents, to avoid accidentally
    using it instead of parents.
  • Remove ClassDenotation#classParents, one can always do info.parents
    instead.

Additionally, this commit introduces ClassDenotation#parentSyms, to be
used when one only care about the symbols of the parents. This method
relies on înitParents instead of parents which avoids any
unnecessary call to asSeenFrom. It is used in computeBaseData
instead of the old classParents to avoid a cyclic reference issue I
ran into while working on an unrelated PR.

@sjrd
Copy link
Member

sjrd commented Feb 19, 2021

It looks like you could also use parentSyms at
https://github.com/lampepfl/dotty/blob/aa7c21e6f78b75279752778eb0359f96e0795311/compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala#L317-L348

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

I'd change initParents to declaredParents. Otherwise LGTM.

Before this commit we had:
- Type#parents: for a ClassInfo, this is the parents of the class as seen from
  the ClassInfo prefix
- ClassInfo#classParents: this is also the parents of the class but
  _not_ as seen from the ClassInfo prefix
- ClassDenotation#classParents: this delegates to ClassInfo#parents and
  not to ClassInfo#classParents as one might think.

To clear things up this commit:
- Renames ClassInfo#classParents to declaredParents, to avoid accidentally
  using it instead of parents.
- Remove ClassDenotation#classParents, one can always do `info.parents`
  instead.

Additionally, this commit introduces ClassDenotation#parentSyms, to be
used when one only care about the symbols of the parents. This method
relies on `declaredParents` instead of `parents` which avoids any
unnecessary call to `asSeenFrom`. It is used in `computeBaseData`
instead of the old `classParents` to avoid a cyclic reference issue I
ran into while working on an unrelated PR.
@smarter smarter merged commit d8a788e into scala:master Feb 22, 2021
@smarter smarter deleted the classParents branch February 22, 2021 21:30
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
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.

4 participants