Skip to content

Commit 1953690

Browse files
committed
[silgen] When emitting a tuple, ignore values with .none ownership, not just trivial values.
Otherwise, if we have a tuple whose first parameter is a .none ownership non-trivial enum, we will leak the other parameters due to us not providing a cleanup on the created tuple. rdar://56806959
1 parent 8b3a15b commit 1953690

File tree

2 files changed

+54
-4
lines changed

2 files changed

+54
-4
lines changed

lib/SILGen/SILGenBuilder.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -767,14 +767,15 @@ ManagedValue SILGenBuilder::createTuple(SILLocation loc, SILType type,
767767
return ManagedValue::forUnmanaged(result);
768768
}
769769

770-
// We need to look for the first non-trivial value and use that as our cleanup
771-
// cloner value.
770+
// We need to look for the first value without .none ownership and use that as
771+
// our cleanup cloner value.
772772
auto iter = find_if(elements, [&](ManagedValue mv) -> bool {
773-
return !mv.getType().isTrivial(getFunction());
773+
return mv.getOwnershipKind() != ValueOwnershipKind::None;
774774
});
775775

776776
llvm::SmallVector<SILValue, 8> forwardedValues;
777-
// If we have all trivial values, then just create the tuple and return. No
777+
778+
// If we have all .none values, then just create the tuple and return. No
778779
// cleanups need to be cloned.
779780
if (iter == elements.end()) {
780781
llvm::transform(elements, std::back_inserter(forwardedValues),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// RUN: %target-swift-emit-silgen %s
2+
//
3+
// Just make sure that we do not trigger the ownership verifier on this code. We
4+
// were previously not emitting a destroy_value for (nil, error) since we were
5+
// seeing the .none for [String: Any]? and propagating that values ownership
6+
// rather than the error.
7+
8+
public enum Outcome<T> {
9+
case success(T)
10+
case error(T?, Error)
11+
}
12+
13+
public protocol RequestContentRepresentable {
14+
}
15+
16+
public class HttpClient {
17+
public func fetch(requestContent: RequestContentRepresentable, completionHandler: @escaping (Outcome<[String: Any]>) -> Void) throws {
18+
fatalError()
19+
}
20+
}
21+
22+
public final class Future <ResultType> {
23+
@discardableResult
24+
public func finish(result: ResultType) -> Bool {
25+
fatalError()
26+
}
27+
}
28+
29+
class Controller {
30+
internal func test() {
31+
let content2: RequestContentRepresentable? = nil
32+
let content = content2!
33+
let httpClient2: HttpClient? = nil
34+
let httpClient: HttpClient = httpClient2!
35+
36+
// Create a Future to encapsulate the response handler.
37+
// This allows us to guarantee we only call it once.
38+
// We set the handler in the success block and we fail the future if we should no longer be allowed to call the completion
39+
let futureResponseHandler = Future<([String: Any]?, Error?)>()
40+
41+
do {
42+
try httpClient.fetch(requestContent: content) { (outcome) in
43+
}
44+
} catch let error {
45+
// This is calling the future's success handler with a tuple.
46+
futureResponseHandler.finish(result: (nil, error))
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)