This will make the forthcoming CanonicalizeInstruction interface more
clear.
This is generally the better approach to utilities that mutate the
instruction stream. It avoids the temptation to assume that only a
single instruction will be deleted or that only instructions before
the current iterator will be deleted. This often happens to work but
eventually fails in the presense of debug and end-of-scope
instructions.
A function returning an iterator has a more clear contract than one
accepting some iterator reference of unknown
providence. Unfortunately, it doesn't work at the lowest level of
utilities, such as recursivelyDeleteTriviallyDeadInstructions, where
we want to handle instruction batches.
This undoes some of Joe's work in 8665342 to add a guarantee: if an
@objc convenience initializer only calls other @objc initializers that
eventually call a designated initializer, it won't result in an extra
allocation. While Objective-C /allows/ returning a different object
from an initializer than the allocation you were given, doing so
doesn't play well with some very hairy implementation details of
compiled nib files (or NSCoding archives with cyclic references in
general).
This guarantee only applies to
(1) calling `self.init`
(2) where the delegated-to initializer is @objc
because convenience initializers must do dynamic dispatch when they
delegate, and Swift only stores allocating entry points for
initializers in a class's vtable. To dynamically find an initializing
entry point, ObjC dispatch must be used instead.
(It's worth noting that this patch does NOT check that the calling
initializer is a convenience initializer when deciding whether to use
ObjC dispatch for `self.init`. If we ever add peer delegation to
designated initializers, which is totally a valid feature, that should
use static dispatch and therefore should not go through objc_msgSend.)
This change doesn't /always/ result in fewer allocations; if the
delegated-to initializer ends up returning a different object after
all, the original allocation was wasted. Objective-C has the same
problem (one of the reasons why factory methods exist for things like
NSNumber and NSArray).
We do still get most of the benefits of Joe's original change. In
particular, vtables only ever contain allocating initializer entry
points, never the initializing ones, and never /both/ (which was a
thing that could happen with 'required' before).
rdar://problem/46823518
In Swift 5 mode, 'self' in a convenience init has a DynamicSelfType, so
the value_metatype instruction returns a DynamicSelfType metatype.
However, the metatype argument to the constructor is a plain class metatype
because it's an interface type, so we have to "open" it by bitcasting it
to a DynamicSelfType.
Fixes <https://bugs.swift.org/browse/SR-9430>, <rdar://problem/46982573>.
This was done early on during the split of predictable mem opts from DI. This
has been done for a long time, so eliminate the "Ownership" basename suffix.
When building on Windows, the std::string being passed will on the
stack, where the destructor will be invoked before the return, nil'ing
out reference. This causes incorrect behaviour when building the
diagnostic or FIXITs. Explicitly create the StringRef to prevent the
std::string from being invalidated.
Compiler passes that intermingle analysis with mutation of the CFG
are fraught with danger. The bug here was that a single AssignInst
could appear twice in DI's Uses list, once as a store use and once
as a load use.
When processing Uses, we would lower AssignInsts on the fly. We would
take care to erase the instruction pointer from the current Use, but
if a subsequent Use *also* referenced the now-deleted AssignInst, we
would crash.
Handle this in the standard way: instead of lowering assignments
right away, just build a list of assignments that we're planning on
lowering and process them all at the very end.
This has the added benefit of simplifying the code, because we no
longer have to work as hard to keep the state of the Uses list
consistent while lowering AssignInsts. The rest of DI should not
care that the lowering got postponed either, since it was already
expected to handle any ordering of elements in the Uses list, so
it could not assume that any particular AssignInst has been lowered.
Fixes <https://bugs.swift.org/browse/SR-9451>.
On Windows at least, the std::string associated with the name of the
property would be copy constructed before being passed to the diagnostic
engine. The resultant DiagnosticArgument in the InFlightDiagnostic
would hold a StringRef to the copy-constructed std::string. However,
because the arguments are inalloca'ed on Windows, the copy constructed
string would be destructed before the return of the argument.
Fortunately, this would result in the standard library overwriting the
string and the use-after-free would fail to print the argument.
Explicitly construct the StringRef before passing the name to the
diagnostic to avoid the use-after-free.
Previously, the `__consuming` decl modifier failed to get propagated to the value ownership of the
method's `self` parameter, causing it to effectively be a no-op. Fix this, and address some of the
downstream issues this exposes:
- `coerceCallArguments` in the type checker failing to handle the single `__owned` parameter case
- Various places in SILGen and optimizer passes that made inappropriate assertions that `self`
was always passed guaranteed
Before we changed convenience inits into allocating entry points, we allowed type(of: self) to be invoked on the uninitialized object, which was fine. This no longer Just Works when self doesn't even exist before `self.init` is called, but to maintain the old semantics and source compatibility, we can still just use the metatype value we were passed in.
Most of this patch is just removing special cases for materializeForSet
or other fairly mechanical replacements. Unfortunately, the rest is
still a fairly big change, and not one that can be easily split apart
because of the quite reasonable reliance on metaprogramming throughout
the compiler. And, of course, there are a bunch of test updates that
have to be sync'ed with the actual change to code-generation.
This is SR-7134.
The only real bug here is that we were looking specifically for `apply`
instructions, so we failed to diagnose `try_apply` calls to mutating
throwing functions on immutable existentials. Fixing this is a
source-compatibility break, but it's clearly in the "obvious bug" category
rather than something we need to emulate. (I found this bug because DI
stopped diagnosing a modification of a property of a `let` existential
when it started being done with `modify`, which of course is called with
`begin_apply`.)
It's totally fine to have a conditional destroy of 'self' because
we now uniformly treat assignment to self and self.init delegation,
both of which can appear multiple times in a value type
initializer.
This change removes the assertion and adds some tFileCheck tests to
ensure that DI was already capable of inserting the correct memory
management operations in this scenario.
Fixes <rdar://problem/40417944>, <https://bugs.swift.org/browse/SR-7727>.
This ensures that DI creates dealloc_box in cases where the box is uninitialized
conditionally.
In the process, I also discovered that we were missing a test case for DI being
used by LLDB. Long term we shouldn't support that code pattern in the general
case, but for now we at least need a test case for it.
rdar://40332620
I am doing this so I can start writing DI tests without this lowering occuring.
There never was a real reason for this code to be in DI beyond convenience. Now
it just makes writing tests more difficult. To prevent any test delta, I changed
all current DI tests to run this pass after DI.
closure lifetimes.
SILGen will now unconditionally emit
%cvt = convert_escape_to_noescape [guaranteed] %op
instructions. The mandatory ClosureLifetimeFixup pass ensures that %op's
lifetime spans %cvt's uses.
The code in DefiniteInitialization that handled a subset of cases is
removed.
To mark when a user of it is known to escape the value. This happens
with materializeForSet arguments which are captured and used in the
write-back. This means we need to keep the context alive until after
the write-back.
Follow-up patches to fully replace the PostponedCleanup hack in SILGen
by a mandatory SIL transformation pass to guarantee the proper lifetime
will use this flag to be more conservative when extending the lifetime.
The problem:
%pa = partial_apply %f(%some_context)
%cvt = convert_escape_to_noescape [not_guaranteed] [escaped] %pa
%ptr = %materialize_for_set(..., %cvt)
... write_back
... // <-- %pa needs to be alive until after write_back
SILGen does this now to maintain ownership balance when a class initializer delegation gets interrupted, such as by an error propagation through one of the arguments to the delegatee. Fixes rdar://problem/37007554 .
