Whenever we want to forward to a +1 value but don't need to destroy
the original memory, use isPlusOneOrTrivial.
This follows the existing naming scheme.
Fixes rdar://108001491 (SIL verification failed: Found mutating or
consuming use of an in_guaranteed parameter?!:
!ImmutableAddressUseVerifier().isMutatingOrConsuming(fArg))
Code can only locally interact with a mutable memory location within a
formal access, and is only responsible for maintaining its invariants
during that access, so the move-only address checker does not need to,
and should not, observe operations that occur outside of the access
marked with the `mark_must_check` instruction. And for immutable
memory locations, although there are no explicit formal accesses, that's
because every access must be read-only, so although individual
accesses are not delimited, they are all compatible as far as
move-only checking is concerned. So we can back out the changes to SILGen
to re-project a memory location from its origin on every access, a
change which breaks invariants assumed by other SIL passes.
This is the first slice of bringing up escaping closure support. The support is
based around introducing a new type of SILGen VarLoc: a VarLoc with a box and
without a value. Because the VarLoc only has a box, we have to in SILGen always
eagerly reproject out the address from the box. The reason why I am doing this
is that it makes it easy for the move checker to distinguish in between
different accesses to the box that we want to check separately. As such every
time that we open the box, we insert a mark_must_check
[assignable_but_not_consumable] on that project. If allocbox_to_stack manages to
determine that the box can be stack allocated, we eliminate all of the
mark_must_check and place a new mark_must_check [consumable_and_assignable] on
the alloc_stack. The end result is that we get the old model that we had before
and also can support escaping closures.
`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
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.
Previously we assumed the exit counter was the
same as the entry counter. Update the logic such
that we track break statements (including
implicitly at the end of a case), as well as
early returns and jumps to parent statements. This
follows a similar logic to what we do for if
statements.
rdar://99141044
`int_trap` doesn't provide a failure message, which makes crash reports hard to understand.
This is mostly the case for optimized casts which fail.
rdar://97681511
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.
This is just the very beginning... I still need to implement more parts of
SILGen for this. But all great things start small. I am going to iterate on top
of this and just wanted to get some initial parts of the work in as I go.
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
Added an AST helper in Types.h:
- isPotentiallyAnyObject()
This formalizes logic for when cast operations forward
ownership. Various OSSA optimization rely on this for
correctness. This fixes latent bugs throughout the optimizer.
I was compelled to fix this now because we want to make OSSA
optimizations across dynamic casts more aggressive. For example, we
want to optimize retain/release across enum formation.
This cleans up 90 instances of this warning and reduces the build spew
when building on Linux. This helps identify actual issues when
building which can get lost in the stream of warning messages. It also
helps restore the ability to build the compiler with gcc.
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.
My goal was to reduce the size of SILLocation. It now contains only of a storage union, which is basically a pointer and a bitfield containing the Kind, StorageKind and flags. By far, most locations are only single pointers to an AST node. For the few cases where more data needs to be stored, this data is allocated separately: with the SILModule's bump pointer allocator.
While working on this, I couldn't resist to do a major refactoring to simplify the code:
* removed unused stuff
* The term "DebugLoc" was used for 3 completely different things:
- for `struct SILLocation::DebugLoc` -> renamed it to `FilePosition`
- for `hasDebugLoc()`/`getDebugSourceLoc()` -> renamed it to `hasASTNodeForDebugging()`/`getSourceLocForDebugging()`
- for `class SILDebugLocation` -> kept it as it is (though, `SILScopedLocation` would be a better name, IMO)
* made SILLocation more "functional", i.e. replaced some setters with corresponding constructors
* replaced the hand-written bitfield `KindData` with C bitfields
* updated and improved comments
For the following discussion, let OP be the switch's operand. This is
implemented by:
* Modeling switch_enum_addr as not forwarding OP and instead delegate the
forwarding of OP to be done by each enum case. This matches what SILGen is
actually doing since the actual taking of the address in SIL is done in each
enum case block by an unchecked_take_enum_data_addr.
* In each enum case, I treat OP as being forwarded into an irrefutable
sub-tree. I follow the pattern of other places this is done by creating a
CleanupStateRestorationScope and using forwardIntoIrrefutableSubTree. This
ensures that if I forward OP in the enum case, it just becomes dormant instead
of being thrown away.
* Inside each case, there is a bunch of code that does some final preparations
to src before dispatching to the inner dispatch. This code was written using
old low-level SILValue APIs where ownership is done by hand. I replaced all of
that by hand ownership with higher level Managed Value APIs that automatically
handle ownership for the user. This simplified the implementation and ensured
correctness via SILGenBuilder API invariants.
The end result of all of these together is that:
1. The cleanup on OP is still live when we emit the default case later than the
cleanups. This eliminates the leak that I am fixing.
2. We have greater correctness since the SILGenBuilder APIs automatically handle
ownership for us.
3. We have eliminated some brittle logic that could in the future introduce
bugs. Specifically, I noticed that if we were ever given a
ConsumableManagedValue that was CopyOnSuccess or BorrowAlways but its
ManagedValue was a +1 value, we would leak. I could not figure out how to
create a Swift test case that would go down this code path though = (. But
that being said, it is one new language feature away from being broken. I
added some asserts to ConsumableManagedValue that ensures this invariant, so
we are safe. If it is inconvenient, we can also cause ConsumableManagedValue
to create a new ManagedValue when it detects this condition without the
cleanup. But lets see how difficult it is to keep this invariant.
In terms of testing, I put in both a SILGen test and also an end<->end
interpreter test to ensure this doesn't break again.
rdar://71992652
SR-13926
Since we are working with values here, it can be confusing when reading the code
that elt is an EnumElementDecl. The rename to eltDecl will make it much clearer.
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.
VarPattern is today used to implement both 'let' and 'var' pattern bindings, so
today is already misleading. The reason why the name Var was chosen was done b/c
it is meant to represent a pattern that performs 'variable binding'. Given that
I am going to add a new 'inout' pattern binding to this, it makes sense to
give it now a better fitting name before I make things more confusing.
* [SILGenFunction] Don't create redundant nested debug scopes
Instead of emitting:
```
sil_scope 4 { loc "main.swift":6:19 parent 3 }
sil_scope 5 { loc "main.swift":7:3 parent 4 }
sil_scope 6 { loc "main.swift":7:3 parent 5 }
sil_scope 7 { loc "main.swift":7:3 parent 5 }
sil_scope 8 { loc "main.swift":9:5 parent 4 }
```
Emit:
```
sil_scope 4 { loc "main.swift":6:19 parent 3 }
sil_scope 5 { loc "main.swift":7:3 parent 4 }
sil_scope 6 { loc "main.swift":9:5 parent 5 }
```
* [IRGenSIL] Diagnose conflicting shadow copies
If we attempt to store a value with the wrong type into a slot reserved
for a shadow copy, diagnose what went wrong.
* [SILGenPattern] Defer debug description of case variables
Create unique nested debug scopes for a switch, each of its case labels,
and each of its case bodies. This looks like:
```
switch ... { // Enter scope 1.
case ... : // Enter scope 2, nested within scope 1.
<body-1> // Enter scope 3, nested within scope 2.
case ... : // Enter scope 4, nested within scope 1.
<body-2> // Enter scope 5, nested within scope 4.
}
```
Use the new scope structure to defer emitting debug descriptions of case
bindings. Specifically, defer the work until we can nest the scope for a
case body under the scope for a pattern match.
This fixes SR-7973, a problem where it was impossible to inspect a case
binding in lldb when stopped at a case with multiple items.
Previously, we would emit the debug descriptions too early (in the
pattern match), leading to duplicate/conflicting descriptions. The only
reason that the ambiguous description was allowed to compile was because
the debug scopes were nested incorrectly.
rdar://41048339
* Update tests
Like switch cases, a catch clause may now include a comma-
separated list of patterns. The body will be executed if any
one of those patterns is matched.
This patch replaces `CatchStmt` with `CaseStmt` as the children
of `DoCatchStmt` in the AST. This necessitates a number of changes
throughout the compiler, including:
- Parser & libsyntax support for the new syntax and AST structure
- Typechecking of multi-pattern catches, including those which
contain bindings.
- SILGen support
- Code completion updates
- Profiler updates
- Name lookup changes
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>
Structurally prevent a number of common anti-patterns involving generic
signatures by separating the interface into GenericSignature and the
implementation into GenericSignatureBase. In particular, this allows
the comparison operators to be deleted which forces callers to
canonicalize the signature or ask to compare pointers explicitly.
Looks like a think-o; even if we see an address-only binding in a shared pattern block, we still
expect bb args in the lowered basic block for anything that comes after it. rdar://problem/53956564
Add `llvm_unreachable` to mark covered switches which MSVC does not
analyze correctly and believes that there exists a path through the
function without a return value.
In SILGenPattern, we need to be able to unforward cleanups when we explode
tuples. Thus we can't use RValue in SILGenPattern since it may implicitly
explode tuples (which without modifying RValue itself we can not
unforward). This patch removes a specific RValue usage that we can replace with
the use of a ManagedValue instead.
rdar://49903264
This is a large patch; I couldn't split it up further while still
keeping things working. There are four things being changed at
once here:
- Places that call SILType::isAddressOnly()/isLoadable() now call
the SILFunction overload and not the SILModule one.
- SILFunction's overloads of getTypeLowering() and getLoweredType()
now pass the function's resilience expansion down, instead of
hardcoding ResilienceExpansion::Minimal.
- Various other places with '// FIXME: Expansion' now use a better
resilience expansion.
- A few tests were updated to reflect SILGen's improved code
generation, and some new tests are added to cover more code paths
that previously were uncovered and only manifested themselves as
standard library build failures while I was working on this change.
Specifically, in order to generate runtime errors when we do not handle an enum
appropriately, we form a metatype from the input of the switch. The problem is
that by the time we get to the leaf of the emission tree where this metatype is
created, the input of the switch may have already been consumed. This means
creating the metatype could be a use after free.
This patch fixes the problem by emitting the value_metatype after we emit the
subject of the switch, but before we emit the switch itself.
rdar://49562761
Specifically the bad pattern was:
```
for (auto *vd : *caseStmt->getCaseBodyVariables()) { ... }
```
The problem is that the optional is not lifetime extended over the for loop. To
work around this, I changed the API of CaseStmt's getCaseBodyVariable methods to
never return the inner Optional<MutableArrayRef<T>>. Now we have the following 3
methods (ignoring const differences):
1. CaseStmt::hasCaseBodyVariables().
2. CaseStmt::getCaseBodyVariables(). Asserts if the case body variable array was
never specified.
3. CaseStmt::getCaseBodyVariablesOrEmptyArray(). Returns either the case body
variables array or an empty array if we were never given any case body
variable array.
This should prevent anyone else in the future from hitting this type of bug.
radar://49609717
