This is an extension of a similar problem that I had fixed earlier where due to
the usage of intermediate Sendable types we do not propagate regions correctly.
The previous issue I fixed was that we were not properly tieing the result of a
foreign async completion handler to the block storage since we used an
intervening UnsafeContinuation (which is Sendable) to propagate the result into
the block storage. I fixed this by changing SILGen to insert a
merge_isolation_region that explicitly ties the result to the block storage.
This new issue is that the block that we create and then pass as the completion
handler is an @Sendable block. Thus when we call the actual objc_method, the
block storage and self are not viewed as being in the same region. In this PR, I
change it so that we add a merge_isolation_region from self onto the block
storage.
The end result of this is that we have that self, the result of the call, and
the block storage are all in the same region meaning that we properly diagnose
that returning an NSObject from the imported Objective-C function is task
isolated and thus we cannot return it as a sending result.
rdar://131422332
Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
Reformatting everything now that we have `llvm` namespaces. I've
separated this from the main commit to help manage merge-conflicts and
for making it a bit easier to read the mega-patch.
This is phase-1 of switching from llvm::Optional to std::optional in the
next rebranch. llvm::Optional was removed from upstream LLVM, so we need
to migrate off rather soon. On Darwin, std::optional, and llvm::Optional
have the same layout, so we don't need to be as concerned about ABI
beyond the name mangling. `llvm::Optional` is only returned from one
function in
```
getStandardTypeSubst(StringRef TypeName,
bool allowConcurrencyManglings);
```
It's the return value, so it should not impact the mangling of the
function, and the layout is the same as `std::optional`, so it should be
mostly okay. This function doesn't appear to have users, and the ABI was
already broken 2 years ago for concurrency and no one seemed to notice
so this should be "okay".
I'm doing the migration incrementally so that folks working on main can
cherry-pick back to the release/5.9 branch. Once 5.9 is done and locked
away, then we can go through and finish the replacement. Since `None`
and `Optional` show up in contexts where they are not `llvm::None` and
`llvm::Optional`, I'm preparing the work now by going through and
removing the namespace unwrapping and making the `llvm` namespace
explicit. This should make it fairly mechanical to go through and
replace llvm::Optional with std::optional, and llvm::None with
std::nullopt. It's also a change that can be brought onto the
release/5.9 with minimal impact. This should be an NFC change.
As I've been iterating on this work, I've been gradually mulling these
over, and I think this is the way to go for now. These should make it
a lot less cumbersome to write these kinds of traversals correctly.
The intent is to the sunset the existing expanded-components stuff
after I do a similar pass for function parameters.
This is all relatively nicely abstracted, which is not to say that
it didn't take an awful lot of plumbing to get it to work. The basic
problem here is inherent: we need to do component-specific setup and
teardown, and unfortunately in the current representation we have to
do that with separate loops and without any dominance relationships.
(This is the same thing preventing us from doing borrows in the
general case.) The result is that the general case of result emission
is to emit every element of the expansion into a temporary tuple
(requiring a pack loop before the call to initialize the pack), then
process those elements in the body of a second pack loop after the
call. And that's terrible enough that we really have to do the work
to try to avoid it, which makes all the APIs more complicated.
Anyway, most of the way through the basic plumbing for variadic
generics now. Next is reabstraction, I think, which I hope will
mostly mean fixing bugs in the infrastructure I've already written.
when two objc async functions are composed with each other,
i.e., f(g()), then the clean-ups for g() would get emitted
at an unexpected time, namely, during the suspension for
the call to f(). This means that using a clean-up to emit
the executor-hop breadcrumb was incorrect. The hop could
appear between a get_async continuation and its matching
await_continuation, which is an unsupported nested suspension.
This commit fixes that by removing the use of the breadcrumb
clean-up in favor of providing that breadcrumb directly to
the result plan, so that it may be emitted later on when the
result plan sees fit.
Fixes rdar://91502776
Previously, SILGen assumed that a foreign function could either have a
foreign async convention or a foreign error convention, but if it had
both, the error would be subsumed into the completion. That resulted in
failures to emit code for async calls of functions like
```
- (BOOL)minimalWithError:(NSError* _Nullable*)error
completionHandler:(void (^ _Nonnull)(void))completionHandler;
```
Here, SILGen gains the ability to emit such functions. To enable that,
a few changes were required when both conventions are present:
- a separate argument for each convention is used
- the ResultPlan is a ForeignErrorResultPlan nesting a
ForeignAsyncResultPlan
- the continuation is always of the form UnsafeContinuation<_, Error>
regardless of whether the completion handler takes an error
- the foreign error block fills the continuation with the error that was
passed by reference out of the ObjC method call
- the foreign error block branches to the block containing the await
instruction
rdar://80704984
Address a FIXME where lowered types rather than formal types were used
when converting from objc to native types which resulted in a failure to
convert block types.
Immediately before invoking the ObjC API, get the current continuation, capture it into a block to
pass as the completion handler, and then await the continuation, whose resume/error successors
serve as the semantic return/throw result of the call. This should complete the caller-side part
of SILGen; the completion handler block implementation is however still only a stub.
I am going to be cleaning up some of the code here so that I can move the
creation of a temporary needed for foreign error handling earlier and move the
handling of the error parameter in general into ResultPlanBuilder.
Otherwise, the temporary is cleaned up when I pop a soon to be committed patch
that creates scopes around call sites.
rdar://30955427
ResultPlan and related constructs have already been written in a type erased
fashion. This commit takes advantage of this by moving the details of the code
to ResultPlan.{cpp,h}.
This slims down SILGenApply.cpp in a NFC way and ensures that other code in
SILGenApply can not depend on the details of ResultPlan. Also it is my hope that
this can become the beginning of the merging of the ResultPlan from SILGenApply
and from SILGenPoly. We should only have 1 way in SILGen to build applies and
create apply results.
rdar://29791263
