Skip to content

Commit 8bc80c1

Browse files
hamishknighthamishknight
committed
[SILGen] Avoid delaying functions with user-written code
Previously we would delay the emission of lazy variable getters and stored property initializers for property wrapper backing storage. This could lead to their definitions being dropped if unused, meaning that we wouldn't run the mandatory diagnostics passes over them. Fix the logic such that we consider such cases as having user-written code, and account for a couple of cases where we can delay emission where we didn't previously. There are more cases we can handle here, but I'm leaving that as future work for now, as `emitOrDelayFunction` is currently only used for a handful of SILDeclRef kinds. This is a source breaking change, but only for invalid (albeit unused) code. rdar://99962285
1 parent 65cc95e commit 8bc80c1

File tree

6 files changed

+216
-3
lines changed

6 files changed

+216
-3
lines changed

‎include/swift/SIL/SILDeclRef.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,12 @@ struct SILDeclRef {
556556

557557
bool isImplicit() const;
558558

559+
/// Whether the referenced function contains user code. This is generally true
560+
/// for a non-implicit decls, but may also be true for implicit decls if
561+
/// explicitly written code has been spliced into the body. This is the case
562+
/// for e.g a lazy variable getter.
563+
bool hasUserWrittenCode() const;
564+
559565
/// Return the scope in which the parent class of a method (i.e. class
560566
/// containing this declaration) can be subclassed, returning NotApplicable if
561567
/// this is not a method, there is no such class, or the class cannot be

‎lib/SIL/IR/SILDeclRef.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,74 @@ bool SILDeclRef::isImplicit() const {
284284
llvm_unreachable("Unhandled case in switch");
285285
}
286286

287+
bool SILDeclRef::hasUserWrittenCode() const {
288+
// Non-implicit decls generally have user-written code.
289+
if (!isImplicit()) {
290+
switch (kind) {
291+
case Kind::PropertyWrapperBackingInitializer: {
292+
// Only has user-written code if any of the property wrappers have
293+
// arguments to apply. Otherwise, it's just a forwarding initializer for
294+
// the wrappedValue.
295+
auto *var = cast<VarDecl>(getDecl());
296+
return llvm::any_of(var->getAttachedPropertyWrappers(), [&](auto *attr) {
297+
return attr->hasArgs();
298+
});
299+
}
300+
case Kind::PropertyWrapperInitFromProjectedValue:
301+
// Never has user-written code, is just a forwarding initializer.
302+
return false;
303+
default:
304+
// TODO: This checking is currently conservative, we ought to
305+
// exhaustively handle all the cases here, and use emitOrDelayFunction
306+
// in more cases to take advantage of it.
307+
return true;
308+
}
309+
llvm_unreachable("Unhandled case in switch!");
310+
}
311+
312+
// Implicit decls generally don't have user-written code, but some splice
313+
// user code into their body.
314+
switch (kind) {
315+
case Kind::Func: {
316+
// Lazy getters splice in the user-written initializer expr.
317+
if (auto *accessor = dyn_cast<AccessorDecl>(getFuncDecl())) {
318+
auto *storage = accessor->getStorage();
319+
if (accessor->isGetter() && !storage->isImplicit() &&
320+
storage->getAttrs().hasAttribute<LazyAttr>()) {
321+
return true;
322+
}
323+
}
324+
return false;
325+
}
326+
case Kind::StoredPropertyInitializer: {
327+
// Property wrapper initializers for the implicit backing storage can splice
328+
// in the user-written initializer on the original property.
329+
auto *var = cast<VarDecl>(getDecl());
330+
if (auto *originalProperty = var->getOriginalWrappedProperty()) {
331+
if (originalProperty->isPropertyMemberwiseInitializedWithWrappedType())
332+
return true;
333+
}
334+
return false;
335+
}
336+
case Kind::Allocator:
337+
case Kind::Initializer:
338+
case Kind::EnumElement:
339+
case Kind::Destroyer:
340+
case Kind::Deallocator:
341+
case Kind::GlobalAccessor:
342+
case Kind::DefaultArgGenerator:
343+
case Kind::IVarInitializer:
344+
case Kind::IVarDestroyer:
345+
case Kind::PropertyWrapperBackingInitializer:
346+
case Kind::PropertyWrapperInitFromProjectedValue:
347+
case Kind::EntryPoint:
348+
case Kind::AsyncEntryPoint:
349+
// Implicit decls for these don't splice in user-written code.
350+
return false;
351+
}
352+
llvm_unreachable("Unhandled case in switch!");
353+
}
354+
287355
namespace {
288356
enum class LinkageLimit {
289357
/// No limit.

‎lib/SILGen/SILGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,7 @@ static void emitOrDelayFunction(SILGenModule &SGM,
11861186
// Implicit decls may be delayed if they can't be used externally.
11871187
auto linkage = constant.getLinkage(ForDefinition);
11881188
bool mayDelay = !forceEmission &&
1189-
(constant.isImplicit() &&
1189+
(!constant.hasUserWrittenCode() &&
11901190
!constant.isDynamicallyReplaceable() &&
11911191
!isPossiblyUsedExternally(linkage, SGM.M.isWholeModule()));
11921192

‎lib/Sema/TypeCheckPropertyWrapper.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,9 @@ Expr *swift::buildPropertyWrapperInitCall(
714714
ApplyExpr *innermostInit = nullptr;
715715

716716
// Projected-value initializers don't compose, so no need to iterate
717-
// over the wrapper attributes.
717+
// over the wrapper attributes. NOTE: If this ever changes, you'll need to
718+
// update SILDeclRef::hasUserWrittenCode to account for any spliced in
719+
// user-written code.
718720
if (initKind == PropertyWrapperInitKind::ProjectedValue) {
719721
auto typeExpr = TypeExpr::createImplicit(backingStorageType, ctx);
720722
auto *argList = ArgumentList::forImplicitSingle(ctx, ctx.Id_projectedValue,

‎test/SILGen/delayed_functions.swift

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// RUN: %target-swift-emit-silgen -primary-file %s | %FileCheck %s --check-prefix=SINGLE
2+
// RUN: %target-swift-emit-silgen %s | %FileCheck %s --check-prefix=WHOLEMOD
3+
// RUN: %target-swift-emit-sil -verify -primary-file %s
4+
5+
// https://github.com/apple/swift/issues/61128
6+
// Ensure we only delay emission of functions that do not contain user code.
7+
8+
// We cannot delay functions that contain user code.
9+
// SINGLE-LABEL: sil hidden [ossa] @$s17delayed_functions3fooSiyF : $@convention(thin) () -> Int
10+
// WHOLEMOD-LABEL: sil hidden [ossa] @$s17delayed_functions3fooSiyF : $@convention(thin) () -> Int
11+
func foo() -> Int { 5 }
12+
13+
// Cannot delay property intializers that contain user code.
14+
struct R {
15+
// variable initialization expression of R.i
16+
// SINGLE-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1RV1iSivpfi : $@convention(thin) () -> Int
17+
// WHOLEMOD-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1RV1iSivpfi : $@convention(thin) () -> Int
18+
var i = 0
19+
20+
// R.j.getter
21+
// SINGLE-LABEL: sil hidden [lazy_getter] [noinline] [ossa] @$s17delayed_functions1RV1jSivg : $@convention(method) (@inout R) -> Int
22+
// WHOLEMOD-LABEL: sil hidden [lazy_getter] [noinline] [ossa] @$s17delayed_functions1RV1jSivg : $@convention(method) (@inout R) -> Int
23+
lazy var j = Int.max + 1 // expected-error {{results in an overflow}}
24+
}
25+
26+
@propertyWrapper
27+
struct Wrapper<T> {
28+
var wrappedValue: T
29+
init(wrappedValue: T) {
30+
self.wrappedValue = wrappedValue
31+
}
32+
init(wrappedValue: T, _ x: Int) {
33+
self.wrappedValue = wrappedValue
34+
}
35+
}
36+
37+
struct Projected<T> {
38+
var value: T
39+
}
40+
@propertyWrapper
41+
struct ProjectedWrapper<T> {
42+
var wrappedValue: T
43+
init(wrappedValue: T) {
44+
self.wrappedValue = wrappedValue
45+
}
46+
var projectedValue: Projected<T> {
47+
.init(value: wrappedValue)
48+
}
49+
init(projectedValue: Projected<T>) {
50+
self.wrappedValue = projectedValue.value
51+
}
52+
}
53+
54+
struct S {
55+
// We can delay property wrapper backing initializers if they don't contain user code.
56+
// property wrapper backing initializer of S.i
57+
// SINGLE-LABEL: sil hidden [ossa] @$s17delayed_functions1SV1iSivpfP : $@convention(thin) (Int) -> Wrapper<Int>
58+
// WHOLEMOD-NOT: sil hidden [ossa] @$s17delayed_functions1SV1iSivpfP : $@convention(thin) (Int) -> Wrapper<Int>
59+
@Wrapper var i: Int
60+
61+
// But not if they contain user code.
62+
// property wrapper backing initializer of S.j
63+
// SINGLE-LABEL: sil hidden [ossa] @$s17delayed_functions1SV1jSSvpfP : $@convention(thin) (@owned String) -> @owned Wrapper<String>
64+
// WHOLEMOD-LABEL: sil hidden [ossa] @$s17delayed_functions1SV1jSSvpfP : $@convention(thin) (@owned String) -> @owned Wrapper<String>
65+
66+
// variable initialization expression of S._j
67+
// SINGLE-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1SV2_j33_6D0D158845E4BE9792202A0D16664A88LLAA7WrapperVySSGvpfi : $@convention(thin) () -> @owned String
68+
// WHOLEMOD-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1SV2_j33_6D0D158845E4BE9792202A0D16664A88LLAA7WrapperVySSGvpfi : $@convention(thin) () -> @owned String
69+
@Wrapper(5) var j = ""
70+
71+
// variable initialization expression of S._k
72+
// SINGLE-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1SV2_k33_6D0D158845E4BE9792202A0D16664A88LLAA7WrapperVySiGvpfi : $@convention(thin) () -> Int
73+
// WHOLEMOD-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1SV2_k33_6D0D158845E4BE9792202A0D16664A88LLAA7WrapperVySiGvpfi : $@convention(thin) () -> Int
74+
@Wrapper var k = Int.max + 1 // expected-error 2{{results in an overflow}}
75+
76+
// property wrapper backing initializer of S.l
77+
// SINGLE-LABEL: sil hidden [ossa] @$s17delayed_functions1SV1lSSvpfP : $@convention(thin) (@owned String) -> @owned Wrapper<Wrapper<String>>
78+
// WHOLEMOD-LABEL: sil hidden [ossa] @$s17delayed_functions1SV1lSSvpfP : $@convention(thin) (@owned String) -> @owned Wrapper<Wrapper<String>>
79+
80+
// variable initialization expression of S._l
81+
// SINGLE-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1SV2_l33_6D0D158845E4BE9792202A0D16664A88LLAA7WrapperVyAGySSGGvpfi : $@convention(thin) () -> @owned String
82+
// WHOLEMOD-LABEL: sil hidden [transparent] [ossa] @$s17delayed_functions1SV2_l33_6D0D158845E4BE9792202A0D16664A88LLAA7WrapperVyAGySSGGvpfi : $@convention(thin) () -> @owned String
83+
@Wrapper
84+
@Wrapper(.random() ? 1 : 2)
85+
var l = ""
86+
}
87+
88+
// The backing init for a projected wrapper can be delayed.
89+
// property wrapper init from projected value of x #1 in takesProjected(_:)
90+
// SINGLE-LABEL: sil hidden [ossa] @$s17delayed_functions14takesProjectedyyAA0D7WrapperVySiGF1xL_SivpfW : $@convention(thin) (Projected<Int>) -> ProjectedWrapper<Int>
91+
// WHOLEMOD-NOT: sil hidden [ossa] @$s17delayed_functions14takesProjectedyyAA0D7WrapperVySiGF1xL_SivpfW : $@convention(thin) (Projected<Int>) -> ProjectedWrapper<Int>
92+
func takesProjected(@ProjectedWrapper _ x: Int) {}
93+
94+
struct HasPrivate {
95+
// These backing initializers can be dropped entirely as they're private and
96+
// unused.
97+
98+
// property wrapper backing initializer of x #1 in HasPrivate.testPrivateWrapper(x:)
99+
// SINGLE-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV04testD7Wrapper{{.*}}LL1xyAA09ProjectedF0VySiG_tFAFL_SivpfP : $@convention(thin) (Int) -> ProjectedWrapper<Int>
100+
// WHOLEMOD-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV04testD7Wrapper{{.*}}LL1xyAA09ProjectedF0VySiG_tFAFL_SivpfP : $@convention(thin) (Int) -> ProjectedWrapper<Int>
101+
102+
// property wrapper init from projected value of x #1 in HasPrivate.testPrivateWrapper(x:)
103+
// SINGLE-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV04testD7Wrapper{{.*}}LL1xyAA09ProjectedF0VySiG_tFAFL_SivpfW : $@convention(thin) (Projected<Int>) -> ProjectedWrapper<Int>
104+
// WHOLEMOD-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV04testD7Wrapper{{.*}}LL1xyAA09ProjectedF0VySiG_tFAFL_SivpfW : $@convention(thin) (Projected<Int>) -> ProjectedWrapper<Int>
105+
106+
// The function itself needs to be emitted because it can contain user code.
107+
// HasPrivate.testPrivateWrapper(x:)
108+
// SINGLE: sil private [ossa] @$s17delayed_functions10HasPrivateV04testD7Wrapper{{.*}}LL1xyAA09ProjectedF0VySiG_tF : $@convention(method) (ProjectedWrapper<Int>, HasPrivate) -> ()
109+
// WHOLEMOD: sil private [ossa] @$s17delayed_functions10HasPrivateV04testD7Wrapper{{.*}}LL1xyAA09ProjectedF0VySiG_tF : $@convention(method) (ProjectedWrapper<Int>, HasPrivate) -> ()
110+
private func testPrivateWrapper(@ProjectedWrapper x: Int) {}
111+
112+
// property wrapper backing initializer of x #1 in HasPrivate.testFilePrivateWrapper(x:)
113+
// SINGLE-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV08testFileD7Wrapper{{.*}}LL1xyAA09ProjectedG0VySiG_tFAFL_SivpfP : $@convention(thin) (Int) -> ProjectedWrapper<Int>
114+
// WHOLEMOD-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV08testFileD7Wrapper{{.*}}LL1xyAA09ProjectedG0VySiG_tFAFL_SivpfP : $@convention(thin) (Int) -> ProjectedWrapper<Int>
115+
116+
// property wrapper init from projected value of x #1 in HasPrivate.testFilePrivateWrapper(x:)
117+
// SINGLE-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV08testFileD7Wrapper{{.*}}LL1xyAA09ProjectedG0VySiG_tFAFL_SivpfW : $@convention(thin) (Projected<Int>) -> ProjectedWrapper<Int>
118+
// WHOLEMOD-NOT: sil private [ossa] @$s17delayed_functions10HasPrivateV08testFileD7Wrapper{{.*}}LL1xyAA09ProjectedG0VySiG_tFAFL_SivpfW : $@convention(thin) (Projected<Int>) -> ProjectedWrapper<Int>
119+
120+
// The function itself needs to be emitted because it can contain user code.
121+
// HasPrivate.testFilePrivateWrapper(x:)
122+
// SINGLE: sil private [ossa] @$s17delayed_functions10HasPrivateV08testFileD7Wrapper{{.*}}LL1xyAA09ProjectedG0VySiG_tF : $@convention(method) (ProjectedWrapper<Int>, HasPrivate) -> ()
123+
// WHOLEMOD: sil private [ossa] @$s17delayed_functions10HasPrivateV08testFileD7Wrapper{{.*}}LL1xyAA09ProjectedG0VySiG_tF : $@convention(method) (ProjectedWrapper<Int>, HasPrivate) -> ()
124+
fileprivate func testFilePrivateWrapper(@ProjectedWrapper x: Int) {}
125+
}

‎test/SILGen/property_wrapper_parameter.swift

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend -emit-module -o %t -enable-library-evolution %S/Inputs/def_structA.swift
3-
// RUN: %target-swift-emit-silgen %s -I %t | %FileCheck %s
3+
4+
// This uses '-primary-file' to ensure we're conservative with lazy SIL emission.
5+
// RUN: %target-swift-emit-silgen -primary-file %s -I %t | %FileCheck %s
6+
47
import def_structA
58

69
public struct Projection<T> {
@@ -388,6 +391,15 @@ struct HasPrivate {
388391

389392
// CHECK-LABEL: sil private [ossa] @$s26property_wrapper_parameter10HasPrivateV08testFileE7Wrapper{{.*}}LL1xyAA0H0VySiG_tF : $@convention(method) (Wrapper<Int>, HasPrivate) -> ()
390393
fileprivate func testFilePrivateWrapper(@Wrapper x: Int) {}
394+
395+
func usesWrapperFunctions() {
396+
// These are needed to ensure we emit the backing initializers. Otherwise
397+
// lazy SILGen emission is happy to drop them.
398+
testPrivateWrapper(x: 0)
399+
testPrivateWrapper($x: Projection(wrappedValue: 0))
400+
testFilePrivateWrapper(x: 0)
401+
testFilePrivateWrapper($x: Projection(wrappedValue: 0))
402+
}
391403
}
392404

393405
@propertyWrapper

0 commit comments

Comments
 (0)