Skip to content

Commit cbbc03a

Browse files
swift-ciswift-ci
authored
Merge pull request #19986 from gottesmm/pr-39aa50102d65c79c698d094c139faab143fa5bbe
2 parents 11d8e86 + 447a11a commit cbbc03a

File tree

3 files changed

+95
-20
lines changed

3 files changed

+95
-20
lines changed

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2855,13 +2855,14 @@ void SILGenFunction::emitCatchDispatch(DoCatchStmt *S, ManagedValue exn,
28552855

28562856
// Set up an initial clause matrix.
28572857
ClauseMatrix clauseMatrix(clauseRows);
2858-
ConsumableManagedValue subject;
2859-
if (F.getModule().getOptions().EnableSILOwnership &&
2860-
exn.getType().isObject()) {
2861-
subject = {exn.borrow(*this, S), CastConsumptionKind::BorrowAlways};
2862-
} else {
2863-
subject = {exn, CastConsumptionKind::TakeOnSuccess};
2864-
}
2858+
2859+
assert(exn.getType().isObject() &&
2860+
"Error is special and should always be an object");
2861+
// Our model is that sub-cases get the exception at +0 and the throw (if we
2862+
// need to rethrow the exception) gets the exception at +1 since we need to
2863+
// trampoline it's ownership to our caller.
2864+
ConsumableManagedValue subject = {exn.borrow(*this, S),
2865+
CastConsumptionKind::BorrowAlways};
28652866

28662867
auto failure = [&](SILLocation location) {
28672868
// If we fail to match anything, just rethrow the exception.
@@ -2874,7 +2875,10 @@ void SILGenFunction::emitCatchDispatch(DoCatchStmt *S, ManagedValue exn,
28742875
return;
28752876
}
28762877

2877-
// Don't actually kill the exception's cleanup.
2878+
// Since we borrowed exn before sending it to our subcases, we know that it
2879+
// must be at +1 at this point. That being said, SILGenPattern will
2880+
// potentially invoke this for each of the catch statements, so we need to
2881+
// copy before we pass it into the throw.
28782882
CleanupStateRestorationScope scope(Cleanups);
28792883
if (exn.hasCleanup()) {
28802884
scope.pushCleanupState(exn.getCleanup(),

‎lib/SILGen/SILGenStmt.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,12 @@ void StmtEmitter::visitDoCatchStmt(DoCatchStmt *S) {
800800
// Emit all the catch clauses, branching to the end destination if
801801
// we fall out of one.
802802
SGF.emitCatchDispatch(S, exn, S->getCatches(), endDest);
803+
804+
// We assume that exn's cleanup is still valid at this point. To ensure that
805+
// we do not re-emit it and do a double consume, we rely on us having
806+
// finished emitting code and thus unsetting the insertion point here. This
807+
// assert is to make sure this invariant is clear in the code and validated.
808+
assert(!SGF.B.hasValidInsertionPoint());
803809
}
804810

805811
if (hasLabel) {

‎test/SILGen/errors.swift

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,5 @@
11
// RUN: %target-swift-emit-silgen -parse-stdlib -enable-sil-ownership -Xllvm -sil-print-debuginfo -verify -primary-file %s %S/Inputs/errors_other.swift | %FileCheck %s
22

3-
// TODO: Turn back on ownership verification. I turned off the verification on
4-
// this file since it shows an ownership error that does not affect
5-
// codegen. Specifically when we destroy the temporary array we use for the
6-
// variadic tuple, we try to borrow the temporary array when we pass it to the
7-
// destroy function. This makes the ownership verifier think that the owned
8-
// value we are trying to destroy is not cleaned up. But once ownership is
9-
// stripped out, the destroy array function still does what it needs to do. The
10-
// actual fix for this would require a bunch of surgery in SILGenApply around
11-
// how function types are computed and preserving non-canonical function types
12-
// through SILGenApply. This is something that can be done after +0 is turned
13-
// on.
14-
153
import Swift
164

175
class Cat {}
@@ -176,6 +164,83 @@ func all_together_now(_ flag: Bool) -> Cat {
176164
}
177165
}
178166

167+
// Make sure that if we catch an error in a throwing function we borrow the
168+
// error and only consume the error in the rethrow block.
169+
//
170+
// CHECK-LABEL: sil hidden @$s6errors20all_together_now_twoyAA3CatCSgSbKF : $@convention(thin) (Bool) -> (@owned Optional<Cat>, @error Error) {
171+
// CHECK: bb0(
172+
// CHECK-NOT: bb1
173+
// CHECK: try_apply {{.*}}, normal [[NORMAL_BB:bb[0-9]+]], error [[ERROR_BB:bb[0-9]+]]
174+
//
175+
// CHECK: [[ERROR_BB]]([[ERROR:%.*]] : @owned $Error):
176+
// CHECK: [[BORROWED_ERROR:%.*]] = begin_borrow [[ERROR]]
177+
// CHECK: [[COPIED_ERROR:%.*]] = copy_value [[BORROWED_ERROR]]
178+
// CHECK: store [[COPIED_ERROR]] to [init] [[CAST_INPUT_MEM:%.*]] : $*Error
179+
// CHECK: checked_cast_addr_br copy_on_success Error in [[CAST_INPUT_MEM]] : $*Error to HomeworkError in [[CAST_OUTPUT_MEM:%.*]] : $*HomeworkError, [[CAST_YES_BB:bb[0-9]+]], [[CAST_NO_BB:bb[0-9]+]],
180+
//
181+
// CHECK: [[CAST_YES_BB]]:
182+
// CHECK: [[SUBERROR:%.*]] = load [take] [[CAST_OUTPUT_MEM]]
183+
// CHECK: switch_enum [[SUBERROR]] : $HomeworkError, case #HomeworkError.TooHard!enumelt: {{bb[0-9]+}}, default [[SWITCH_MATCH_FAIL_BB:bb[0-9]+]],
184+
//
185+
// CHECK: [[SWITCH_MATCH_FAIL_BB]]([[SUBERROR:%.*]] : @owned $HomeworkError):
186+
// CHECK: destroy_value [[SUBERROR]]
187+
// CHECK: end_borrow [[BORROWED_ERROR]]
188+
// CHECK: br [[RETHROW_BB:bb[0-9]+]]([[ERROR]] : $Error)
189+
//
190+
// CHECK: [[CAST_NO_BB]]:
191+
// CHECK: end_borrow [[BORROWED_ERROR]]
192+
// CHECK: br [[RETHROW_BB]]([[ERROR]] : $Error)
193+
//
194+
// CHECK: [[RETHROW_BB]]([[ERROR_FOR_RETHROW:%.*]] : @owned $Error):
195+
// CHECK: throw [[ERROR_FOR_RETHROW]]
196+
// CHECK: } // end sil function '$s6errors20all_together_now_twoyAA3CatCSgSbKF'
197+
func all_together_now_two(_ flag: Bool) throws -> Cat? {
198+
do {
199+
return try dont_return(Cat())
200+
} catch HomeworkError.TooHard {
201+
return nil
202+
}
203+
}
204+
205+
// Same as the previous test, but with multiple cases instead of just one.
206+
//
207+
// CHECK-LABEL: sil hidden @$s6errors22all_together_now_threeyAA3CatCSgSbKF : $@convention(thin) (Bool) -> (@owned Optional<Cat>, @error Error) {
208+
// CHECK: bb0(
209+
// CHECK-NOT: bb1
210+
// CHECK: try_apply {{.*}}, normal [[NORMAL_BB:bb[0-9]+]], error [[ERROR_BB:bb[0-9]+]]
211+
//
212+
// CHECK: [[ERROR_BB]]([[ERROR:%.*]] : @owned $Error):
213+
// CHECK: [[BORROWED_ERROR:%.*]] = begin_borrow [[ERROR]]
214+
// CHECK: [[COPIED_ERROR:%.*]] = copy_value [[BORROWED_ERROR]]
215+
// CHECK: store [[COPIED_ERROR]] to [init] [[CAST_INPUT_MEM:%.*]] : $*Error
216+
// CHECK: checked_cast_addr_br copy_on_success Error in [[CAST_INPUT_MEM]] : $*Error to HomeworkError in [[CAST_OUTPUT_MEM:%.*]] : $*HomeworkError, [[CAST_YES_BB:bb[0-9]+]], [[CAST_NO_BB:bb[0-9]+]],
217+
//
218+
// CHECK: [[CAST_YES_BB]]:
219+
// CHECK: [[SUBERROR:%.*]] = load [take] [[CAST_OUTPUT_MEM]]
220+
// CHECK: switch_enum [[SUBERROR]] : $HomeworkError, case #HomeworkError.TooHard!enumelt: {{bb[0-9]+}}, case #HomeworkError.TooMuch!enumelt: {{bb[0-9]+}}, default [[SWITCH_MATCH_FAIL_BB:bb[0-9]+]],
221+
//
222+
// CHECK: [[SWITCH_MATCH_FAIL_BB]]([[SUBERROR:%.*]] : @owned $HomeworkError):
223+
// CHECK: destroy_value [[SUBERROR]]
224+
// CHECK: end_borrow [[BORROWED_ERROR]]
225+
// CHECK: br [[RETHROW_BB:bb[0-9]+]]([[ERROR]] : $Error)
226+
//
227+
// CHECK: [[CAST_NO_BB]]:
228+
// CHECK: end_borrow [[BORROWED_ERROR]]
229+
// CHECK: br [[RETHROW_BB]]([[ERROR]] : $Error)
230+
//
231+
// CHECK: [[RETHROW_BB]]([[ERROR_FOR_RETHROW:%.*]] : @owned $Error):
232+
// CHECK: throw [[ERROR_FOR_RETHROW]]
233+
// CHECK: } // end sil function '$s6errors22all_together_now_threeyAA3CatCSgSbKF'
234+
func all_together_now_three(_ flag: Bool) throws -> Cat? {
235+
do {
236+
return try dont_return(Cat())
237+
} catch HomeworkError.TooHard {
238+
return nil
239+
} catch HomeworkError.TooMuch {
240+
return nil
241+
}
242+
}
243+
179244
// Catch in non-throwing context.
180245
// CHECK-LABEL: sil hidden @$s6errors11catch_a_catAA3CatCyF : $@convention(thin) () -> @owned Cat
181246
// CHECK-NEXT: bb0:

0 commit comments

Comments
 (0)