Regression: incorrect scope in T of typealias of Listing<T>/Mapping<K, T> #823
Labels
bug
Something isn't working
Milestone
Comments
bioball
changed the title
bioball
changed the title
odenix
added a commit
to odenix/pkl
that referenced
this issue
Nov 28, 2024
When a Pkl object is instantiated, it captures the current frame's receiver and owner by storing `frame.materialize()`. Because a materialized frame is a reference to, not a snapshot of, a virtual frame, this way of capturing receiver and owner is only safe if the virtual frame's receiver and owner are never mutated. However, TypeAliasTypeNode does mutate receiver and owner (3d1db25), which is why a VmListingOrMapping instantiated by ListingOrMappingTypeNode must store them individually instead of storing `frame.materialize()`. To rule out that the same problem surfaces elsewhere, it might be a good idea to do one of the following: * stop mutating receiver and owner * change the way Pkl objects capture receiver and owner
odenix
added a commit
to odenix/pkl
that referenced
this issue
Dec 4, 2024
Motivation: - simplify implementation of lazy type checking - fix correctness issues of lazy type checking (apple#785) Changes: - implement listing/mapping type cast via amendment (`parent`) instead of delegation (`delegate`) - handle type checking of *computed* elements/entries in the same way as type checking of computed properties - ElementOrEntryNode is the equivalent of TypeCheckedPropertyNode - remove fields VmListingOrMapping.delegate/typeNodeFrame/cachedMembers/checkedMembers - fix apple#785 by executing all type casts between a member's owner and receiver - fix apple#823 by storing owner and receiver directly instead of storing the mutable frame containing them (typeNodeFrame) - remove overrides of VmObject methods that are no longer required - good for Truffle partial evaluation and JVM inlining - revert a85a173 except for added tests - move `VmUtils.setOwner` and `VmUtils.setReceiver` and make them private - these methods aren't generally safe to use Result: - simpler code with greater optimization potential - VmListingOrMapping can now have both a type node and new members - fewer changes to surrounding code - smaller memory footprint - better performance in some cases - fixes apple#785 - fixes apple#823 Potential future optimizations: - avoid lazy type checking overhead for untyped listings/mappings - improve efficiency of forcing a typed listing/mapping - currently, lazy type checking will traverse the parent chain once per member, reducing the performance benefit of shallow-forcing a listing/mapping over evaluating each member individually - avoid creating an intermediate untyped listing/mapping in the following cases: - `new Listing<X> {...}` - amendment of `property: Listing<X>`
odenix
added a commit
to odenix/pkl
that referenced
this issue
Dec 4, 2024
Motivation: - simplify implementation of lazy type checking - fix correctness issues of lazy type checking (apple#785) Changes: - implement listing/mapping type cast via amendment (`parent`) instead of delegation (`delegate`) - handle type checking of *computed* elements/entries in the same way as type checking of computed properties - ElementOrEntryNode is the equivalent of TypeCheckedPropertyNode - remove fields VmListingOrMapping.delegate/typeNodeFrame/cachedMembers/checkedMembers - fix apple#785 by executing all type casts between a member's owner and receiver - fix apple#823 by storing owner and receiver directly instead of storing the mutable frame containing them (typeNodeFrame) - remove overrides of VmObject methods that are no longer required - good for Truffle partial evaluation and JVM inlining - revert a85a173 except for added tests - move `VmUtils.setOwner` and `VmUtils.setReceiver` and make them private - these methods aren't generally safe to use Result: - simpler code with greater optimization potential - VmListingOrMapping can now have both a type node and new members - fewer changes to surrounding code - smaller memory footprint - better performance in some cases - fixes apple#785 - fixes apple#823 Potential future optimizations: - avoid lazy type checking overhead for untyped listings/mappings - improve efficiency of forcing a typed listing/mapping - currently, lazy type checking will traverse the parent chain once per member, reducing the performance benefit of shallow-forcing a listing/mapping over evaluating each member individually - avoid creating an intermediate untyped listing/mapping in the following cases: - `new Listing<X> {...}` - amendment of `property: Listing<X>`
stackoverflow
pushed a commit
that referenced
this issue
Dec 6, 2024
Motivation: - simplify implementation of lazy type checking - fix correctness issues of lazy type checking (#785) Changes: - implement listing/mapping type cast via amendment (`parent`) instead of delegation (`delegate`) - handle type checking of *computed* elements/entries in the same way as type checking of computed properties - ElementOrEntryNode is the equivalent of TypeCheckedPropertyNode - remove fields VmListingOrMapping.delegate/typeNodeFrame/cachedMembers/checkedMembers - fix #785 by executing all type casts between a member's owner and receiver - fix #823 by storing owner and receiver directly instead of storing the mutable frame containing them (typeNodeFrame) - remove overrides of VmObject methods that are no longer required - good for Truffle partial evaluation and JVM inlining - revert a85a173 except for added tests - move `VmUtils.setOwner` and `VmUtils.setReceiver` and make them private - these methods aren't generally safe to use Result: - simpler code with greater optimization potential - VmListingOrMapping can now have both a type node and new members - fewer changes to surrounding code - smaller memory footprint - better performance in some cases - fixes #785 - fixes #823 Potential future optimizations: - avoid lazy type checking overhead for untyped listings/mappings - improve efficiency of forcing a typed listing/mapping - currently, lazy type checking will traverse the parent chain once per member, reducing the performance benefit of shallow-forcing a listing/mapping over evaluating each member individually - avoid creating an intermediate untyped listing/mapping in the following cases: - `new Listing<X> {...}` - amendment of `property: Listing<X>`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Given:
This breaks:
Error:
The text was updated successfully, but these errors were encountered: