The `bare` attribute indicates that the object header is not used throughout the lifetime of the object.
This means, no reference counting operations are performed on the object and its metadata is not used.
The header of bare objects doesn't need to be initialized.
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.
Some notes:
1. I implemented this as a contextual keyword that can only apply directly to
lvalues. This ensures that we can still call functions called copy, define
variables named copy, etc. I added tests for both the c++ and swift-syntax based
parsers to validate this. So there shouldn't be any source breaks.
2. I did a little bit of type checker work to ensure that we do not treat
copy_expr's result as an lvalue. Otherwise, one could call mutating functions on
it or assign to it, which we do not want since the result of copy_value is
3. As expected, by creating a specific expr, I was able to have much greater
control of the SILGen codegen and thus eliminate extraneous copies and other
weirdness than if we used a function and had to go through SILGenApply.
rdar://101862423
This ensures that given a class that contains a noncopyable type that contains
another noncopyable type:
```
@_moveOnly struct S2 {}
@_moveOnly struct S { var s2: S2 }
class C { var s: S }
```
if we call a resilient function that takes C.S.S2:
```
borrowVal(c.s.s2)
```
we properly spill s2 onto the stack using a store_borrow.
Why Do This?
------------
Currently SILGenLValue treats ref_element_addr as a base that it needs to load
from for both copyable and non-copyable types. We keep a separation of concerns
and require emission of resilient functions to handle these loaded values. For
copyable types this means copying the value and storing it into a temporary
stack allocation. For noncopyable types, we never actually implemented this so
we would hit an error in SILGenApply telling us that our resilient function
expected an address argument, but we are passing an object.
To work around this, I updated how we emit borrowed lvalue arguments to in this
case to spill the value into a temporary allocation using a store_borrow. I also
included a test that validates that we properly have a read exclusivity scope
around the original loaded from memory for the entire call site so even though
we are performing a load_borrow and then spilling it, we still have read
exclusivity to the original memory for the entire region meaning that we still
preserve the semantics.
rdar://109171001
The reason why I am doing this is that otherwise if one has a function that
takes both a guaranteed and an owned parameter, we will break OSSA invariants
since the load [take] will invalidate the load_borrow. So instead, we put in a
load_borrow knowing that the move checker will convert it to a load_borrow
assuming that the two pass exclusivity checking.
NOTE: Because of some missing functionality in subsequent tests, I had to
disable one test (moveonly_escaping_definite_initialization.swift) and also add
some checks for copy of noncopyable object errors. They will go away in the next
2 commits.
rdar://108510987
This patch replaces the stateful generation of SILScope information in
SILGenFunction with data derived from the ASTScope hierarchy, which should be
100% in sync with the scopes needed for local variables. The goal is to
eliminate the surprising effects that the stack of cleanup operations can have
on the current state of SILBuilder leading to a fully deterministic (in the
sense of: predictible by a human) association of SILDebugScopes with
SILInstructions. The patch also eliminates the need to many workarounds. There
are still some accomodations for several Sema transformation passes such as
ResultBuilders, which don't correctly update the source locations when moving
around nodes. If these were implemented as macros, this problem would disappear.
This necessary rewrite of the macro scope handling included in this patch also
adds proper support nested macro expansions.
This fixes
rdar://88274783
and either fixes or at least partially addresses the following:
rdar://89252827
rdar://105186946
rdar://105757810
rdar://105997826
rdar://105102288
The verify-di-hole SILVerifier pass is very useful in catching incorrectly set
SILDebugScopes, but it can be very tedious to track down the root cause of a
verification failure. This patch replicates much of its functionality inside an
assertion in SILBuilder, which will result in a backtrace pointing directly to
where the offending scope is being set.
Although nonescaping closures are representationally trivial pointers to their
on-stack context, it is useful to model them as borrowing their captures, which
allows for checking correct use of move-only values across the closure, and
lets us model the lifetime dependence between a closure and its captures without
an ad-hoc web of `mark_dependence` instructions.
During ownership elimination, We eliminate copy/destroy_value instructions and
end the partial_apply's lifetime with an explicit dealloc_stack as before,
for compatibility with existing IRGen and non-OSSA aware passes.
Consider the following example:
```
class Klass {}
@_moveOnly struct Butt {
var k = Klass()
}
func mixedUse(_: inout Butt, _: __owned Butt) {}
func foo() {
var y = Butt()
mixedUse(&y, y)
}
```
In this case, we want to have an exclusivity violation. Before this patch, we
did a by-value load [copy] of y and then performed the inout access. Since the
access scopes did not overlap, we would not get an exclusivity violation.
Additionally, since the checker assumes that exclusivity violations will be
caught in such a situation, we convert the load [copy] to a load [take] causing
a later memory lifetime violation as seen in the following SIL:
```
sil hidden [ossa] @$s4test3fooyyF : $@convention(thin) () -> () {
bb0:
%0 = alloc_stack [lexical] $Butt, var, name "y" // users: %4, %5, %8, %12, %13
%1 = metatype $@thin Butt.Type // user: %3
// function_ref Butt.init()
%2 = function_ref @$s4test4ButtVACycfC : $@convention(method) (@thin Butt.Type) -> @owned Butt // user: %3
%3 = apply %2(%1) : $@convention(method) (@thin Butt.Type) -> @owned Butt // user: %4
store %3 to [init] %0 : $*Butt // id: %4
%5 = begin_access [modify] [static] %0 : $*Butt // users: %7, %6
%6 = load [take] %5 : $*Butt // user: %10 // <————————— This was a load [copy].
end_access %5 : $*Butt // id: %7
%8 = begin_access [modify] [static] %0 : $*Butt // users: %11, %10
// function_ref mixedUse2(_:_:)
%9 = function_ref @$s4test9mixedUse2yyAA4ButtVz_ADntF : $@convention(thin) (@inout Butt, @owned Butt) -> () // user: %10
%10 = apply %9(%8, %6) : $@convention(thin) (@inout Butt, @owned Butt) -> ()
end_access %8 : $*Butt // id: %11
destroy_addr %0 : $*Butt // id: %12
dealloc_stack %0 : $*Butt // id: %13
%14 = tuple () // user: %15
return %14 : $() // id: %15
} // end sil function '$s4test3fooyyF'
```
Now, instead we create a [consume] access and get the nice exclusivity error we
are looking for.
NOTE: As part of this I needed to tweak the verifier so that [deinit] accesses
are now allowed to have any form of access enforcement before we are in
LoweredSIL. I left in the original verifier error in LoweredSIL and additionally
left in the original error in IRGen. The reason why I am doing this is that I
need the deinit access to represent semantically what consuming from a
ref_element_addr, global, or escaping mutable var look like at the SIL level so
that the move checker can error upon it. Since we will error upon such
consumptions in Canonical SIL, such code patterns will never actually hit
Lowered/IRGen SIL, so it is safe to do so (and the verifier/errors will help us
if we make any mistakes). In the case of a non-escaping var though, we will be
able to use deinit statically and the move checker will make sure that it is not
reused before it is reinitialized.
rdar://101767439
- SILPackType carries whether the elements are stored directly
in the pack, which we're not currently using in the lowering,
but it's probably something we'll want in the final ABI.
Having this also makes it clear that we're doing the right
thing with substitution and element lowering. I also toyed
with making this a scalar type, which made it necessary in
various places, although eventually I pulled back to the
design where we always use packs as addresses.
- Pack boundaries are a core ABI concept, so the lowering has
to wrap parameter pack expansions up as packs. There are huge
unimplemented holes here where the abstraction pattern will
need to tell us how many elements to gather into the pack,
but a naive approach is good enough to get things off the
ground.
- Pack conventions are related to the existing parameter and
result conventions, but they're different on enough grounds
that they deserve to be separated.
I am adding this to make it easy to determine if a SILFunction that is not inout
aliasable is captured. This is useful when emitting certain types of
diagnostics like I need to emit with move only.
The current diagnostic that we are emitting is not perfect, but at least this
prevents us from saying that an error is not occuring here. I am going to file a
bug to track the QoI work of improving the diagnostic.
rdar://103313305
`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
Also renaming it to be UncheckedConversionComponent since it's a better name.
As a physical component, we'd run into problems in assignment statements.
The problem was that if we had something like:
```
SomeOtherComponent // first component
GetterSetterComponent
ABISafeConversionComponent // last component
```
When emitting the assignment, we always drill down through all but
the last component by calling `project()` on each one. Then on the last
component, we'd do the actual setting operation. But GetterSetterComponent
cannot be projected when the access is for writing.
So, to work around this I decided to model it as a TranslationComponent, because
those are specifically designed to be handled during an assignment by popping those
off the end of the component sequence, untranslating the value we're about to assign
as we go, until we hit the GetterSetterComponent.
By "untranslating" we're effectively putting Sendable back onto the set's argument
prior to calling set, because the underlying property's type still has `@Sendable`
on it (e.g., it's accessors still have that on its argument type). When
"translating" we're effectively taking Sendable off after reading it.
I think actually works really well and makes much more sense now.
resolves rdar://99619834
Some notes:
1. I added support for both loadable/address only types.
2. These tests are based off of porting the move only object tests for inout,
vars, mutating self, etc.
3. I did not include already written tests for address only types in this
specific merge since I need to change us to borrow move only var like types.
Without that, we get a lot of spurious error msgs and the burden of writing that
is not worth it. So instead in a forthcoming commit where I fix that issue in
SILGen, I will commit the corresponding address only tests for this work.
4. I did not include support for trivial types in this. I am going to do
object/address for that at the same time.
This change ensures all store_borrows are ended with an end_borrow, and uses of the store_borrow
destination are all in the enclosing store_borrow scope and via the store_borrow return address.
Fix tests to reflect new store_borrow pattern
Andy some time ago already created the new API but didn't go through and update
the old occurences. I did that in this PR and then deprecated the old API. The
tree is clean, so I could just remove it, but I decided to be nicer to
downstream people by deprecating it first.
Specifically this means that rather than always being owned, we now have owned
and guaranteed versions of copyable_to_moveonlywrapper. Similar to
moveonlywrapper_to_copyable, one chooses which variant one gets by using
specific SILBuilder APIs:
create{Owned,Guaranteed}CopyableToMoveOnlyWrapperValueInst. It is still
forwarding and the rest of the forwarding APIs work as expected except that the
forwarding ownership is fixed (and an assertion will result if one attempts to
do so).
NOTE: It is assumed that trivial operands are always passed to the owned
variant.
The reason why this is necessary is that the trivial input value doesn't have a
cleanup associated with it, but the result of the instruction is owned. So
otherwise we will not have a cleanup for the result, resulting in an OSSA error.
I discovered this as I began to write some Interpreter tests for move only. This
was triggered by SILGen attempting to pass a non-trivial type to StringBuilder
which is generic, so we needed to materialize.
I also discovered a small bug where we were not properly ignoring class_method
in the move only type eliminator. I just folded the small fix + a test for that
into this commit.
This involved doing the following:
1. Update the move only checker to look for new patterns.
2. Teach emitSemanticStore to use a moveonlywrapper_to_copyable to store moveonly
values into memory. The various checkers will validate that this code is
correct.
3. When emitting an apply, always unwrap move only variables. In the
future, I am going to avoid this if a parameter is explicitly marked as also
being moveonly (e.x.: @moveOnly parameter or @noEscape argument).
4. Convert from moveOnly -> copyable on return inst automatically in SILGen.
5. Fix SILGenLValue emission so we emit an error diagnostic later rather than
crash. This is needed to keep SILGen emitting move only addresses (that is no
implicit copy address only lets) in a form that the move only checker then
will error upon. Without this change, SILGen crashes instead of emitting an
error diagnostic in the following test:
.//test/SILOptimizer/move_only_checker_addressonly_fail.swift
Reduces the number of _ContiguousArrayStorage metadata.
In order to support constant time bridging we do need to set the correct
metadata when we bridge to Objective-C. This is so that the type check
succeeds when bridging back from Objective-C to reuse the storage
instance rather than bridging the elements.
To support dynamically setting the `_ContiguousArrayStorage` element
type i needed to add support for optimizing `alloc_ref_dynamic`
throughout the optimizer.
Possible future improvements:
* Use different metadata such that we can disambiguate native Swift
classes during destruction -- allowing native release rather then unknown
release usage.
* Optimize the newly added semantic function
getContiguousArrayStorageType
rdar://86171143
The reason why I am doing this is that currently checked_cast_br of an AnyObject
may return a different value due to boxing. As an example, this can occur when
performing a checked_cast_br of a __SwiftValue(AnyHashable(Klass())).
To model this in SIL, I added to OwnershipForwardingMixin a bit of information
that states if the instruction directly forwards ownership with a default value
of true. In checked_cast_br's constructor though I specialize this behavior if
the source type is AnyObject and thus mark the checked_cast_br as not directly
forwarding its operand. If an OwnershipForwardingMixin directly forwards
ownership, one can assume that if it forwards ownership, it will always do so in
a way that ensures that each forwarded value is rc-identical to whatever result
it algebraically forwards ownership to. So in the context of checked_cast_br, it
means that the source value is rc-identical to the argument of the success and
default blocks.
I added a verifier check to CheckedCastBr that makes sure that it can never
forward guaranteed ownership and have a source type of AnyObject.
In SemanticARCOpts, I modified the code that builds extended live ranges of
owned values (*) to check if a OwnershipForwardingMixin is directly
forwarding. If it is not directly forwarding, then we treat the use just as an
unknown consuming use. This will then prevent us from converting such an owned
value to guaranteed appropriately in such a case.
I also in SILGen needed to change checked_cast_br emission in SILGenBuilder to
perform an ensurePlusOne on the input operand (converting it to +1 with an
appropriate cleanup) if the source type is an AnyObject. I found this via the
verifier check catching this behavior from SILGen when compiling libswift. This
just shows how important IR verification of invariants can be.
(*) For those unaware, SemanticARCOpts contains a model of an owned value and
all forwarding uses of it called an OwnershipLiveRange.
rdar://85710101
I am purposely doing this in SILGen rather than at the type system level to
avoid having to have to add a bunch of boilerplate to the type system. Instead
of doing that, I am in SILGen checking for the isNoImplicitCopy bit on the
ParamDecl when we emit arguments. At that point, I set on the specific
SILArgument being emitted the bit that it is no implicit copy. In terms of
printing at the SIL level, I just printed it in front of the function argument
type like @owned, e.x.:
func myFunc(_ x: @_noImplicitCopy T) -> T {
...
}
becomes:
bb0(%0 : @noImplicitCopy @owned $T):
Some notes:
* Just to be explicit, I am making it so that no implicit copy parameters by
default are always passed at +1. The reason why I think this makes sense is
that this is the natural way of working with a move only value.
* As always, one can not write no implicit copy the attribute without passing
the flag -enable-experimental-move-only so this is NFC.
rdar://83957088
Previsouly we were evaluating a tuple elt and then performing the relevant
sub-initialization. The problem is that a sub-initialization can invoke code
that could perform an early exit cleanup. So any later tuple-elements that may
need to be cleaned up along such path will not have had their cleanups
initialized, resulting in a leak along such paths.
With this commit, we instead evaluate all of the tuple elements and only them
perform the sub-initialization ensuring that any early exits clean up all of the
tuple elements.
rdar://83770295
Use APIs for creating terminator results that handle forwarding
ownership consistently.
Add ManagedValue::forForwardedRValue(SILValue) to handle cleanups
consistently based on ownership forwarding.
Add SILGenBuilder::createForwardedTermResult(SILType type) for
creating termator results with the correct ownership and cleanups.
Add SILGenBuilder::createTermResult(SILType type, ValueOwnershipKind
ownership) that handles cleanup based on terminator result ownership.
Add SILGenBuilder::createOptionalSomeResult(SwitchEnumInst) so a lot
of code doesn't need to deal with unwrapping Optional types,
terminator results, and ownership rules.
Replace the existing "phi" APIs with a single
SILGenBuilder::createPhi(SILType, ValueOwnershipKind) that handles
cleanup based on phi ownership.
Phis and terminator results are fundamentally different and need to be handled differently everywhere. Remove the confusion where terminator results were generated with a "phi argument" API.
This makes it easier to understand conceptually why a ValueOwnershipKind with
Any ownership is invalid and also allowed me to explicitly document the lattice
that relates ownership constraints/value ownership kinds.
The Original Bug
----------------
In ffbfcfa131, we fixed a bug around implicit
value initializers but did not cherry-pick it to 5.3. While investigating a bug
that turned out to be that same bug (no worries!), I noticed that there is
additional code that is "unsafely" correct in this area and that while
ffbfcfa131 is correct in the small, we can expand
on the fix to prevent future bugs.
The Larger Bug
--------------
Here we are still open coding using ManagedValue/Cleanup APIs /without/ a top
level function scope. The code is only correct since we never emit unconditional
cleanups and always manually forward conditional cleanups. If we did not do
either of these things, we would have another instance of this bug, namely a
cleanup that is never actually emitted. So the code on master today is correct,
albeit unsafe, and we already have coverage for this (namely the test case from
ffbfcfa131).
That being said, in general when working with ManagedValue APIs (especially in
utility functions) we assume that we have a scope already created for us by our
caller. So by fixing this issue we are standardizing to safer SILGen invariants.
Building on ffbfcfa131
----------------------------------------------------
This commit builds on the shoulders of ffbfcfa131
by adding the function level scope mentioned in the previous section so that we
are now "safely" correct.
While looking at this I also realized that just using a normal scope when open
coding here may be a bit bugprone for open coding situations like this since:
1. If one just creates a scope in open coding situations, the scope will fire at
end of the c++ function /after/ one has probably emitted a return.
2. Once one has emitted the return, the insertion point will no longer be set
implying =><=.
To avoid this, I created a move only composition type on top of Scope called
AssertingManualScope. This type just asserts in its destructor if the scope it
contains has not been popped yet.
While, one can pop it by ones self, I added an overload of createReturnInst on
SILGenBuilder that also takes an AssertingManualScope and pops it at the
appropriate time.
So now when performing simple open coding tasks, we have the ability to in code
tie together the function level scope to the actual creation of return inst,
simulating the hand-off of lifetimes/resources from caller/callee that often
happens in the epilog of functions.
<rdar://problem/63189210>
This means that it can only have a guaranteed object as an operandand that we
validate that all uses of the result address of open_existential_box occur only
within the lifetime of said object's borrow scope.
(BaseT, @inout @unowned(unsafe) T) -> @guaranteed T
The reason for the weird signature is that currently the Builtin infrastructure
does not handle results well. Also, note that we are not actually performing a
call here. We are SILGening directly so we can create a guaranteed result.
The intended semantics is that one passes in a base value that guarantees the
lifetime of the unowned(unsafe) value. The builtin then:
1. Borrows the base.
2. Loads the trivial unowned (unsafe), converts that value to a guaranteed ref
after unsafely unwrapping the optional.
3. Uses mark dependence to tie the lifetimes of the guaranteed base to the
guaranteed ref.
I also updated my small UnsafeValue.swift test to make sure we get the codegen
we expect.
For those who are unaware, a transformation terminator is a terminator like
switch_enum/checked_cast_br that always dominate their successor blocks. Since
they dominate their successor blocks by design and transform their input into
the args form, we can validate that they obey guaranteed ownership semantics
just like a forwarding instruction.
Beyond removing unnecessary code bloat, this also makes it significantly more
easier to optimize/work with transformation terminators when converting @owned
-> @guaranteed since we do not need to find end_borrow points when the owned
value is consumed.
<rdar://problem/59097063>
