Add this as a self type - #1708
Conversation
6fb74b2 to
fa93f3d
Compare
a1d16f8 to
55f923c
Compare
71958eb to
a8c3282
Compare
| /** | ||
| * If {@code frame} is provided then self types should be resolved to real types, otherwise return | ||
| * the self PType | ||
| */ | ||
| protected PType doExport(@Nullable VirtualFrame frame) { |
There was a problem hiding this comment.
ReferenceTypeNode needs to export the real class here, so when a frame is passed NonFinalSelfTypeNode uses it to get the target's class instead of exporting PType.MODULE/PType.THIS.
There was a problem hiding this comment.
Do we need the logic around passing a frame in still?
There was a problem hiding this comment.
I think so, yeah. Even with the changes from #1796, ReferenceTypeNode still needs to erase self types in referents to their real classes before calling into VmReference. With #1796 and the other changes here, I think that could be done post-export, but it would mean walking/transforming the PType tree, which we have no machinery for. Preserving this mechanism is both more efficient (don't need to walk the type tree twice) and avoids potentially introducing new API to or helper code for PType.
6c21103 to
84c587e
Compare
bioball
left a comment
bioball
left a comment
There was a problem hiding this comment.
Left some comments!
The major ones are:
- Much of the resolution logic should happen inside
AstBuilder; we shouldn't need to traverse the truffle tree - Not sure about the
realTypeAliasFramelogic added to type node execution; this feels like a workaround to a deeper issue - Not sure if
TypeNode.export()should be deferencing self types; this feels like the responsibility of the caller, notTypeNodeitself
1dd722f to
528bd4f
Compare
30c1ed4 to
6db4376
Compare
| /** | ||
| * If {@code frame} is provided then self types should be resolved to real types, otherwise return | ||
| * the self PType | ||
| */ | ||
| protected PType doExport(@Nullable VirtualFrame frame) { |
There was a problem hiding this comment.
Do we need the logic around passing a frame in still?
| public MaterializedFrame materialize() { | ||
| return this; | ||
| } | ||
| } |
There was a problem hiding this comment.
I went down the rabbit hole today and I think I have a better solution to this whole thing.
The whole reason we are doing owner/receiver replacements is so that we can resolve variables within constraint expressions.
However, now that we are resolving variables at parse time (yay!), we don't need to be swapping out owner/receiver anymore. Instead, we can just produce ReadProperty/InvokeMethod nodes that are qualified off of the typealias's enclosing module. Then, we shouldn't need to be dealing with this logic around preserving the frame's original owner/reciever.
Since this is all orthogonal to the this type, here's a PR that implements this change!
There was a problem hiding this comment.
I've rebased this PR on #1796 and fully agree that it is the path forward, reducing a lot of the complexity around resolving the correct receiver for determining the this type. All of the aux frame slot stuff has been removed here now.
4419cd5 to
39c38ca
Compare
00421a9 to
4aa9f97
Compare
bb5718c to
97dac9c
Compare
2 participants
Depends on #1796
This PR:
this, a fully featured self type for use in modules or classes.thisin typealias bodies.moduletype in const scopes (class and typealias bodies, annotations, const properties/methods) with a warning, to be an error in a future release.moduletypes as typelias type args are evaluated in the context of the alias's module, not wheremoduleis used #1712).thisor a self-import and use of the current module.thisin the standard library:pkl.base#Any.getClass()now returnsClass<this>.pkl.base#Any.ifNonNull()now accepts an argument of type(this) -> Result.pkl.ref#Domain.renderReference()now accepts aReference<this, Any>.ReferenceTypeNode,moduletypes, and type aliases that causedmoduletypes in the referent position to always be resolved to the module where theReferencetype annotation is instead of wheremoduleis used.Loggerthrough toAstBuilderso warnings/traces can be printed from it.For ease of review, this PR consists of two commits; the first is all implementation changes, the second includes regenerated pkl-doc test output.
Resolves #1612
Resolves #1712