A directly applied noescape closure that captures a local requires
dynamic enforcement if that local escaped prior to the direct
application. Previously this was only statically enforced, which could
miss conflicts.
It's unlikely that an actual conflict could occur, but can happen in
cases like this:
func noescapeConflict() {
var localVal = 0
let nestedModify = { localVal = 3 }
_ = {
takeInoutAndPerform(&localVal, closure: nestedModify)
}()
}
Code isn't normally written like this, but the general pattern of
passing an escaping closure as an argument to a noescape closure may
arise when using withoutActuallyEscaping. Even in that case, it's
highly unlikely that a real conflict would exist.
For this case to be handled correcly, we must also never allow a
closure passed to withoutActuallyEscaping to be SILGen's as a
non-escaping closure. Doing so would result in a non-escaping closure
to be passed as an argument to the withoutActuallyEscaping trailing
closure, which is also non-escaping. That directly violates the
Non-Escaping Recursion Restriction, but there would be no way to
diagnose the violation, creating a hole in the exclusivity model.
This allows us to layout-optimize Optional<T> when T is a struct with an
extra-inhabitant-bearing field anywhere in its definition, not only at
the beginning. rdar://problem/43019427
Instead of just making sure IRGen correctly executes the fix
(performing fully fragile layout for all classes with
resiliently-sized fields, not just those with Objective-C ancestry)
actually add a test case for the reported bug -- that Objective-C
categories on classes with resiliently-sized fields work.
We cannot use field offset globals if *any* field of a generic class
with Objective-C ancestry is dependent.
This is because the Swift runtime first performs layout starting
from a static instance start offset, and then asks the Objective-C
runtime to slide the offsets based on the dynamic superclass size.
So if the class has a field of generic type, the alignment of that
type can change the offsets of fields *before* it as well as after.
So we cannot assuem that any fields in such a class have the same
offset across instantiations at all.
The previous fix captured the intent of the above, but it only
kicked in if the immediate superclass of the class was imported
from Objective-C. But really we need to do this for any class with
Objective-C ancestry.
While fixing this, re-organize the code in ClassLayoutBuilder a
little bit to untangle the stored property iteration from the
interesting FieldAccess adjustments that take place after.
As part of this, rename TypeMetadataRecordKind to TypeReferenceKind
and consistently give it three bits of storage.
The better modelling of these type references appears to have been
sufficient to make dynamic conformance checks succeed, which is good
but unexpected.
The central thrust of this patch is to get these metadata initializations
off of `swift_once` and onto the metadata-request system where we can
properly detect and resolve dependencies. We do this by first introducing
runtime support for resolving metadata requests for "in-place"
initializations (committed previously) and then teaching IRGen to actually
generate code to use them (this patch).
A non-trivial amount of this patch is just renaming and refactoring some of
existing infrastructure that was being used for in-place initializations to
try to avoid unnecessary confusion.
The remaining cases that are still using `swift_once` resolution of
metadata initialization are:
- non-generic classes that can't statically fill their superclass or
have resilient internal layout
- foreign type metadata
Classes require more work because I'd like to switch at least the
resilient-superclass case over to using a pattern much more like what
we do with generic class instantiation. That is, I'd like in-place
initialization to be reserved for classes that actually don't need
relocation.
Foreign metadata should also be updated to the request/dependency scheme
before we declare ABI stability. I'm not sure why foreign metadata
would ever require a type to be resolved, but let's assume it's possible.
Fixes part of SR-7876.
* Make _sanityCheck internal
* Make _debugPrecondition internal
* Make Optional._unsafelyUnwrappedUnchecked internal.
* Make _precondition internal
* Switch Foundation _sanityChecks to assertions
* Update file check tests
* Remove one more _debugPrecondition
* Update Optimization-with-check tests
For now, the accessors have been underscored as `_read` and `_modify`.
I'll prepare an evolution proposal for this feature which should allow
us to remove the underscores or, y'know, rename them to `purple` and
`lettuce`.
`_read` accessors do not make any effort yet to avoid copying the
value being yielded. I'll work on it in follow-up patches.
Opaque accesses to properties and subscripts defined with `_modify`
accessors will use an inefficient `materializeForSet` pattern that
materializes the value to a temporary instead of accessing it in-place.
That will be fixed by migrating to `modify` over `materializeForSet`,
which is next up after the `read` optimizations.
SIL ownership verification doesn't pass yet for the test cases here
because of a general fault in SILGen where borrows can outlive their
borrowed value due to being cleaned up on the general cleanup stack
when the borrowed value is cleaned up on the formal-access stack.
Michael, Andy, and I discussed various ways to fix this, but it seems
clear to me that it's not in any way specific to coroutine accesses.
rdar://35399664
I messed up the condition here---'targetEnvironment(device)' is
ignored and should be written '!targetEnvironment(simulator)'---but
even besides that I didn't actually have the right conditions for
which iOS devices this passes on and which it doesn't. Rather than
trying to perfectly match this with an XFAIL, just skip the parts of
the test that depend on having a new enough 2018 OS build if we, well,
don't have it.
rdar://problem/42398849
Follow-up to 3ed3774e07. On Apple OSs that don't have the new
Objective-C runtime function 'objc_setHook_getImageName', override the
system definition of 'class_getImageName' by literally rewriting
symbol tables at run time.
Yes, you read that correctly.
The low-level part of this patch was written by Greg Parker, then
simplified and tweaked by me to fit the Swift coding style. Don't try
this at home; it comes with all sorts of caveats and won't actually
work on this year's iOS. (Fortunately we don't need it there, because
that will have the new ObjC entry point.)
The rest of the patch is pretty straightforward: the replacement
implementation calls the code that supports Swift objects (the same
code we use on newer OSs), which then chains back to the original
system implementation of class_getImageName. May we never have to
touch this again.
rdar://problem/41535552
Narrow the list of address producers that verification can "peek through"
to avoid trigger an assertion in the previous commit with
-enable-verify-exclusivity.
My previous attempt to strengthen this verification ignored the fact that a
projection or addressor that returns UnsafePointer can have nested access after
being passed as an @inout argument. After inlining, this caused the verifier to
assert.
Instead, handle access to an UnsafePointer as a valid but unidentified access. I
was trying to avoid this because an UnsafePointer could refer to a global or
class property, which we can never consider "Unidentified". However, this is
reasonably safe because it can only result from a _nested_ access. If a global
or class property is being addressed, it must already have its own dynamic
access within the addressor, and that access will be exposed to the
optimizer. If a non-public KeyPath is being addressed, a keypath instruction
must already exist somewhere in the module which is exposed to the optimizer.
Fixes <rdar://problem/41660554> Swift CI (macOS release master, OSS): SIL verification failed: Unknown formal access pattern: storage.
This not only restores the correct behavior for classes with generic
ancestry, but also handles actual generic classes as well. (This is
the function that backs Foundation's Bundle.init(for: AnyClass)
initializer.)
https://bugs.swift.org/browse/SR-1917
rdar://problem/33450609&40367300
Because people put all sorts of nonsense into @objc enums (most
reasonably, "private cases", which represent valid values that are not
API), the Swift-synthesized implementation of 'hash(into:)' needs to
not expect a switch statement to be exhaustive. And since
Swift-defined @objc enums are supposed to behave enough like C-defined
enums, they should at least handle simple method calls with an invalid
raw value, which means that 'rawValue' likewise should not use a
switch.
This patch provides alternate implementations that look like this:
extension ImportedEnum {
public var rawValue: Int {
return unsafeBitCast(self, to: Int.self)
}
public func hash(into hasher: inout Hasher) {
hasher.combine(self.rawValue)
}
}
rdar://problem/41913284
Dynamic subclasses aren't /really/ valid Swift type metadata, but
they can still be used as values of type AnyClass. Make sure we
don't assert when that happens.
No intended functionality change.
These are all tests that would otherwise fail if the expression type
checker support for Swift 3 is removed.
I've moved some of the code from deleted Migrator tests into new
Constraints tests that verify that we do not support the constructs.
The major important thing here is that by using copy_unowned_value we can
guarantee that the non-ownership SIL ARC optimizer will treat the release
associated with the strong_retain_unowned as on a distinc rc-identity from its
argument. As an example of this problem consider the following SILGen like
output:
----
%1 = copy_value %0 : $Builtin.NativeObject
%2 = ref_to_unowned %1
%3 = copy_unowned_value %2
destroy_value %1
...
destroy_value %3
----
In this case, we are converting a strong reference to an unowned value and then
lifetime extending the value past the original value. After eliminating
ownership this lowers to:
----
strong_retain %0 : $Builtin.NativeObject
%1 = ref_to_unowned %0
strong_retain_unowned %1
strong_release %0
...
strong_release %0
----
From an RC identity perspective, we have now blurred the lines in between %3 and
%1 in the previous example. This can then result in the following miscompile:
----
%1 = ref_to_unowned %0
strong_retain_unowned %1
...
strong_release %0
----
In this case, it is possible that we created a lifetime gap that will then cause
strong_retain_unowned to assert. By not lowering copy_unowned_value throughout
the SIL pipeline, we instead get this after lowering:
----
strong_retain %0 : $Builtin.NativeObject
%1 = ref_to_unowned %0
%2 = copy_unowned_value %1
strong_release %0
...
strong_release %2
----
And we do not miscompile since we preserved the high level rc identity
pairing.
There shouldn't be any performance impact since we do not really optimize
strong_retain_unowned at the SIL level. I went through all of the places that
strong_retain_unowned was referenced and added appropriate handling for
copy_unowned_value.
rdar://41328987
**NOTE** I am going to remove strong_retain_unowned in a forthcoming commit. I
just want something more minimal for cherry-picking purposes.
When we have a private resilient enum that is resilient because one of
its payloads is resilient but we have disabled resilience in the
context of lowering the enum as a class member (sigh), we must consider
it's payload's layout enum in the minimal domain (ignore the private
visibility) because we don't truly know the layout.
rdar://41308521
Validation of the input side of FunctionTypeRepr was previously being done in Sema because of expression folding. If we instead push the invariant that the input TypeRepr should always be a TupleTypeRepr into the AST a number of nice cleanups fall out:
- The SIL Parser no longer accepts Swift 2-style type declarations
- Parse is more cleanly able to reject invalid FunctionTypeReprs
- Clients of the AST can be assured the input type is always a TupleType so we can flush Swift 2 hacks
It is safe to omit the retain/release dance in the reabstraction thunk
because we know we have an aditional reference outstanding for the
is_escaping verification.
The problem with throwing an objc exception inside a noescape closure is
that we verify the reference count of the closure sentinel. The
reabstraction thunk would increase the reference count call the
implementation function that then throws skipping the decrement.
rdar://40857699
swift_unknownRetain does not work on indirect enum heap buffers.
The existing logic would say: oh one case is
ReferenceCounting::BridgeObject (which on its own probably would not
work with swift_unknownRetain), oh and another second case is NativeObject (for
the indirect buffer), let's use unknowRetain.
Pair the logic down to only use a single style of reference count
operation if all cases are of the same ReferenceCounting type.
rdar://40525268
