And here’s a test case that crashes at runtime in Swift 3.1 and asserts on an
asserts-on build that does not involve protocol compositions, only protocol
refinement and a class constraint:
protocol P {
func bar()
}
// Comment out the 'class ,' to make the problem go away
protocol Q : class, P {}
extension P {
mutating func foo() {
bar()
}
}
class C : Q {
let x: Int = 100
func bar() {
print(x) // crash here
}
}
func takesQ(q: Q) {
var qq = q
qq.foo()
}
takesQ(q: C())
> On Apr 27, 2017, at 12:35 AM, Slava Pestov via swift-dev
> <[email protected]> wrote:
>
> Hi all,
>
> I’ve spent most of the last two days debugging various issues with property
> and subscript accesses on class-constrained existentials and I’ve just now
> realized the root cause is a much more fundamental issue.
>
> Consider the following code:
>
> protocol P {
> var x: Int { get set }
>
> init()
> }
>
> func takesP(p: inout P & AnyObject) {
> p.x = 1
> }
>
> Note that ‘takesP’ has a class-constrained existential type as a parameter.
> You can replace AnyObject with any class protocol, it won’t change anything
> below.
>
> Also, the init() requirement in the protocol will come into play later — for
> now, just consider the ‘var x’, which has a mutating setter.
>
> The above code compiles fine. Let’s take a look at the SILGen though:
>
> %2 = load [copy] %0 : $*AnyObject & P
> %3 = open_existential_ref %2 : $AnyObject & P to
> $@opened("52860F0E-2B19-11E7-8595-34363BD08DA0") AnyObject & P
> ...
> %8 = alloc_stack $@opened("52860F0E-2B19-11E7-8595-34363BD08DA0") AnyObject
> & P
> store %3 to [init] %8 : $*@opened("52860F0E-2B19-11E7-8595-34363BD08DA0")
> AnyObject & P
> %10 = witness_method $@opened("52860F0E-2B19-11E7-8595-34363BD08DA0")
> AnyObject & P, #P.x!setter.1, %3 :
> $@opened("52860F0E-2B19-11E7-8595-34363BD08DA0") AnyObject & P :
> $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (Int, @inout τ_0_0) -> ()
> %11 = apply %10<@opened("52860F0E-2B19-11E7-8595-34363BD08DA0") AnyObject &
> P>(%7, %8) : $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (Int,
> @inout τ_0_0) -> ()
>
> So we load the ‘AnyObject & P’ existential value from our inout argument,
> then in order to call the protocol requirement for x’s setter, which takes
> the ‘self’ value as an indirect argument, we allocate a box on the stack,
> store the reference into the box, call the requirement with the address of
> the box as the value of ‘self’, and destroy the box. We never write the
> result back to the inout.
>
> So consider the following implementation of a concrete type conforming to P:
>
> extension P {
> var x: Int {
> get { return 0 }
> set {
> self = Self()
> }
> }
> }
>
> class C : P {
> required init() {}
> }
>
> Note that the mutating setter for ‘x’ assigns to ‘self’, which is totally
> fine — it’s defined in a protocol extension of a non-class protocol P.
>
> Let’s try to use our concrete class type with our function takesP():
>
> let c = C()
> var cc = c
>
> takesP(p: &cc)
>
> print(c === cc)
>
> When you compile and run this, it outputs ‘true’, which is not what you would
> expect from reading the code, since the setter re-assigns ‘self’.
>
> Ok, maybe we can just tell people that if your class conforms to a non-class
> protocol, then assignments to ‘self’ inside implementations of the protocol
> requirement are not allowed, and are silently dropped.
>
> Now look at how we compile this version of takesP:
>
> extension P {
> mutating func foo() {
> self = Self()
> }
> }
>
> func takesP(p: inout P & AnyObject) {
> p.foo()
> }
>
> Here instead of calling a mutating setter, we call a mutating method.
>
> If we look at the SILGen, there is no stack allocation anymore, instead we
> pass the class reference directly as an inout argument, which is wrong:
>
> %7 = open_existential_ref %6 : $AnyObject & P to
> $@opened("F78B53CA-2B18-11E7-A73C-34363BD08DA0") AnyObject & P
> %8 = witness_method $@opened("F78B53CA-2B18-11E7-A73C-34363BD08DA0")
> AnyObject & P, #P.foo!1, %7 :
> $@opened("F78B53CA-2B18-11E7-A73C-34363BD08DA0") AnyObject & P :
> $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (@inout τ_0_0) -> ()
> %9 = apply %8<@opened("F78B53CA-2B18-11E7-A73C-34363BD08DA0") AnyObject &
> P>(%7) : $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (@inout τ_0_0)
> -> ()
>
> In a no-asserts build, like Swift 3.1 GM, this produces a binary which
> crashes at runtime. In an asserts build, we hit an assertion in Sema:
>
> Assertion failed: (fromType->is<LValueType>() && "Can only convert lvalues to
> inout"), function coerceObjectArgumentToType, file
> /Users/slava/new/swift/lib/Sema/CSApply.cpp, line 6336.
>
> There seem to be two fundamental problems here:
>
> 1) Sema models the base expression of a mutating member as an RValue, because
> it checks if the base expression type has reference semantics, not the *self
> type of the member*.
>
> 2) SIL cannot express an inout access performed on the payload of class
> existential.
>
> #1 is easy enough to fix, and allows my code examples involving subclass
> existentials to type check without hitting the LValue assertion.
>
> #2 is more fundamental. At first I thought it would be sufficient to change
> SILGen to only use class existential representation for protocol compositions
> where *all* members are class-constrained, instead of protocol compositions
> where *any* member is class-constrained. However this is insufficient because
> a class-constrained protocol can refine a non-class constrained protocol.
>
> We could say that a class-constrained protocol only uses class existential
> representation if all protocols it inherits also use class existential
> representation. Basically, split up ProtocolDecl::requiresClass() into two
> predicates — the current requiresClass() that expresses a constraint in the
> formal type system, and a stricter requiresNonMutatingSelfAccess() or similar
> which expresses that requirements and extension methods cannot be ‘mutating’.
> The latter would only be true if all refined protocols are also class
> constrained, and would be the condition for using a class existential
> representation.
>
> Now the reason the first example generates valid (but surprising) SIL, while
> the second one crashes, is that we seem to have a hack somewhere to handle
> mutating property access on a non-mutating value by consing up a box, copying
> the value into the box, and tearing down the box. We could conceivably
> generalize this hack to work on mutating methods as well, but it would lead
> to the same surprising behavior where the assignment to ’self’ is ‘lost'.
>
> However, there would be no loss of efficiency in that case, as we would still
> use the class existential representation in all the cases where we can
> currently use it.
>
> Another potential fix is to simply disallow mutating methods from being
> called on class-constrained existentials altogether, but that is a source
> breaking change (even though the old behavior was extremely flaky).
>
> Any thoughts?
>
> Slava
> _______________________________________________
> swift-dev mailing list
> [email protected]
> https://lists.swift.org/mailman/listinfo/swift-dev
_______________________________________________
swift-dev mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-dev
