-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Enable returning classes from MacroAnnotations (part 3) #16534
Enable returning classes from MacroAnnotations (part 3) #16534
Conversation
1f0c08f
to
f6d8019
Compare
This comment was marked as resolved.
This comment was marked as resolved.
f6d8019
to
fdb14f2
Compare
e488e10
to
0576d72
Compare
Enable modification of classes with `MacroAnnotation`: * Can annotate `class` to transform it * Can annotate `object` to transform the companion class Supported class modifications: * Modify the implementations of `def`, `val`, `var`, `lazy val`, `class`, `object` in the class * Add new `def`, `val`, `var`, `lazy val`, `class`, `object` members to the class * Add a new override for a `def`, `val`, `var`, `lazy val` members in the class Restrictions: * An annotation on a top-level class cannot return a top-level `def`, `val`, `var`, `lazy val`. Related PRs: * Includes #16513 * Follows #16392 * Followed by #16534
Enable the addition of classes from a `MacroAnnotation`: * Can add new `class` definitions next to the annotated definition Special cases: * An annotated top-level `def`, `val`, `var`, `lazy val` can return a `class` definition that is owned by the package or package object. Related PRs: * Follows scala#16454
0576d72
to
5fcb869
Compare
59f3aa3
to
079a786
Compare
@experimental def newClass(parent: Symbol, name: String, parents: List[TypeRepr], decls: Symbol => List[Symbol], selfType: Option[TypeRepr]): Symbol | ||
|
||
/** Generates a new module symbol with an associated module class symbol. |
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.
/** Generates a new module symbol with an associated module class symbol. | |
/** Generates a new module symbol with an associated module class symbol, | |
* this is equivalent to an `object` declaration in source code. |
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.
It looks like this change was not applied.
/** List of top level classes added by macro annotation in a package object. | ||
* These are added the PackageDef that owns this particular package object. | ||
*/ | ||
private val topClasses = new collection.mutable.ListBuffer[Tree] |
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.
Unfortunately I think this isn't sufficient because package objects can be nested:
package foo {
val x = 1
package bar {
val y = 2
}
}
Instead, maybe the MemberDef case of transform should return a Thicket with the top-level classes, and we should add an extra case to transform
to handle the package object module class itself, where we should also return a Thicket with the top-level classes
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.
This use case was considered and works. I added tests for it in tests/run-macros/annot-add-global-class
.
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.
Note that after after post typer the tree is
package foo {
package bar {
val y = 2
}
val x = 1
}
This implies that nested classes are processed first and the buffer never overlaps and is emptied just after transforming the nested package.
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.
This implies that nested classes are processed first and the buffer never overlaps and is emptied just after transforming the nested package.
This is subtle, so this precondition should be documented in the code (and ideally checked somewhere, in case it breaks)
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 found cases where this precondition does not hold. I updated the implementation to handle such cases.
dfe1c14
to
89c550b
Compare
Similar to newNormalizedClassSymbol but for modules.
2d3c60f
to
1acd745
Compare
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.
Otherwise LGTM.
Co-authored-by: Guillaume Martres <smarter@ubuntu.com>
Enable the addition of classes from a
MacroAnnotation
:class
/object
definitions next to the annotated definitionSpecial cases:
def
,val
,var
,lazy val
can return aclass
/object
definition that is owned by the package or package object.
Related PRs: