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)
There's an unfortunate layering difference in the cleanup order between address-only
and loadable error values during `catch` pattern matching: for address-only values,
the value is copied into a temporary stack slot, and the stack slot is cleaned up
on exit from the pattern match, meaning the value must be moved into the error return
slot on the "no catch" case before cleanups run. But if it's a loadable value, then
we borrow it for the duration of the switch, and the borrow is released during cleanup
on exit from the pattern match, so the value must be forwarded after running cleanups.
The way the code is structured, it handles these cases properly when the convention of
the function being emitted is in sync with the fundamental properties of the error type
(when the error type is loadable and the error return is by value, or when the error
type is address-only and the error return is indirect, in other words). But when
a closure literal with a loadable error type is emitted in an argument context that
expects a function with an indirect error return, we would try to forward the loadable
error value into the error return slot while a borrow is still active on it, leading
to verifier errors. Defer forwarding the value into memory until after cleanups are
popped, fixing rdar://126576356.
A tidier solution might be to always emit the function body to use a bbarg on the
throw block to pass the error value from the body emission to the epilog when the
type is loadable, deferring the move into memory to the epilog block. This would
make the right behavior fall out of the existing implementation, but would require
a bit more invasive changes (pretty much everywhere that checks IndirectErrorReturn
would need to check a different-tracked AddressOnlyErrorType bit instead or in
addition). This change is more localized.
We don't want the dispatch phase of a pattern match to invalidate the subject,
because we don't define the order in which patterns are evaluated, and if a
particular match attempt fails, we need to still have an intact subject value
on hand to try a potentially arbitrary other pattern against it. For
noncopyable types, this means we have to always emit the match phase as a
borrow, including the variable bindings for a guard expression if any.
For a consuming pattern match, end the borrow scope and reproject the variable
bindings by using consuming destructuring operations on the subject in the
match block.
For now, this new code path only handles single-case-label-per-block switches
without fallthroughs.
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.
This is largely a matter of changing the main loop over subst
params in TranslateArguments to use the generators I added,
then plugging back into the general reabstraction infrastructure.
Because we don't have pack coroutines, we're kind of stuck in
the code generation for pack reabstraction: we have to write
+1 r-values into a temporary tuple and then write those tuple
element addresses into the output pack. It's not great. We
also have lifetime problems with things like non-escaping
closures --- we have that problem outside of reabstraction
thunks, too.
Other than that glaring problem, I'm feeling relatively good
about the code here. It's missing some peepholes, but it should
work. But that that's not to say that arity reabstraction works
in general; my attempts to test it have been exposing some
problems elsewhere, and in particular the closure case crashes,
which is really bad. But this gets a few more things working,
and this PR is quite large already.
More missing infrastructure. In this case, it's really *existing*
missing infrastructure, though; we should have been imploding tuples
this way all along, given that we're doing it in the first place.
I don't like that we're doing all these extra tuple copies. I'm not
sure yet if they're just coming out of SILGen and eliminated immediately
after in practice; maybe so. Still, it should be obvious that they're
unnecessary.
`getValue` -> `value`
`getValueOr` -> `value_or`
`hasValue` -> `has_value`
`map` -> `transform`
The old API will be deprecated in the rebranch.
To avoid merge conflicts, use the new API already in the main branch.
rdar://102362022
This has been a long standing issue that we have been hacking around in various
points in SILGen. Now CleanupCloner just does the right thing.
I was unable to cause any issues to pop up in tree (since I believe we hacked
around this and converged on correctness). But it does come up in combination
with new code in https://github.com/apple/swift/pull/31779.
rdar://63514765
Previously we would just forward the cleanup and create a normal "destroy"
cleanup resulting in early deallocation and use after frees along error paths.
As part of this I also had to tweak how DI recognizes self init writebacks to
not use SILLocations. This is approach is more robust (since we aren't relying
on SourceLocs/have verifiers to make sure we don't violate SIL) and also avoids
issues from the write back store not necessarily have the same SILLocation.
<rdar://problem/59830255>
This simplifies CleanupCloner so that it only needs to model the cleanup of a
single managed value. It also eliminates a tie in between RValue and
CleanupCloner which is not needed.
This code was organized such that one had first definitions for CleanupManager,
then CleanupStateRestorationScope, and then again CleanupManager. This is just
confusing/hard to read. This commit fixes that by creating contiguous sections
of definitions with comment flags to document said sections.
For now, the accessors have been underscored as `_read` and `_modify`.
I'll prepare an evolution proposal for this feature which should allow
us to remove the underscores or, y'know, rename them to `purple` and
`lettuce`.
`_read` accessors do not make any effort yet to avoid copying the
value being yielded. I'll work on it in follow-up patches.
Opaque accesses to properties and subscripts defined with `_modify`
accessors will use an inefficient `materializeForSet` pattern that
materializes the value to a temporary instead of accessing it in-place.
That will be fixed by migrating to `modify` over `materializeForSet`,
which is next up after the `read` optimizations.
SIL ownership verification doesn't pass yet for the test cases here
because of a general fault in SILGen where borrows can outlive their
borrowed value due to being cleaned up on the general cleanup stack
when the borrowed value is cleaned up on the formal-access stack.
Michael, Andy, and I discussed various ways to fix this, but it seems
clear to me that it's not in any way specific to coroutine accesses.
rdar://35399664
This rename makes since since:
1. This is SILGen specific functionality.
2. In the next commit I am going to be adding a SIL SavedInsertionPoint class. I
want to make sure the two can not be confused.
This method verifies that all non-dead cleanup handles that a formal evaluation
scope would pop have not been popped yet.
The body of the function is if-defed out when asserts are disabled.
rdar://31313534
Previously, we were emitting these cleanups at the end of the lexical scope
instead of at the end of the formal evaluation scope. This change ensures that
we always emit the cleanup immediately at the end of the formal evaluation
scope.
Previously in most cases we got away with this due to the +0 self
hack. Basically we would emit a get for a self parameter and then immediately
use that self parameter as a guaranteed parameter. Then the hack would insert
the destroy value forwarding the lexical scope level cleanup at the same time.
rdar://29791263
We already have CleanupManager::dump() that dumps the entire cleanup
stack. Sometimes though when debugging you want to dump a specific cleanup. This
API provides such functionality.
This is useful to discover when a specific cleanup is being eliminated while
debugging. The implementation is compiled out when assertions are disabled.
rdar://29791263
Changes:
* Terminate all namespaces with the correct closing comment.
* Make sure argument names in comments match the corresponding parameter name.
* Remove redundant get() calls on smart pointers.
* Prefer using "override" or "final" instead of "virtual". Remove "virtual" where appropriate.
DeferCleanup pushes a new temporary cleanup to catch non-local returns
from the defer block, so we have to use stable iterators while emitting
cleanups.
There's no good deterministic test case for this -- it would manifest
as memory corruption if the underlying storage of the DiverseStack
grew beyond the inline storage. Add a reduced version of the original
user-reported test case that triggers it reliably -- I had a hard time
coming up with anything simpler.
Fixes <rdar://problem/21437203>.
Swift SVN r29658
The other part of rdar://problem/21444126. This is a little trickier since SIL doesn't track uses of witness tables in a principled way. Track uses in SILGen by putting a "SILGenBuilder" wrapper in front of SILBuilder, which marks conformances from apply, existential erasure, and metatype lookup instructions as used, so we can avoid emitting shared Clang importer witnesses when they aren't needed.
Swift SVN r29544
in terms of the pattern binding/emission facilities that are currently
used for switches. They are more general (handling all patterns,
not hacked up just for optionals).
This leads to us producing better code for if/let bindings, because we
don't alloc_stack a temporary and deal with memory for non-address-only
types (e.g. the common case of an optional pointer). Instead, the code
emits a select_enum{_addr} on the value.
While this changes the generated code in the compiler, there is no exposed
behavioral change to the developer.
Swift SVN r26142
rdar://problem/19450969
Undo reverts r24381..24384 with one fix: pull cleanup blocks from the
back of the list. When breaking out of a while loop, an extra release
could over-release a reference.
Swift SVN r24553
completely destroyed when forwarded.
Also, make forwarding a cleanup a first-class operation
on cleanups, rather than setting the cleanup state directly.
Swift SVN r19332
state of a cleanup to be restored after emitting
a path that may have e.g. activated it.
This should be used very carefully, because it makes
it quite simple to foul up your cleanup logic.
To be useful for temporarily deactivating cleanups,
we need a state that prevents forwarding from killing
the cleanup.
Swift SVN r19331
