Skip to content

Commit 855b3e4

Browse files
atrickatrick
committed
[nonescapable] remove '@_lifetime' requirement on implicit accessors
This avoids diagnostic errors on synthesized accessors, which are impossible for developers to understand. Fixes rdar://153793344 (Lifetime-dependent value returned by generated accessor '_read')
1 parent a8da66a commit 855b3e4

File tree

3 files changed

+189
-23
lines changed

3 files changed

+189
-23
lines changed

‎lib/AST/LifetimeDependence.cpp‎

Lines changed: 106 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ class LifetimeDependenceChecker {
412412
return false;
413413
}
414414

415+
// Infer ambiguous cases for backward compatibility.
415416
bool useLazyInference() const {
416417
return isInterfaceFile()
417418
|| ctx.LangOpts.EnableExperimentalLifetimeDependenceInference;
@@ -873,7 +874,7 @@ class LifetimeDependenceChecker {
873874
// Infer non-Escapable results.
874875
if (isDiagnosedNonEscapable(getResultOrYield())) {
875876
if (hasImplicitSelfParam()) {
876-
// Methods and accessors that return or yield a non-Escapable value.
877+
// Methods that return a non-Escapable value.
877878
inferNonEscapableResultOnSelf();
878879
return;
879880
}
@@ -910,31 +911,42 @@ class LifetimeDependenceChecker {
910911
return true;
911912
}
912913

913-
// Infer method dependence: result depends on self. This includes _modify.
914+
// Infer method dependence of result on self for
915+
// methods, getters, and _modify accessors.
914916
void inferNonEscapableResultOnSelf() {
915917
Type selfTypeInContext = dc->getSelfTypeInContext();
916918
if (selfTypeInContext->hasError()) {
917919
return;
918920
}
919-
bool nonEscapableSelf = isDiagnosedNonEscapable(selfTypeInContext);
920921

921-
// Avoid diagnosing inference on mutating methods when 'self' is
922-
// non-Escapable. The inout 'self' also needs an inferred dependence on
923-
// itself. This will be diagnosed when checking for missing dependencies.
924-
if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
925-
if (auto accessor = dyn_cast<AccessorDecl>(afd)) {
926-
inferMutatingAccessor(accessor);
922+
bool nonEscapableSelf = isDiagnosedNonEscapable(selfTypeInContext);
923+
if (auto accessor = dyn_cast<AccessorDecl>(afd)) {
924+
if (isImplicitOrSIL() || useLazyInference()) {
925+
if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
926+
// Implicit accessors that return or yield a non-Escapable value may
927+
// infer dependency on both self and result.
928+
inferMutatingAccessor(accessor);
929+
}
930+
// Infer the result dependency on self based on the kind of accessor
931+
// that is wrapped by this synthesized accessors.
932+
if (auto dependenceKind =
933+
getAccessorDependence(accessor, selfTypeInContext)) {
934+
pushDeps(createDeps(resultIndex).add(selfIndex, *dependenceKind));
935+
}
936+
return;
927937
}
938+
// Explicit accessors are inferred the same way as regular methods.
939+
}
940+
// Do infer the result of a mutating method when 'self' is
941+
// non-Escapable. The missing dependence on inout 'self' will be diagnosed
942+
// later anyway, so an explicit annotation will still be needed.
943+
if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
928944
return;
929945
}
930-
931946
// Methods with parameters only apply to lazy inference.
932947
if (!useLazyInference() && afd->getParameters()->size() > 0) {
933948
return;
934949
}
935-
// Allow inference for implicit getters, which simply return a stored,
936-
// property, and for implicit _read/_modify, which cannot be defined
937-
// explicitly alongside a regular getter.
938950
if (!useLazyInference() && !isImplicitOrSIL()) {
939951
// Require explicit @_lifetime(borrow self) for UnsafePointer-like self.
940952
if (!nonEscapableSelf && isBitwiseCopyable(selfTypeInContext, ctx)) {
@@ -950,9 +962,12 @@ class LifetimeDependenceChecker {
950962
return;
951963
}
952964
}
965+
// Infer based on ownership if possible for either explicit accessors or
966+
// methods as long as they pass preceding ambiguity checks.
953967
auto kind = inferLifetimeDependenceKind(
954968
selfTypeInContext, afd->getImplicitSelfDecl()->getValueOwnership());
955969
if (!kind) {
970+
// Special diagnostic for an attempt to depend on a consuming parameter.
956971
diagnose(returnLoc,
957972
diag::lifetime_dependence_cannot_infer_scope_ownership,
958973
"self", diagnosticQualifier());
@@ -961,6 +976,8 @@ class LifetimeDependenceChecker {
961976
pushDeps(createDeps(resultIndex).add(selfIndex, *kind));
962977
}
963978

979+
// Infer the kind of dependence that makes sense for reading or writing a
980+
// stored property (for getters or initializers).
964981
std::optional<LifetimeDependenceKind>
965982
inferLifetimeDependenceKind(Type sourceType, ValueOwnership ownership) {
966983
if (!sourceType->isEscapable()) {
@@ -974,8 +991,9 @@ class LifetimeDependenceChecker {
974991
auto loweredOwnership = ownership != ValueOwnership::Default
975992
? ownership
976993
: getLoweredOwnership(afd);
977-
if (loweredOwnership != ValueOwnership::Shared &&
978-
loweredOwnership != ValueOwnership::InOut) {
994+
// It is impossible to depend on a consumed Escapable value (unless it is
995+
// BitwiseCopyable as checked above).
996+
if (loweredOwnership == ValueOwnership::Owned) {
979997
return std::nullopt;
980998
}
981999
return LifetimeDependenceKind::Scope;
@@ -1125,7 +1143,10 @@ class LifetimeDependenceChecker {
11251143
// Handle implicit setters before diagnosing mutating methods. This
11261144
// does not include global accessors, which have no implicit 'self'.
11271145
if (auto accessor = dyn_cast<AccessorDecl>(afd)) {
1128-
inferMutatingAccessor(accessor);
1146+
// Explicit setters require explicit lifetime dependencies.
1147+
if (isImplicitOrSIL() || useLazyInference()) {
1148+
inferMutatingAccessor(accessor);
1149+
}
11291150
return;
11301151
}
11311152
if (afd->getParameters()->size() > 0) {
@@ -1142,12 +1163,29 @@ class LifetimeDependenceChecker {
11421163
LifetimeDependenceKind::Inherit));
11431164
}
11441165

1145-
// Infer a mutating accessor's non-Escapable 'self' dependencies.
1146-
void inferMutatingAccessor(AccessorDecl *accessor) {
1166+
// Infer dependence for an accessor whose non-escapable result depends on
1167+
// self. This includes _read and _modify.
1168+
void inferAccessor(AccessorDecl *accessor, Type selfTypeInContext) {
1169+
// Explicit accessors require explicit lifetime dependencies.
11471170
if (!isImplicitOrSIL() && !useLazyInference()) {
1148-
// Explicit setters require explicit lifetime dependencies.
11491171
return;
11501172
}
1173+
bool nonEscapableSelf = isDiagnosedNonEscapable(selfTypeInContext);
1174+
if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
1175+
// First, infer the dependency on the inout non-Escapable self. This may
1176+
// result in two inferred dependencies for accessors.
1177+
inferMutatingAccessor(accessor);
1178+
}
1179+
// Infer the result dependency on self based on the kind of accessor that
1180+
// is wrapped by this synthesized accessors.
1181+
if (auto dependenceKind =
1182+
getAccessorDependence(accessor, selfTypeInContext)) {
1183+
pushDeps(createDeps(resultIndex).add(selfIndex, *dependenceKind));
1184+
}
1185+
}
1186+
1187+
// Infer a mutating accessor's non-Escapable 'self' dependencies.
1188+
void inferMutatingAccessor(AccessorDecl *accessor) {
11511189
switch (accessor->getAccessorKind()) {
11521190
case AccessorKind::Read:
11531191
case AccessorKind::Read2:
@@ -1164,8 +1202,6 @@ class LifetimeDependenceChecker {
11641202
// _modify for them even though it won't be emitted.
11651203
pushDeps(createDeps(selfIndex).add(selfIndex,
11661204
LifetimeDependenceKind::Inherit));
1167-
pushDeps(createDeps(resultIndex).add(selfIndex,
1168-
LifetimeDependenceKind::Scope));
11691205
break;
11701206
case AccessorKind::Set: {
11711207
const unsigned newValIdx = 0;
@@ -1185,7 +1221,7 @@ class LifetimeDependenceChecker {
11851221
// dependency, so the setter cannot depend on 'newValue'.
11861222
if (!paramTypeInContext->isEscapable()) {
11871223
targetDeps = std::move(targetDeps)
1188-
.add(newValIdx, LifetimeDependenceKind::Inherit);
1224+
.add(newValIdx, LifetimeDependenceKind::Inherit);
11891225
}
11901226
pushDeps(std::move(targetDeps));
11911227
break;
@@ -1207,6 +1243,54 @@ class LifetimeDependenceChecker {
12071243
}
12081244
}
12091245

1246+
// Implicit accessors must be consistent with the accessor that they
1247+
// wrap. Otherwise, the sythesized implementation will report a diagnostic
1248+
// error.
1249+
std::optional<LifetimeDependenceKind>
1250+
getAccessorDependence(AccessorDecl *accessor, Type selfTypeInContext) {
1251+
std::optional<AccessorKind> wrappedAccessorKind = std::nullopt;
1252+
switch (accessor->getAccessorKind()) {
1253+
case AccessorKind::Read:
1254+
case AccessorKind::Read2:
1255+
case AccessorKind::Modify:
1256+
case AccessorKind::Modify2:
1257+
// read/modify are syntesized as calls to the getter.
1258+
wrappedAccessorKind = AccessorKind::Get;
1259+
break;
1260+
case AccessorKind::Get:
1261+
// getters are synthesized as access to a stored property.
1262+
break;
1263+
default:
1264+
// Unknown synthesized accessor.
1265+
// Setters are handled in inferMutatingAccessor() because they don't
1266+
// return a value.
1267+
return std::nullopt;
1268+
}
1269+
if (wrappedAccessorKind) {
1270+
auto *var = cast<VarDecl>(accessor->getStorage());
1271+
for (auto *wrappedAccessor : var->getAllAccessors()) {
1272+
if (wrappedAccessor->isImplicit())
1273+
continue;
1274+
if (wrappedAccessor->getAccessorKind() == wrappedAccessorKind) {
1275+
if (auto deps = wrappedAccessor->getLifetimeDependencies()) {
1276+
for (auto &dep : *deps) {
1277+
if (dep.getTargetIndex() != resultIndex)
1278+
continue;
1279+
if (dep.checkInherit(selfIndex))
1280+
return LifetimeDependenceKind::Inherit;
1281+
if (dep.checkScope(selfIndex))
1282+
return LifetimeDependenceKind::Scope;
1283+
}
1284+
}
1285+
}
1286+
}
1287+
}
1288+
// Either a Get or Modify without any wrapped accessor. Handle these like a
1289+
// read of the stored property.
1290+
return inferLifetimeDependenceKind(
1291+
selfTypeInContext, afd->getImplicitSelfDecl()->getValueOwnership());
1292+
}
1293+
12101294
// Infer 'inout' parameter dependency when the only parameter is
12111295
// non-Escapable.
12121296
//

‎test/SILOptimizer/lifetime_dependence/accessors.swift‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature Lifetimes -primary-file %s | %FileCheck %s
2+
3+
// REQUIRES: swift_feature_Lifetimes
4+
5+
struct NE : ~Escapable {
6+
let _p: UnsafeRawPointer
7+
8+
@_lifetime(borrow p)
9+
init(_ p: UnsafeRawPointer) { self._p = p }
10+
}
11+
12+
struct NCNE : ~Copyable, ~Escapable {
13+
let _p: UnsafeRawPointer
14+
15+
@_lifetime(borrow p)
16+
init(_ p: UnsafeRawPointer) {
17+
self._p = p
18+
}
19+
}
20+
21+
struct NEStoredProp : ~Escapable {
22+
var ne: NE
23+
// NEStoredProp.ne.getter
24+
// CHECK-LABEL: sil hidden [transparent] @$s9accessors12NEStoredPropV2neAA2NEVvg : $@convention(method) (@guaranteed NEStoredProp) -> @lifetime(copy 0) @owned NE {
25+
26+
// NEStoredProp.ne.setter
27+
// CHECK-LABEL: sil hidden [transparent] @$s9accessors12NEStoredPropV2neAA2NEVvs : $@convention(method) (@owned NE, @lifetime(copy 0, copy 1) @inout NEStoredProp) -> () {
28+
29+
// NEStoredProp.ne.modify
30+
// CHECK-LABEL: sil hidden [transparent] @$s9accessors12NEStoredPropV2neAA2NEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NEStoredProp) -> @lifetime(copy 0) @yields @inout NE {
31+
32+
// NEStoredProp.init(ne:)
33+
// CHECK-LABEL: sil hidden @$s9accessors12NEStoredPropV2neAcA2NEV_tcfC : $@convention(method) (@owned NE, @thin NEStoredProp.Type) -> @lifetime(copy 0) @owned NEStoredProp {
34+
}
35+
36+
struct NCNEStoredProp : ~Copyable & ~Escapable {
37+
var ncne: NCNE
38+
// NCNEStoredProp.ncne.read
39+
// CHECK-LABEL: sil hidden [transparent] @$s9accessors14NCNEStoredPropV4ncneAA4NCNEVvr : $@yield_once @convention(method) (@guaranteed NCNEStoredProp) -> @lifetime(copy 0) @yields @guaranteed NCNE {
40+
41+
// NCNEStoredProp.ncne.setter
42+
// CHECK-LABEL: sil hidden [transparent] @$s9accessors14NCNEStoredPropV4ncneAA4NCNEVvs : $@convention(method) (@owned NCNE, @lifetime(copy 0, copy 1) @inout NCNEStoredProp) -> () {
43+
44+
// NCNEStoredProp.ncne.modify
45+
// CHECK-LABEL: sil hidden [transparent] @$s9accessors14NCNEStoredPropV4ncneAA4NCNEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NCNEStoredProp) -> @lifetime(copy 0) @yields @inout NCNE {
46+
47+
// NCNEStoredProp.init(ncne:)
48+
// CHECK-LABEL: sil hidden @$s9accessors14NCNEStoredPropV4ncneAcA4NCNEV_tcfC : $@convention(method) (@owned NCNE, @thin NCNEStoredProp.Type) -> @lifetime(copy 0) @owned NCNEStoredProp {
49+
}
50+
51+
struct NCNEHolder : ~Copyable, ~Escapable {
52+
var p: UnsafeRawPointer
53+
54+
// this cannot be public in order to synthesize the _read accessor.
55+
var ncneBorrow: NCNE {
56+
// CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV10ncneBorrowAA4NCNEVvr : $@yield_once @convention(method) (@guaranteed NCNEHolder) -> @lifetime(borrow 0) @yields @guaranteed NCNE {
57+
@_lifetime(borrow self)
58+
borrowing get {
59+
_overrideLifetime(NCNE(p), borrowing: self)
60+
}
61+
// CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV10ncneBorrowAA4NCNEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NCNEHolder) -> @lifetime(borrow 0) @yields @inout NCNE {
62+
@_lifetime(self: copy self)
63+
set {
64+
p = newValue._p
65+
}
66+
}
67+
68+
// this cannot be public in order to synthesize the _read accessor.
69+
var ncneCopy: NCNE {
70+
// CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV8ncneCopyAA4NCNEVvr : $@yield_once @convention(method) (@guaranteed NCNEHolder) -> @lifetime(copy 0) @yields @guaranteed NCNE {
71+
@_lifetime(copy self)
72+
borrowing get {
73+
_overrideLifetime(NCNE(p), copying: self)
74+
}
75+
// CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV8ncneCopyAA4NCNEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NCNEHolder) -> @lifetime(copy 0) @yields @inout NCNE {
76+
@_lifetime(self: copy self)
77+
set {
78+
p = newValue._p
79+
}
80+
}
81+
}

‎test/SILOptimizer/lifetime_dependence/coroutine.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ func use(_ o : borrowing View) {}
8787
// CHECK: [[ACCESS:%.*]] = begin_access [read] [static] [[NC]]
8888
// CHECK: [[NCVAL:%.*]] = load [[ACCESS]]
8989
// CHECK: ([[WRAPPER:%.*]], [[TOKEN1:%.*]]) = begin_apply %{{.*}}([[NCVAL]]) : $@yield_once @convention(method) (@guaranteed NCContainer) -> @lifetime(borrow 0) @yields @guaranteed Wrapper
90-
// CHECK: [[MDI:%.*]] = mark_dependence [nonescaping] [[WRAPPER]] on [[ACCESS]]
90+
// CHECK: [[SCOPE:%.*]] = mark_dependence [nonescaping] [[WRAPPER]] on [[TOKEN1]]
91+
// CHECK: [[MDI:%.*]] = mark_dependence [nonescaping] [[SCOPE]] on [[ACCESS]]
9192
// CHECK: retain_value [[MDI]]
9293
// CHECK: debug_value [[MDI]], let, name "wrapper"
9394
// let view = wrapper.view

0 commit comments

Comments
 (0)