[Definite Initialization] Fix a bug that made DI wrongly think that an unused access to "self" in a delegating initializer is a use of the form: `type(of:self)`
This at least ensures that we error immediately after processing the bad
root function. NOTE: we will inline recursively into it still, but at least we
stop before processing the entire module.
This was exposed by test/SILOptimizer/diagnostic_constant_propagation.swift. It
isn't a pattern in the mandatory inlining tests. I added some tests for it.
Specifically, when we optimize conversions such as:
Optional<@escaping () -> ()>
->
Optional<@noescape () -> ()>
->
Optional<@noescape @convention(block) () -> ()>
previously we were lifetime extending over the @noescape lifetime barrier by
making a copy and then putting a mark_dependence from the copy onto the original
value. This was just a quick way to tell the ownership verifier that the copy
was tied to the other value and thus should not be eliminated. The correctness
of the actual lifetime extension comes from the optimizer being conservative
around rr insts.
This commit instead changes our optimization to borrow the copied optional
value, extract the payload, and use that instead.
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.
I am doing this to eliminate some differences in codegen before/after
serialization ownership. It just means less of the tests need to be touched when
I flip the switch.
Specifically, today this change allows us to handle certain cases where there is
a dead allocation being used to pass around a value at +1 by performing a load
[take] and then storing a value back into the memory. The general format is an
allocation that only has stores, load [take], and destroy_addr users. Consider
the following SIL:
```
store %x to [init] %mem (0)
%xhat = load [take] %mem (1)
%xhat_cast = apply %f(%xhat) (2)
store %xhat_cast to [init] %mem (3)
destroy_addr %mem
```
Notice how assuming that we can get rid of the store, we can perform the
following store -> load forwarding:
```
%xhat_cast = apply %f(%x) (2)
store %xhat_cast to [init] %mem (3)
destroy_addr %mem
```
In contrast, notice how we get an ownership violation (double consume of %x by
(0) and (2)) if we can not get rid of the store:
```
store %x to [init] %mem
%xhat_cast = apply %f(%x)
store %xhat_cast to [init] %mem (2)
destroy_addr %mem
```
This is in fact the same condition for promoting a destroy_addr since when a
destroy_addr is a load [take] + destroy_value. So I was able to generalize the
code for destroy_addr to handle this case.
With the advent of dynamic_function_ref the actual callee of such a ref
my vary. Optimizations should not assume to know the content of a
function referenced by dynamic_function_ref. Introduce
getReferencedFunctionOrNull which will return null for such function
refs. And getInitialReferencedFunction to return the referenced
function.
Use as appropriate.
rdar://50959798
Previously, any function marked [dynamically_replaceable] that was
partially applied and captured by address would not be diagnosed.
This is a rare thing. For example:
struct S {
var x = 0
public mutating func testCallDynamic() {
dynamic func bar(_ i: inout Int) {
i = 1
x = 2
}
bar(&x)
}
}
Fixes <rdar://problem/50972786> Fix exclusivity diagnostics to be
aware of [dynamically_replaceable].
As a follow-up to reviewing the dynamically replaceable
implementation, document the place where this analysis will likely
crash in the future once we start optimizing non-escaping closure
captures.
Fixes the following warning:
```console
third_party/unsupported_toolchains/swift/src/swift/lib/SILOptimizer/Mandatory/OSLogOptimization.cpp:396:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^
```
An inout capture of 'self' is not lowered as a SIL argument inside
an initializer, so add a new check to handle this case.
Fixes <rdar://problem/50412872>.
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.
Cleaning up in preparation for making changes that improve
compile-time issues in AccessEnforcementOpts.
This is a simple but important enum with a handful of cases. The cases
need to be easily referenced from the header. Don't define them in a
separate .def. Remove the visitor biolerplate because it doesn't serve
any purpose.
This enum is meant to be used with covered switches. The enum cases do
not have their own types, so there's no performance reason to use a
Visitor pattern.
It should not be possible to add a case to this enum without carefully
considering the impact on the encoding of this class and the impact on
each and every one of the uses. We always want covered switches at the
use sites.
This is a major improvement in readability and usability both in the
definition of the class and in the one place where a visitor was used.
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.
If the final expression in a function or closure which is missing a
return has the appropriate type, rather than producing the usual
diagnostic about a missing return, produce a diagnostic with a fixit to
insert a return before thatn final expression.
h/t Nate Cook
This fixes a test involving transitive captures of local functions,
as well as an infinite recursion possible with the old code.
Fixes <rdar://problem/34496304>.
The new pass is based on existing asserts in DiagnoseStaticExclusivity.
They were compiled out in release builds and only checked for captures of
inout parameters. This patch converts the assertions into diagnostics and
adds checks for captures of non-escaping function values.
Unlike the Sema-based checks that this replaces, the new code handles
transitive captures from recursive local functions, which means certain
invalid code that used to compile will now be rejected with an error.
The new analysis also looks at the ultimate usages of a local function
instead of just assuming all local functions are escaping, which fixes
issues where the compiler would reject valid code.
Fixes a bunch of related issues, including:
- <rdar://problem/29403178>
- <https://bugs.swift.org/browse/SR-8546> / <rdar://problem/43355341>
- <https://bugs.swift.org/browse/SR-9043> / <rdar://problem/45511834>
