Skip to content

Commit ceb17b7

Browse files
committed
SILGen: Don't reference external property descriptors for @_alwaysEmitIntoClient properties.
Their definition is fully visible to clients to be copied into them, and there isn't necessarily a symbol for the property descriptor in the defining module, so it isn't necessary or desirable to try to use a property descriptor with them. Trying to reference the descriptor leads to missing symbol errors at load time when trying to use keypaths with older versions of the defining dylib, which goes against the purpose of `@_alwaysEmitIntoClient` meaning "no ABI liabilities for the defining module". Fixes rdar://94049160.
1 parent e16b83f commit ceb17b7

File tree

3 files changed

+59
-15
lines changed

3 files changed

+59
-15
lines changed

lib/SILGen/SILGenExpr.cpp

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3580,21 +3580,47 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
35803580
/// Returns true if a key path component for the given property or
35813581
/// subscript should be externally referenced.
35823582
auto shouldUseExternalKeyPathComponent = [&]() -> bool {
3583-
return (!forPropertyDescriptor &&
3584-
(baseDecl->getModuleContext() != SwiftModule ||
3585-
baseDecl->isResilient(SwiftModule, expansion)) &&
3586-
// Protocol requirements don't have nor need property descriptors.
3587-
!isa<ProtocolDecl>(baseDecl->getDeclContext()) &&
3588-
// Properties that only dispatch via ObjC lookup do not have nor
3589-
// need property descriptors, since the selector identifies the
3590-
// storage.
3591-
// Properties that are not public don't need property descriptors
3592-
// either.
3593-
(!baseDecl->requiresOpaqueAccessors() ||
3594-
(!getAccessorDeclRef(getRepresentativeAccessorForKeyPath(baseDecl))
3595-
.isForeign &&
3596-
getAccessorDeclRef(getRepresentativeAccessorForKeyPath(baseDecl))
3597-
.getLinkage(ForDefinition) <= SILLinkage::PublicNonABI)));
3583+
// The property descriptor has the canonical key path component information
3584+
// so doesn't have to refer to another external descriptor.
3585+
if (forPropertyDescriptor) {
3586+
return false;
3587+
}
3588+
3589+
// Don't need to use the external component if we're inside the resilience
3590+
// domain of its defining module.
3591+
if (baseDecl->getModuleContext() == SwiftModule
3592+
&& !baseDecl->isResilient(SwiftModule, expansion)) {
3593+
return false;
3594+
}
3595+
3596+
// Protocol requirements don't have nor need property descriptors.
3597+
if (isa<ProtocolDecl>(baseDecl->getDeclContext())) {
3598+
return false;
3599+
}
3600+
3601+
// Always-emit-into-client properties can't reliably refer to a property
3602+
// descriptor that may not exist in older versions of their home dylib.
3603+
// Their definition is also always entirely visible to clients so it isn't
3604+
// needed.
3605+
if (baseDecl->getAttrs().hasAttribute<AlwaysEmitIntoClientAttr>()) {
3606+
return false;
3607+
}
3608+
3609+
// Properties that only dispatch via ObjC lookup do not have nor
3610+
// need property descriptors, since the selector identifies the
3611+
// storage.
3612+
// Properties that are not public don't need property descriptors
3613+
// either.
3614+
if (baseDecl->requiresOpaqueAccessors()) {
3615+
auto representative = getAccessorDeclRef(
3616+
getRepresentativeAccessorForKeyPath(baseDecl));
3617+
if (representative.isForeign)
3618+
return false;
3619+
if (representative.getLinkage(ForDefinition) > SILLinkage::PublicNonABI)
3620+
return false;
3621+
}
3622+
3623+
return true;
35983624
};
35993625

36003626
auto strategy = storage->getAccessStrategy(AccessSemantics::Ordinary,

validation-test/Evolution/Inputs/keypaths.swift.gyb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,9 @@ public final class AFinalClass {
123123

124124
}
125125

126+
public struct AEIC {
127+
#if AFTER
128+
@_alwaysEmitIntoClient
129+
public var aeic: Int { return 0 }
130+
#endif
131+
}

validation-test/Evolution/test_keypaths.swift.gyb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,16 @@ tests.test("AFinalClass.${name}") {
102102

103103
% end
104104

105+
#if AFTER
106+
tests.test("AlwaysEmitIntoClient") {
107+
// AEIC declarations are vendored into their importing module, so
108+
// equality doesn't currently work reliably with them. We can however
109+
// test that the reference to the property works and is backward-compatible
110+
// when using the `after` test suite with the `before` dylib.
111+
let kp = \AEIC.aeic
112+
113+
expectEqual(kp, kp)
114+
}
115+
#endif
116+
105117
runAllTests()

0 commit comments

Comments
 (0)