Handling address_to_pointer as a plain inout missed some mutations and lead to miscompiles.
We now treat address_to_pointer as escaping address.
Fixes SR-3554
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.
This was made redundant by typed boxes, and the type operand was already removed from textual SIL, but the field was never removed from the instruction's in memory representation. It becomes wrong in the face of compound boxes with layout.
This was already done for getSuccessorBlocks() to distinguish getting successor
blocks from getting the full list of SILSuccessors via getSuccessors(). This
commit just makes all of the successor/predecessor code follow that naming
convention.
Some examples:
getSingleSuccessor() => getSingleSuccessorBlock().
isSuccessor() => isSuccessorBlock().
getPreds() => getPredecessorBlocks().
Really, IMO, we should consider renaming SILSuccessor to a more verbose name so
that it is clear that it is more of an internal detail of SILBasicBlock's
implementation rather than something that one should consider as apart of one's
mental model of the IR when one really wants to be thinking about predecessor
and successor blocks. But that is not what this commit is trying to change, it
is just trying to eliminate a bit of technical debt by making the naming
conventions here consistent.
Before this commit all code relating to handling arguments in SILBasicBlock had
somewhere in the name BB. This is redundant given that the class's name is
already SILBasicBlock. This commit drops those names.
Some examples:
getBBArg() => getArgument()
BBArgList => ArgumentList
bbarg_begin() => args_begin()
This eliminates all inline creation of SILBasicBlock via placement new.
There are a few reasons to do this:
1. A SILBasicBlock is always created with a parent function. This commit
formalizes this into the SILBasicBlock API by only allowing for SILFunctions to
create SILBasicBlocks. This is implemented via the type system by making all
SILBasicBlock constructors private. Since SILFunction is a friend of
SILBasicBlock, SILFunction can still create a SILBasicBlock without issue.
2. Since all SILBasicBlocks will be created in only a few functions, it becomes
very easy to determine using instruments the amount of memory being allocated
for SILBasicBlocks by simply inverting the call tree in Allocations.
With LTO+PGO, normal inlining can occur if profitable so there shouldn't be
overhead that we care about in shipping compilers.
Today, loads and stores are treated as having @unowned(unsafe) ownership
semantics. This leaves the user to specify ownership changes on the loaded or
stored value independently of the load/store by inserting ARC operations. With
the change to Semantic SIL, this will no longer be true. Instead loads, stores
have ownership semantics that one must reason about such as copy, take, and
trivial.
This change moves us closer to that world by eliminating the default
OwnershipQualification argument from create{Load,Store}. This means that the
compiler developer cannot ignore reasoning about the ownership semantics of the
memory operation that they are creating.
Operationally, this is a NFC change since I have just gone through the compiler
and updated all places where we create loads, stores to pass in the former
default argument ({Load,Store}OwnershipQualifier::Unqualified), to
SILBuilder::create{Load,Store}(...). For now, one can just do that in situations
where one needs to create loads/stores, but over time, I am going to tighten the
semantics up via the verifier.
rdar://28685236
This is just old code that needs to be updated given changes in DebugInfo and
this should be a NFC change.
I am going to use this so that I can test a specific test case where DI emits
different errors depending on use-list ordering.
rdar://28851920
Extend the checks in `LifetimeChecker` in
`SILOptimizer/Mandatory/DefiniteInitialization.cpp`
to catch when the memory object corresponding to a let constant is used
as an inout parameter in a protocol witness method.
Initializing a let constant separate from its declaration requires write
access. Therefore it is treated as `@lvalue TestProtocol` in the AST for
a certain scope.
Checking that the constant is not written to after being initialized is
supposed to happen in the Mandatory SILOptimizer phase instead.
On loads, the Optimizer checks, that a variable is fully initialized,
but perviously did not validate what happens to it after it is loaded.
This allowed loading the memory object through an open_existential_addr
instruction and applying the result as an Operand to a mutating witness
method.
Previously, we were only able to detect factory initializers
dispatched through class_method. This didn't work for
factory initializers defined in protocol extensions.
The end result would be that we would strong_release an
uninitialized class instance, which could cause crashes.
Fix DI to correctly release the old instance using
dealloc_partial_ref instead.
Fixes <rdar://problem/27713221>.
In failable initializers the "return from initializer without initializing all stored properties" (resp. "return from enum initializer method without storing to 'self'" for enums) diagnostic is now reported at the early return instead of the end of the initializer.
We were giving special handling to ApplyInst when we were attempting to use
getMemoryBehavior(). This commit changes the special handling to work on all
full apply sites instead of just AI. Additionally, we look through partial
applies and thin to thick functions.
I also added a dumper called BasicInstructionPropertyDumper that just dumps the
results of SILInstruction::get{Memory,Releasing}Behavior() for all instructions
in order to verify this behavior.
As there are no instructions left which produce multiple result values, this is a NFC regarding the generated SIL and generated code.
Although this commit is large, most changes are straightforward adoptions to the changes in the ValueBase and SILValue classes.
This change is needed for the next update to ToT LLVM. It can be put
into place now without breaking anything so I am committing it now.
The churn upstream on ilist_node is neccessary to remove undefined
behavior. Rather than updating the different ilist_node patches for the
hacky change required to not use iterators, just use iterators and keep
everything as ilist_nodes. Upstream they want to eventually do this, so
it makes sense for us to just do it now.
Please do not introduce new invocations of
ilist_node::get{Next,Prev}Node() into the tree.
And use project_box to get to the address value.
SILGen now generates a project_box for each alloc_box.
And IRGen re-uses the address value from the alloc_box if the operand of project_box is an alloc_box.
This lets the generated code be the same as before.
Other than that most changes of this (quite large) commit are straightforward.
its fields are initialized. Before:
t.swift:18:24: error: variable 'self.B' captured by a closure before being initialized
after:
t.swift:19:24: error: 'self' captured by a closure before all members were initialized
self.A.withCString { cString -> () in
^
t.swift:14:7: note: 'self.B' not initialized
var B: Int
^
This drives home the fact that 'self' is being captured here, not the individual
properties.
When a root class delegates to a non-class-bound protocol method, the self value
gets wrapped up in a SIL alloc_stack so it can be passed by address. Recognize
that the store involved is doing this, so we can provide a more specific diagnostic.
Before this, we produced:
variable 'self.x' used before being initialized
Now we produce:
error: use of 'self' in method call 'getg' before all stored properties are initialized
note: 'self.x' not initialized
Having a separate address and container value returned from alloc_stack is not really needed in SIL.
Even if they differ we have both addresses available during IRGen, because a dealloc_stack is always dominated by the corresponding alloc_stack in the same function.
Although this commit quite large, most changes are trivial. The largest non-trivial change is in IRGenSIL.
This commit is a NFC regarding the generated code. Even the generated SIL is the same (except removed #0, #1 and @local_storage).
(libraries now)
It has been generally agreed that we need to do this reorg, and now
seems like the perfect time. Some major pass reorganization is in the
works.
This does not have to be the final word on the matter. The consensus
among those working on the code is that it's much better than what we
had and a better starting point for future bike shedding.
Note that the previous organization was designed to allow separate
analysis and optimization libraries. It turns out this is an
artificial distinction and not an important goal.
