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)
llvm::SmallSetVector changed semantics
(https://reviews.llvm.org/D152497) resulting in build failures in Swift.
The old semantics allowed usage of types that did not have an
`operator==` because `SmallDenseSet` uses `DenseSetInfo<T>::isEqual` to
determine equality. The new implementation switched to using
`std::find`, which internally uses `operator==`. This type is used
pretty frequently with `swift::Type`, which intentionally deletes
`operator==` as it is not the canonical type and therefore cannot be
compared in normal circumstances.
This patch adds a new type-alias to the Swift namespace that provides
the old semantic behavior for `SmallSetVector`. I've also gone through
and replaced usages of `llvm::SmallSetVector` with the
`Swift::SmallSetVector` in places where we're storing a type that
doesn't implement or explicitly deletes `operator==`. The changes to
`llvm::SmallSetVector` should improve compile-time performance, so I
left the `llvm::SmallSetVector` where possible.
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.
`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
Don't allow an owned call argument to be considered a valid BorrowingOperand.
More generally, make sure there is a perfect equivalence between valid
BorrowingOperand and the corresponding OperandOwnership kind.
This assert does not make sense because consumingUses in some cases
only contains the destroying uses. Owned values may not be destroyed
because they may be converted to ValueOwnershipKind::None on all paths
reaching a return. Instead, this utility needs to find liveness first
considering all uses (or at least all uses that may be on a lifetime
boundary). We probably then won't need this assert, but I'm leaving
the FIXME as a placeholder for that work.
Fixes rdar://71240363.
A reborrow occurs when a Borrowing Operand ends the lifetime of a borrowed value
and propagates forward a new guaranteed value that continues the guaranteed
lifetime of the value.
I think this assert was just testing the wrong thing. Specifically, it is trying
to say that either the given value has a consuming use or it is post-dominated
by dead end blocks.
Instead of checking that directly by using DeadEndBlocks::isDeadEnd(), it was
using a different empty check that caused the assert to actually semantically
say that:
A value must have a lifetime ending use *or* the dead end blocks analysis must
have found at least one block at all in the function that is reachable from a
function terminating terminator (e.x.: return).
This is clearly not the former and was causing the linear lifetime error to hit
this assert in certain cases.
<rdar://problem/70690127>
This updates how we model reborrow's lifetimes for ownership verification.
Today we follow and combine a borrow's lifetime through phi args as well.
Owned values lifetimes end at a phi arg. This discrepency in modeling
lifetimes leads to the OwnershipVerifier raising errors incorrectly for
cases such as this, where the borrow and the base value do not dominate
the end_borrow:
bb0:
cond_br undef, bb1, bb2
bb1:
%copy0 = copy_value %0
%borrow0 = begin_borrow %copy0
br bb3(%borrow0, %copy0)
bb2:
%copy1 = copy_value %1
%borrow1 = begin_borrow %copy1
br bb3(%borrow1, %copy1)
bb3(%borrow, %baseVal):
end_borrow %borrow
destroy_value %baseVal
This PR adds a new ReborrowVerifier. The ownership verifier collects borrow's
lifetime ending users and populates the worklist of the ReborrowVerifier
with reborrows and the corresponding base value.
ReborrowVerifier then verifies that the lifetime of the reborrow is
within the lifetime of the base value.
Otherwise, we can return true in cases where we do not have a proper linear
lifetime which can occur in the presence of destructures.
<rdar://problem/67698939>
Beyond allowing us to emit better errors, this will allow me to (in a subsequent
commit) count the amount of uses that are "outside" of the linear lifetime. I
can then compare that against a passed in set of "non consuming uses". If the
count of the number of uses "outside" of the linear lifetime equals the count of
the passed in set of "non consuming uses", then I know that /all/ non consuming
uses that I am testing against are not co-incident with the linear lifetime,
meaning that they can not effect (in a local, direct sense) the linear lifetime.
I am going to use that information to determine when it is safe to convert an
inout form a load [copy] to a load_borrow in the face of local mutations. I can
pass the set of local mutations as non-consuming uses to a linear lifetime
consisting of the load [copy]/destroy_values and thus prove that no writes occur
in between the load [copy]/destroy_value meaning it is safe to conver them to
borrow form.
NOTE: The aforementioned optimization is an extension of an optimization already
in tree that just bails if we have any writes to an inout locally, which is so
unfortunate.
This will just make them easier to update over time since adding a new function
doesn't require one to renumber the /entire/ file... *face-palm*.
Originally the reason why I added this is b/c I need to ensure that we handle
all errors exactly once so making sure we control the exact amount of emitted
errors is important. I can still do this with the new approach by just doing
per-function max error counts. Thus I also in this commit added FileCheck
patterns that implemented this scheme so now we have everything.
This is doing a few things with a simple change to use a builder:
1. It cleans up how we emit errors so we have a builder object that constructs
errors. The errors then just become dumb POD data that the builder vends to
callers that via the boolean values describe what errors were found.
2. Now that we have one place where we are actually emitting these errors, I
cleaned up how we emit the errors by normalizing the output so function names
are quoted the same.
3. I changed our error emission so that we emit a unique count of the errors as
we emit them. This makes it so that our pattern matching is much more robust
against weird pattern match errors that can be difficult to debug due to the
errors having unrelated test cases/file check patterns bleed
together. Before/end checks eliminate this problem. I updated all of the
relevant test cases.
The reason /why/ I am doing this though is that I am going to be adding support
to the LinearLifetimeChecker for flagging objects that are outside of the
lifetime that we are verifying (meaning either before or after). This is going
to cause me to need to track /all/ non consuming uses when performing linear
lifetime checks and thus most likely emit more errors. I was finding it to be
difficult to update the current tests given the state of the world before this
patch, so I was inspired to clean this up to satisfy practical as well as debt
concerns.
This verifier validates that while a load_borrow's value is live (that is until
it is invalidated by its end_borrow), the load_borrow's address source is never
written to.
The reason why this verifier is especially important now is that I am adding
many optimizations that convert `load [copy]` -> `load_borrow`. If that
optimization messes up, we break this invariant [in fact, an optimization I am
working on right now violated the invariant =--(]. So by adding this verifier I
am checking that semantic arc opts doesn't break it as well as eliminating any
other such bugs from the compiler (in the future).
Specifically, I split it into 3 initial categories: IR, Utils, Verifier. I just
did this quickly, we can always split it more later if we want.
I followed the model that we use in SILOptimizer: ./lib/SIL/CMakeLists.txt vends
a macro (sil_register_sources) to the sub-folders that register the sources of
the subdirectory with a global state variable that ./lib/SIL/CMakeLists.txt
defines. Then after including those subdirs, the parent cmake declares the SIL
library. So the output is the same, but we have the flexibility of having
subdirectories to categorize source files.
