@Joe_Groff @John_McCall @Andrew_Trick
Just wanted to get some thoughts/feedback if any one has any objections upon this. TLDR: I think we should at least codegen a begin_borrow [var_decl] for this case to provide a SILLocation/use for diagnostics to enable Sema/SIL diagnostics to have parity and enable SIL diagnostics around this form of use.
Today when we codegen the following:
class Klass {}
func test(_ x: Klass) {
_ = x
}
we do not get any output from SILGen:
// test(_:)
sil hidden [ossa] @$s5test24testyyAA5KlassCF : $@convention(thin) (@guaranteed Klass) -> () {
// %0 "x" // user: %1
bb0(%0 : @guaranteed $Klass):
debug_value %0 : $Klass, let, name "x", argno 1 // id: %1
%2 = tuple () // user: %3
return %2 : $() // id: %3
} // end sil function '$s5test24testyyAA5KlassCF'
This means that one cannot emit diagnostics since we do not even have any instructions with a SILLocation for _ = x.
In the past when only working with SIL diagnostics (e.x.: when I was bringing up the noncopyable type checkers), this was not an issue since we took the position that _ = x was just a way to provide a use to quiet an unused variable diagnostic from Sema but it did not signify codegen.
That is IMO defensible for noncopyable types that are purely a SIL based diagnostic... but it clashes and creates special rules when dealing with concurrency diagnostics/semantics that are partially Sema based and partially SIL based. Despite SIL not being able to emit diagnostics in such a case, using Sema information we have in the past and we do today. As an example:
func testNonSendableCaptures(ns: NotSendable, a: isolated MyActor) {
Task {
_ = a
_ = ns
}
Task { [a] in // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between 'a'-isolated code and concurrent execution of the closure}}
_ = a
_ = ns // expected-tns-note {{closure captures 'a'-isolated 'ns'}}
}
}
Aside: The above example was supported before RBI was implemented in Sema and in RBI we today use Sema information to emit this via a layering violation which I want to eliminate. I had to use that information due to the lack of codegen in this case.
in the above example by using Sema information, we can actually emit a diagnostic since ns is viewed as a true capture of the closure passed to the Task initializer. By moving away from using Sema information here and using a pure SIL soluation, we cannot handle this test case since there is literally nothing for SIL to process.
So we would have to introduce a special case that concurrency rules do not apply if one captures a value using _ = ns. This is an active contradiction from other concurrency behavior where (as an example), if we capture self using _ = self will result in a semantic change. As an example:
actor MyActor {
func captureSelf() {
// Disconnected, not isolated.
let _ = { }
// Isolated to self since we capture self... even though self is used as a
// '_ = self'.
let _ = { _ = self }
}
}
Notice how in the emitted SIL, despite the position that _ = self does not have real codegen effects, it does change codegen since our capture of self makes the second closure actor isolated to self:
// closure #1 in MyActor.captureSelf()
// Isolation: nonisolated
sil private [ossa] @$s4test7MyActorC11captureSelfyyFyycfU_ : $@convention(thin) () -> () {
bb0:
%0 = tuple () // user: %1 return %0 // id: %1
} // end sil function '$s4test7MyActorC11captureSelfyyFyycfU_'
// closure #2 in MyActor.captureSelf()
// Isolation: actor_instance. name: 'self'
sil private [ossa] @$s4test7MyActorC11captureSelfyyFyycfU0_ : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> ()
{
// %0 "self" // user: %1
bb0(%0 : @closureCapture @guaranteed $MyActor):
debug_value %0, let, name "self", argno 1 // id: %1
%2 = tuple () // user: %3
return %2 // id: %3
} // end sil function '$s4test7MyActorC11captureSelfyyFyycfU0_'
This to me is a clear contradiction of the stated position that _ = x does not have a true semantic effect on the program and can be ignored.
With that in mind, I propose that we change the codegen here to make it so that we actually do codegen a variable here. It would just involve emitting a begin_borrow [var_decl] in such a case. The begin_borrow [lexical] would provide an instruction to place the self location upon. It would also preserve the current behavior that with noncopyable types _ = value is not a consuming operation. This would ensure that SIL level optimizations have something to work with. As an example the above closure would look as follows:
// closure #2 in MyActor.captureSelf()
// Isolation: actor_instance. name: 'self'
sil private [ossa] @$s4test7MyActorC11captureSelfyyFyycfU0_ : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> ()
{
// %0 "self" // user: %1
bb0(%0 : @closureCapture @guaranteed $MyActor):
debug_value %0, let, name "self", argno 1 // id: %1
%1 = begin_borrow [var_decl] %0 : $MyActor
end_borrow %1 : $MyActor
%2 = tuple () // user: %3
return %2 // id: %3
} // end sil function '$s4test7MyActorC11captureSelfyyFyycfU0_'
NOTE: I am just using begin_borrow here b/c we already have a var_decl bit on it. One could imagine adding another instruction that just marked the value.