Skip to content

SILGen: Emit property descriptors for conditionally Copyable and Escapable types. #81617

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

Merged

Conversation

jckarter
Copy link
Contributor

@jckarter jckarter commented May 19, 2025

Key paths can't reference non-escapable or non-copyable storage declarations,
so we don't need to refer to them resiliently, and can elide their property
descriptors.

However, declarations may still be conditionally Copyable and Escapable, and
if so, then they still need a property descriptor for resilient key path
references. When a property or subscript can be used in a context where it
is fully Copyable and Escapable, emit the property descriptor in a generic
environment constrained by the necessary conditional constraints.

Fixes rdar://151628396.

@jckarter jckarter requested a review from eeckstein as a code owner May 19, 2025 19:32
@jckarter jckarter requested review from atrick and removed request for eeckstein May 19, 2025 19:32
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter jckarter force-pushed the no-non-escapable-property-descriptors branch from 177d989 to 2ae0954 Compare May 20, 2025 00:36
@jckarter jckarter requested a review from a team as a code owner May 20, 2025 00:36
@jckarter
Copy link
Contributor Author

@swift-ci Please test

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jckarter jckarter force-pushed the no-non-escapable-property-descriptors branch from 2ae0954 to 3aedcc0 Compare May 24, 2025 01:32
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter
Copy link
Contributor Author

@swift-ci Please test Linux


// If the type is an archetype, then it just needs Copyable and Escapable
// constraints imposed.
if (ty->is<ArchetypeType>()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupConformance() returns an abstract conformance if you give it an archetype or a type parameter. Is this conditional necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupConformance seems to actually check the conformances of ArchetypeTypes, which I don't want here. The doc comment for lookupConformance does say it doesn't check conditional conformances, so it seems like it would be morally appropriate for it to unconditionally return an abstract conformance given an archetype, but (at least for 6.2) I didn't want to track down all the reliances on the existing behavior.

…pable types.

Key paths can't reference non-escapable or non-copyable storage declarations,
so we don't need to refer to them resiliently, and can elide their property
descriptors.

However, declarations may still be conditionally Copyable and Escapable, and
if so, then they still need a property descriptor for resilient key path
references. When a property or subscript can be used in a context where it
is fully Copyable and Escapable, emit the property descriptor in a generic
environment constrained by the necessary conditional constraints.

Fixes rdar://151628396.
@jckarter jckarter force-pushed the no-non-escapable-property-descriptors branch from 3aedcc0 to 22eb7e6 Compare May 27, 2025 16:36
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter jckarter changed the title SILGen: Don't emit property descriptors for ~Escapable types. SILGen: Emit property descriptors for conditionally Copyable and Escapable types. May 27, 2025
@jckarter jckarter merged commit dd59797 into swiftlang:main May 28, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants