-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add reflect Symbol.info and TypeRepr.asSeenFrom #15024
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
Conversation
We found this missing method because of this use case #11685 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I think
tpe
as a name is too close to typeRef/termRef while doing something different. I'm not a fan ofinfo
but at least it's established jargon in Scala. - If we add this we should also add
asSeenFrom
since most of the time we're interested in the info as seen from a specific prefix and forgetting to use the correct prefix is a common source of bugs. This was also requested recently: "asSeenFrom" in Scala 3 #14957 - For the purpose of overriding we should provide some convenience method (
Symbol.newMethodOverride
?) which given a method symbol in a parent takes care of generating a new symbol with the correct info and flags to override it.
4e01f55
to
838a454
Compare
val parents = List(TypeTree.of[Object], TypeTree.of[T]) | ||
|
||
def decls(cls: Symbol): List[Symbol] = methods.map { method => | ||
Symbol.newMethod(cls, method.name, method.info, flags = Flags.EmptyFlags /*TODO: method.flags */, privateWithin = method.privateWithin.fold(Symbol.noSymbol)(_.typeSymbol)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you opened #15035 but it's classified as an enhancement when I think it should be required to stabilize Symbol.newClass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm working on that one now. I intend to open a separate PR for that one as it is not directly related with the info
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #15041
Very keen to see this get into the mainline! |
838a454
to
d48b91e
Compare
d48b91e
to
98e10d8
Compare
No description provided.