Skip to content

Commit 7464de2

Browse files
committed
[Profiler] Fix handling of property wrapper backing inits
Previously we were walking into the PropertyWrapperValuePlaceholderExpr when generating coverage for a property wrapper backing initializer. This meant that we were duplicating the coverage of the initializer expression, and it could cause crashes if a refined counter was introduced within the top-most expression region, such as with a throwing expression in a `try!`, or the use of an `if` expression. rdar://118939162
1 parent bd4007f commit 7464de2

File tree

2 files changed

+74
-0
lines changed

2 files changed

+74
-0
lines changed

lib/SIL/IR/SILProfiler.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,12 @@ struct MapRegionCounters : public ASTWalker {
270270
return LazyInitializerWalking::InAccessor;
271271
}
272272

273+
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
274+
// Don't walk into PropertyWrapperValuePlaceholderExprs, these should be
275+
// mapped as part of the wrapped value initialization.
276+
return false;
277+
}
278+
273279
void mapRegion(ASTNode N) {
274280
mapRegion(ProfileCounterRef::node(N));
275281
}
@@ -584,6 +590,12 @@ struct PGOMapping : public ASTWalker {
584590
return LazyInitializerWalking::InAccessor;
585591
}
586592

593+
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
594+
// Don't walk into PropertyWrapperValuePlaceholderExprs, these should be
595+
// mapped as part of the wrapped value initialization.
596+
return false;
597+
}
598+
587599
MacroWalking getMacroWalkingBehavior() const override {
588600
return MacroWalking::Expansion;
589601
}
@@ -945,6 +957,12 @@ struct CoverageMapping : public ASTWalker {
945957
return LazyInitializerWalking::InAccessor;
946958
}
947959

960+
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
961+
// Don't walk into PropertyWrapperValuePlaceholderExprs, these should be
962+
// mapped as part of the wrapped value initialization.
963+
return false;
964+
}
965+
948966
MacroWalking getMacroWalkingBehavior() const override {
949967
return MacroWalking::Expansion;
950968
}

test/Profiler/coverage_property_wrapper_backing.swift

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,29 @@ struct PassThroughWrapper<T> {
2525
// CHECK: [[BB]]:
2626
// CHECK-NEXT: increment_profiler_counter 1
2727

28+
// CHECK-LABEL: sil hidden @$s33coverage_property_wrapper_backing1UV1xSivpfP : $@convention(thin) (Int) -> Wrapper<Int>
29+
// CHECK: function_ref @$sSb6randomSbyFZ : $@convention(method) (@thin Bool.Type) -> Bool
30+
// CHECK: cond_br {{%[0-9]+}}, [[BB_TRUE:bb[0-9]+]], [[BB_FALSE:bb[0-9]+]]
31+
//
32+
// CHECK: [[BB_FALSE]]:
33+
// CHECK: integer_literal {{.*}}, 2
34+
//
35+
// CHECK: [[BB_TRUE]]:
36+
// CHECK: increment_profiler_counter 1
37+
// CHECK: integer_literal {{.*}}, 1
38+
39+
// CHECK-LABEL: sil hidden [transparent] @$s33coverage_property_wrapper_backing1UV2_{{.*}}7WrapperVySiGvpfi : $@convention(thin) () -> Int
40+
// CHECK: increment_profiler_counter 0
41+
// CHECK: function_ref @$sSb6randomSbyFZ : $@convention(method) (@thin Bool.Type) -> Bool
42+
// CHECK: cond_br {{%[0-9]+}}, [[BB_TRUE:bb[0-9]+]], [[BB_FALSE:bb[0-9]+]]
43+
44+
// CHECK: [[BB_FALSE]]:
45+
// CHECK: integer_literal {{.*}}, 4
46+
//
47+
// CHECK: [[BB_TRUE]]:
48+
// CHECK: increment_profiler_counter 1
49+
// CHECK: integer_literal {{.*}}, 3
50+
2851
struct S {
2952
// CHECK-LABEL: sil_coverage_map {{.*}} "$s33coverage_property_wrapper_backing1SV1iSivpfP" {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.S.i
3053
// CHECK-NEXT: [[@LINE+4]]:4 -> [[@LINE+4]]:30 : 0
@@ -55,3 +78,36 @@ struct T {
5578
@Wrapper(.random() ? 1 : 2)
5679
var k = 3
5780
}
81+
82+
// rdar://118939162 - Make sure we don't include the initialization expression
83+
// in the backing initializer.
84+
struct U {
85+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.U.x
86+
// CHECK-NEXT: [[@LINE+4]]:4 -> [[@LINE+4]]:30 : 0
87+
// CHECK-NEXT: [[@LINE+3]]:24 -> [[@LINE+3]]:25 : 1
88+
// CHECK-NEXT: [[@LINE+2]]:28 -> [[@LINE+2]]:29 : (0 - 1)
89+
// CHECK-NEXT: }
90+
@Wrapper(.random() ? 1 : 2)
91+
var x = if .random() { 3 } else { 4 }
92+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_property_wrapper_backing.U.(_x
93+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:40 : 0
94+
// CHECK-NEXT: [[@LINE-3]]:14 -> [[@LINE-3]]:23 : 0
95+
// CHECK-NEXT: [[@LINE-4]]:24 -> [[@LINE-4]]:29 : 1
96+
// CHECK-NEXT: [[@LINE-5]]:29 -> [[@LINE-5]]:40 : 0
97+
// CHECK-NEXT: [[@LINE-6]]:35 -> [[@LINE-6]]:40 : (0 - 1)
98+
// CHECK-NEXT: }
99+
}
100+
101+
struct V {
102+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.V.x
103+
// CHECK-NEXT: [[@LINE+2]]:4 -> [[@LINE+2]]:14 : 0
104+
// CHECK-NEXT: }
105+
@Wrapper(0)
106+
var x = switch Bool.random() { case true: 0 case false: 0 }
107+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_property_wrapper_backing.V.(_x
108+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:62 : 0
109+
// CHECK-NEXT: [[@LINE-3]]:18 -> [[@LINE-3]]:31 : 0
110+
// CHECK-NEXT: [[@LINE-4]]:34 -> [[@LINE-4]]:46 : 1
111+
// CHECK-NEXT: [[@LINE-5]]:47 -> [[@LINE-5]]:60 : 2
112+
// CHECK-NEXT: }
113+
}

0 commit comments

Comments
 (0)