Add class parameters, flags, and privateWithin to newClass in reflect API #21880
Conversation
jchyb
added
the
needs-minor-release
| * | ||
| * Parameters can be obtained via classSymbol.memberField | ||
| */ | ||
| @experimental def newClass(parent: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr], paramNames: List[String], paramTypes: List[TypeRepr], flags: Flags, privateWithin: Symbol): Symbol |
There was a problem hiding this comment.
would it be possible to reformulate such that parents is also a function (to support param refs?)
There was a problem hiding this comment.
Do you mean that it should be a Symbol => List[TypeRepr] type? I have some trouble visualizing where we would need that classSymbol to define the parent TypeRepr (class Cls extends Cls.InnerCls?, is that legal?). Could you give me an example of a tree like that?
There was a problem hiding this comment.
Just saw class Cls extends Cls.InnerCls in the wild but in that case, Cls also had a companion object which contained InnerCls - not sure if that is possible to define right now but I'll have to look into the newModule method and see if it also needs updates
There was a problem hiding this comment.
I mean something like class anon(x: Int) extends Box[x.type] - I don't know the best solution, but this would be good to be able to support I assume - same with tracked eventually, but maybe that can be cleanly added later?
jchyb
force-pushed
the
improvement/quotes-new-class-upgrades
branch
from
28cb854 to
33fbd5f
Compare
| import quotes.reflect.* | ||
|
|
||
| val name = nameExpr.valueOrAbort | ||
| val parents = List(TypeTree.of[Object]) |
There was a problem hiding this comment.
there should be a test case where the parent needs an argument supplied via the constructor parameters
There was a problem hiding this comment.
I tried doing that in the not-very-well-named tests/run-macros/newClassParamsExtendsClassParams, which generates something like this:
'{
class `name`(idx: Int) extends Foo(idx)
new `name`(22)
}Or do you mean something else?
There was a problem hiding this comment.
Also such test cases could be added:
- Creating a trait with Symbol.newClass (is this possible?)
- Creating an abstract class with Symbol.newClass
- trait with parameter extending another trait with parameter (not sure about this)
- class with multiple parameters (this probably not needed)
- extending java class with parameter
|
To be clear, this is not ready, I did put this up since I wanted some feedback if anyone had some (Thank You @bishabosha!) and did not want to rush it last minute, that's why it a draft |
jchyb
force-pushed
the
improvement/quotes-new-class-upgrades
branch
from
a839263 to
81cea85
Compare
| paramNames: List[String], | ||
| paramTypes: List[TypeRepr], | ||
| clsFlags: Flags, | ||
| clsPrivateWithin: Symbol |
There was a problem hiding this comment.
Some nit comments.
Since it is public API, maybe align argument names with already existing? E.G. flags and not clsFlags, privateWithin and not clsPrivateWithin
| clsPrivateWithin: Symbol, | ||
| consFlags: Flags, | ||
| consPrivateWithin: Symbol, | ||
| conParamFlags: List[List[Flags]] |
There was a problem hiding this comment.
constructor related arguments may have same prefix (now there are 3 constructor, cons and con) and maybe put all constructor related arguments together after everything else
|
Want this very much 👀 |
|
@jchyb @nicolasstucki Can I somehow help proceeding with this? Maybe contribute with a spree? |
|
It's pretty much done in terms of the difficult stuff. I still need to add some test cases (your suggestions should help, thank you!), docs and perhaps more checks for flags. I planned for this for 3.7, and we still have some time for that, thus the seemingly slow progress. |
jchyb
changed the title
Still some TODOs here, like tests for flags and privateWithin, scaladoc, adding assertions and adding similar updates to newModule. I probably will need to split off the changes relating to
-Xmacro-checkfrom here to a separate PR.Fixes #21739 and addresses some old TODOs