Skip to content

Commit cbd2cd7

Browse files
committed
Strengthen assertions around cleanup and formal evaluation scopes.
I haven't actually tested this separately from the changes to scope usage that are coming in the follow-up, but it's logically somewhat separate.
1 parent b08a3f3 commit cbd2cd7

File tree

3 files changed

+39
-39
lines changed

3 files changed

+39
-39
lines changed

lib/SILGen/Cleanup.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ void CleanupManager::emitCleanups(CleanupsDepth depth, CleanupLocation loc,
108108

109109
// Pop now if that was requested.
110110
if (popCleanups) {
111+
#ifndef NDEBUG
112+
// This is an implicit deactivation.
113+
if (stackCleanup.isActive()) {
114+
SGF.FormalEvalContext.checkCleanupDeactivation(cleanupHandle);
115+
}
116+
#endif
117+
111118
stack.pop();
112119

113120
#ifndef NDEBUG

lib/SILGen/FormalEvaluation.cpp

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -106,25 +106,28 @@ void FormalEvaluationScope::popImpl() {
106106
"popping formal-evaluation scopes out of order");
107107
context.innermostScope = previous;
108108

109-
// Check to see if there is anything going on here.
110-
111-
using iterator = FormalEvaluationContext::iterator;
112-
using stable_iterator = FormalEvaluationContext::stable_iterator;
109+
auto endDepth = *savedDepth;
113110

114-
iterator unwrappedSavedDepth = context.find(savedDepth.getValue());
115-
iterator iter = context.begin();
116-
if (iter == unwrappedSavedDepth)
111+
// Check to see if there is anything going on here.
112+
if (endDepth == context.stable_begin())
117113
return;
118114

115+
#ifndef NDEBUG
116+
// Verify that all the accesses are valid.
117+
for (auto i = context.begin(), e = context.find(endDepth); i != e; ++i) {
118+
i->verify(SGF);
119+
}
120+
#endif
121+
119122
// Save our start point to make sure that we are not adding any new cleanups
120123
// to the front of the stack.
121-
stable_iterator originalBegin = context.stable_begin();
122-
(void)originalBegin;
124+
auto originalBegin = context.stable_begin();
123125

124126
// Then working down the stack until we visit unwrappedSavedDepth...
125-
for (; iter != unwrappedSavedDepth; ++iter) {
126-
// Grab the next evaluation...
127-
FormalAccess &access = *iter;
127+
auto i = originalBegin;
128+
do {
129+
// Grab the next evaluation.
130+
FormalAccess &access = context.findAndAdvance(i);
128131

129132
// If this access was already finished, continue. This can happen if an
130133
// owned formal access was forwarded.
@@ -149,10 +152,9 @@ void FormalEvaluationScope::popImpl() {
149152
// code. We do a simple N^2 comparison here to detect this because it is
150153
// extremely unlikely more than a few writebacks are active at once.
151154
if (access.getKind() == FormalAccess::Exclusive) {
152-
iterator j = iter;
153-
++j;
154-
155-
for (; j != unwrappedSavedDepth; ++j) {
155+
// Note that we already advanced 'iter' above, so we can just start
156+
// iterating from there. Also, this doesn't invalidate the iterators.
157+
for (auto j = context.find(i), je = context.find(endDepth); j != je; ++j){
156158
FormalAccess &other = *j;
157159
if (other.getKind() != FormalAccess::Exclusive)
158160
continue;
@@ -167,33 +169,25 @@ void FormalEvaluationScope::popImpl() {
167169
//
168170
// This evaluates arbitrary code, so it's best to be paranoid
169171
// about iterators on the context.
170-
access.finish(SGF);
171-
}
172+
DiverseValueBuffer<FormalAccess> copiedAccess(access);
173+
copiedAccess.getCopy().finish(SGF);
174+
175+
} while (i != endDepth);
172176

173177
// Then check that we did not add any additional cleanups to the beginning of
174178
// the stack...
175179
assert(originalBegin == context.stable_begin() &&
176-
"more formal eval cleanups placed onto context during formal eval scope pop?!");
180+
"pushed more formal evaluations while popping formal evaluations?!");
177181

178182
// And then pop off all stack elements until we reach the savedDepth.
179-
context.pop(savedDepth.getValue());
183+
context.pop(endDepth);
180184
}
181185

182186
void FormalEvaluationScope::verify() const {
183-
// Check to see if there is anything going on here.
187+
// Walk up the stack to the saved depth.
184188
auto &context = SGF.FormalEvalContext;
185-
using iterator = FormalEvaluationContext::iterator;
186-
187-
iterator unwrappedSavedDepth = context.find(savedDepth.getValue());
188-
iterator iter = context.begin();
189-
if (iter == unwrappedSavedDepth)
190-
return;
191-
192-
// Then working down the stack until we visit unwrappedSavedDepth...
193-
for (; iter != unwrappedSavedDepth; ++iter) {
194-
// Grab the next evaluation verify that we can successfully access this
195-
// formal access.
196-
(*iter).verify(SGF);
189+
for (auto i = context.begin(), e = context.find(*savedDepth); i != e; ++i) {
190+
i->verify(SGF);
197191
}
198192
}
199193

lib/SILGen/FormalEvaluation.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ class FormalEvaluationContext {
139139
stable_iterator stable_begin() { return stabilize(begin()); }
140140
iterator find(stable_iterator iter) { return stack.find(iter); }
141141

142+
FormalAccess &findAndAdvance(stable_iterator &stable) {
143+
return stack.findAndAdvance(stable);
144+
}
145+
142146
template <class U, class... ArgTypes> void push(ArgTypes &&... args) {
143147
stack.push<U>(std::forward<ArgTypes>(args)...);
144148
}
@@ -236,12 +240,7 @@ class FormalEvaluationScope {
236240
#ifndef NDEBUG
237241
inline void
238242
FormalEvaluationContext::checkCleanupDeactivation(CleanupHandle handle) {
239-
// Start at the innermost scope depth. Note that we pop scopes off the
240-
// stack before we start emitting their cleanups.
241-
if (!innermostScope) return;
242-
assert(!innermostScope->isPopped());
243-
for (auto i = find(*innermostScope->savedDepth), e = end(); i != e; ++i) {
244-
auto &access = *i;
243+
for (auto &access : *this) {
245244
assert((access.isFinished() || access.getCleanup() != handle) &&
246245
"popping active formal-evaluation cleanup");
247246
}

0 commit comments

Comments
 (0)