> On Feb 23, 2017, at 3:55 PM, Matthew Johnson <[email protected]> wrote: > >> >> On Feb 23, 2017, at 4:40 AM, Anton Mironov <[email protected] >> <mailto:[email protected]>> wrote: >> >> >>> On Feb 23, 2017, at 4:28 AM, Matthew Johnson <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>>> >>>> On Feb 22, 2017, at 7:13 PM, Anton Mironov <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>>> >>>>> On Feb 23, 2017, at 02:19, Matthew Johnson <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>> >>>>>> >>>>>> On Feb 22, 2017, at 6:06 PM, Anton Mironov <[email protected] >>>>>> <mailto:[email protected]>> wrote: >>>>>> >>>>>> >>>>>>> On Feb 23, 2017, at 01:18, Matthew Johnson <[email protected] >>>>>>> <mailto:[email protected]>> wrote: >>>>>>> >>>>>>>> >>>>>>>> On Feb 22, 2017, at 5:06 PM, Anton Mironov <[email protected] >>>>>>>> <mailto:[email protected]>> wrote: >>>>>>>> >>>>>>>> -1 >>>>>>>> I support improvements in this area but I do not think that adding >>>>>>>> guarded closures will fix the case. >>>>>>>> It raises multiple concerns: >>>>>>>> - prepending ? to the closure declaration is as forgettable as `[weak >>>>>>>> self]` >>>>>>> >>>>>>> No, this is why I included the `@guarded` parameter annotation. This >>>>>>> allows an API to require its callers to use a guarded closure. Strong >>>>>>> references would have to be explicit in the capture list. >>>>>>> >>>>>>>> - reactive programming often assumes chaining of operations. How >>>>>>>> guarded closures affect next operations in the chain? >>>>>>> >>>>>>> Can you provide a concrete example of real world code you wrote >>>>>>> manually? I will convert it to use guarded closures to show how it is >>>>>>> affected. >>>>>> >>>>>> You can use the second piece of code I’ve provided before. >>>>> >>>>> How do `map` and `onUpdate` store the reference to the context? Is it >>>>> weak? If so, what happens when the context is released? After you >>>>> answer that I will be able to show how it would look under this proposal. >>>> >>>> Yes, they store a weak reference to the context. >>>> >>>> This code implies using primitive that has multiple update events (button >>>> actions in this case) followed by a single completion event (can only be >>>> an error of context deallocation in this case). I call it `Channel` >>>> (because name `Stream` is already taken by `Foundation.Stream` aka >>>> `NSStream`). >>>> >>>> A release of context will lead to a release of closure and all operations >>>> it depends on. >>>> >>>> `Channel.map`’s context does not exist anymore so closure will be released >>>> and `Channel` returned from `map` will be immediately completed with >>>> failure. >>>> This will also lead to a release of `Channel` returned by >>>> `Channel.debounce()`. `Channel.debounce()` will lead to release of >>>> `Channel` returned by `actions(forEvents: [.touchUpInside])`. >>>> >>>> `Channel.onUpdate`’s context does not exist anymore so closure will be >>>> released. >>>> This will also lead to a release of `Channel` returned by >>>> `Channel.distinct()`. >>>> >>>> It might look complex but it is based on a simple idea: release all >>>> retained resources if your context is gone. >>> >>> I’m very familiar with patterns like this but didn’t want to make any >>> assumptions about how your code works. How does this code currently detect >>> that the context has been released? Does it wait until an event is pushed >>> through the channel and see that the context is now nil? >>> >> >> No, it does not wait. It releases all retained resources (closure and >> channels it depends on) when context drains it’s `disposableBag`. > > Can you explain further? You gave it a context of `self`. Is the context > required to be a generic argument that conforms to a protocol which exposes a > `disposableBag` member? What does it mean to drain the `disposableBag`? > What causes it to get drained? How does the library detect that the context > has drained it’s `disposableBag`? I need to understand all of the details to > show how your use case can be modeled with guarded closures.
All contexts must conform to `ExecutionContext ` protocol. Being `ExecutionContext` means that instances of class can notify about their `deinit`. `disposableBag` (`ReleasePool` in my library) is one of a mechanisms of implementing this behavior. So `ReleasePool` will be automatically drained on `deinit`. Unlike in `RxSwift.DisposableBag`, `AsyncNinja.ReleasePool` is 99.99% of the time a constant being declared once and never used again in user code. You can see a full declaration here <https://github.com/AsyncNinja/AsyncNinja/blob/master/Sources/ExecutionContext.swift>. > >> >>> If this system was using guarded closures the `map` and `onUpdate` methods >>> would specify their function argument `@guarded` which would automatically >>> make all captures guarded. You are not limited to a single context object, >>> but if that is the need of your code you of course can capture a single >>> object. >>> >> I did not think of multiple contexts before. Maybe I should. I thought that >> two (or more) context you want to interact with are either: >> - have a simple relation (child-parent or a regular ownership) so there is >> no need for the weak reference for the second context >> - are operating on unrelated `DispatchQueue`s so mutation of an internal >> state of both contexts on the same queue may not exist > > Many times you won’t need more than one context, all you need is `self`. But > sometimes you write a closure that captures more than one object. Both of > these objects are part of the closure context and with a guarded closure they > are all guarded by default. It’s always *possible* to write code explicitly > in the style of a single context object like you are, but it’s often far more > convenient to just let a closure capture everything into an implicit single > context (the closure’s context that the compiler creates when capture > happens). With guarded closures we have the ability to do this and be > ensured that we don’t extend the lifetime of anything without explicitly > stating a strong capture in the capture list. > >> >>> The calling code would look like this: >>> >>> self.button.actions(forEvents: [.touchUpInside]) >>> .debounce(interval: 3.0) >>> .map ?{ >>> return self.searchField.text >>> } >>> .distinct() >>> .onUpdate ?{ (searchQuery) in >>> self.performSearch(query: searchQuery) >>> } >>> >>> In the implementation of the library where you used to store a weak >>> reference to the context and a strong reference to the closure you would >>> just store a strong reference to the guarded closure. The guarded closure >>> itself manages the weak / strong dance. Where you used to check the weak >>> reference to the context for nil to see if it is alive or not you would >>> check the `isAlive` property on the closure reference to determine if the >>> closure is still alive or not. When it is no longer alive you tear down >>> exactly the same as you do today when you detect that the context reference >>> is nil. >> >> That is a point. Next check `isAlive` will be performed right before the >> next call of the closure. This call could be performed in a minute or in an >> hour or even never performed. Guarded closure will capture variables until >> then. > > Can you explain how your mechanism for detecting earlier release works? Weak > references get nil`d lazily upon access so you must have some other kind of > plumbing that detects this. > > If we can identify a way to support the immediate release use case with > guarded closures I would be very happy. After you explain how your system > handles this I will try to think of a way to support it well. Unfortunately, I did not found a way without conformance to `ExecutionContext`. > > Even if we can’t, I think guarded closures would still benefit your use case. > The don’t prevent you from building an API that accepts a separate lifetime > object and releasing your reference to the closure immediately when that > lifetime object drains its `disposeBag`. What they would give you in that > scenario is a guarantee that the closures passed to `map` and `onUpdate` > don’t capture anything strongly without an explicit capture list. You can > get this guarantee while still using a manual context that controls the > lifetime of the closure. > > This design would look something like this: > > self.button.actions(forEvents: [.touchUpInside]) > .debounce(interval: 3.0) > .map(lifetime: self) ?{ > return self.searchField.text > } > .distinct() > .onUpdate(lifetime: self) ?{ > self.performSearch(query: $0) > } > > You pass an independent lifetime object that controls the duration of the > library’s reference to the closure. The difference here is that you don’t > pass the lifetime object to the closure as a context. Instead, you require a > guarded closure to be used to capture whatever context is necessary. The > lifetime and context are completely independent. Neither extends the > lifetime of any objects, with the exception of any strong captures in the > capture list of the guarded closure. > > Separating lifetime from context gives you the ability to have finer grained > control over lifetime (they are often the same, but there are probably good > use cases where you would like them to be different). Using guarded closures > allows you to reference names from the surrounding scope without have to > re-declare them in the closures’s signature. > > This approach does add an additional weak reference. If the advantages above > aren’t worth the extra weak reference you could simply stick with the design > you already have, but adopt `@guarded` to caution users against strong > captures. > > It’s also worth considering the use case of passing instance methods to `map` > and `onUpdate`. In your current design it might looks something like this > (assuming you have overloads that handle unbound instance methods): > > self.button.actions(forEvents: [.touchUpInside]) > .debounce(interval: 3.0) > .map(context: self, MyType.instanceMethod) > .distinct() > .onUpdate(context: self, MyType.otherInstanceMethod) > > With this proposal, if you switch to the idea of a lifetime instead of a > context you could write the above as: > > self.button.actions(forEvents: [.touchUpInside]) > .debounce(interval: 3.0) > .map(lifetime: self, ?instanceMethod) > .distinct() > .onUpdate(lifetime: self, ?otherInstanceMethod) > > You no longer have to explicitly state your type name repeatedly. You just > pass a guarded bound instance method. Maybe I could benefit from that. But extending lifetime is not the only reason why `ExecutionContext` exists. The library is made to provide concurrency primitives. So binding closure to context means both extending lifetime and providing a way of executing closure: `DispatchQueue`, `NSOperationQueue`, `NSManagedObjectContext`, … I think that using popular reactive programming libraries has 3 weak points: `[weak self]`, `.disposed(by: disposeBag)`, `.observeOn(MainScheduler.instance)`. - Guarded self in closures will allow you to avoid explicit `[weak self]`. - Maybe you will be able to avoid `.disposed(by: disposeBag)` if you’ll figure out how to get noticed about `deinit`. You can avoid all of that with the library <https://github.com/AsyncNinja/AsyncNinja>. In my opinion, fixing one weak point is not worth language support. > > >> >>> >>>> >>>>>> >>>>>>> >>>>>>>> - the closure must exist until either the control deallocates (source >>>>>>>> of actions) or self deallocates (destination of actions). Guarded >>>>>>>> closure will not provide an expected behavior >>>>>>> >>>>>>> Yes they will. The guarded closure lives until the control releases >>>>>>> it. But it becomes a no-op if any of the references captured with a >>>>>>> guard are released before that happens. This is much like the behavior >>>>>>> of the target / action pattern but generalized to support closures. >>>>>> >>>>>> I doubt that turning closure into no-op is a simple thing to do. It will >>>>>> require having a registry of closures that depend on an instance. A >>>>>> runtime will have to go through the registry and turn closures into >>>>>> no-op. Or there is an another solution that I do not see. >>>>> >>>>> What I mean when I say no-op is that the code the user places in the >>>>> closure would be prefixed by a `guard` clause with an early return. The >>>>> no-op is when the guard clause is triggered, just as if you had written >>>>> it manually. >>>> >>>> I think that this is an important part. Using no-op (or avoiding execution >>>> of closure as I understand it) leads to another unwanted retain of >>>> variables captured by the closure. >>> >>> There is no additional retain of the variables over a solution that >>> captures the variables independently of the closure and passes them as >>> arguments if they’re alive when called (like the context in your example). >>> Where do you think you see an extra retain of the variables? >> >> I meant that keeping the closure alive when a context is dead is an extra >> retain of a captured variables. >> >> Look, I’m not saying that extra care about retain cycles is bad. I’m just >> saying that making an implicit weak reference does not solve the whole >> problem. I think that adding guarded closures is not a step towards a >> complete solution. >> >>> >>>> >>>>> >>>>> I imagine the compiler might also place an additional check inside the >>>>> `else` of the `guard` which would release any context it was still >>>>> hanging on to as it would know that the context was no longer needed. >>>>> Imagine this as if the compiler synthesized code had access to an >>>>> optional strong reference to the context and set it to nil in the else >>>>> clause. It would also be possible for this closure to expose an >>>>> `isAlive: Bool` property that would check whether or not the context was >>>>> still around. This is a bit of hand waving - I’m sure the real >>>>> implementation would be more sophisticated. But I think that conveys the >>>>> basic idea. >>>>> >>>>> Here is some pseudocode: >>>>> >>>>> let foo: Foo = Foo() >>>>> let bar: Bar = Bar() >>>>> >>>>> // hand out references to the owner that controls the lifetime of foo and >>>>> bar >>>>> >>>>> ?{ >>>>> // compiler synthesized code >>>>> // this is a property on the closure object itself, not visible >>>>> within the scope of the user code in the closure >>>>> // it is used by libraries to detect when they can safely >>>>> discard their reference to the closure because it has become a no-op >>>>> // context is a super secret compiler reference to the context >>>>> of the closure >>>>> var isActive: Bool { return context != nil } >>>>> >>>>> // compiler synthesized code that prefixes the user code >>>>> guard let foo = foo, let bar = bar else { >>>>> context = nil >>>>> return >>>>> } >>>>> // end compiler synthesized code >>>>> >>>>> // begin user code >>>>> // do something with foo and bar >>>>> } >>>>> >>>>>> >>>>>>> >>>>>>>> - managing lifecycle of nested guarded closures could be complex to >>>>>>>> understand and implement into the language >>>>>>> >>>>>>> I’m glad you brought this up. I’ll give it some thought. If there >>>>>>> does turn out to be complexity involved I wouldn’t have a problem >>>>>>> prohibiting that. >>>>>>> >>>>>>>> - why would you consider using @escaping instead of @guarded? >>>>>>> >>>>>>> Because sometimes the right default for a function taking an escaping >>>>>>> closure is a strong reference. I wouldn't want `DispatchQueue.async` >>>>>>> to take a guarded closure. That API doesn’t contain any semantic >>>>>>> content around *why* you dispatched async. It’s not a callback, but >>>>>>> instead a way of moving work around. >>>>>>> >>>>>>>> >>>>>>>> I personally prefer doing something like this: >>>>>>>> >>>>>>>> ```swift >>>>>>>> self.button.onAction(forEvents: [.touchUpInside], context: self) { >>>>>>>> (self, sender, event) in >>>>>>>> self.performSearch(query: self.searchField.text) >>>>>>>> } >>>>>>>> ``` >>>>>>>> >>>>>>>> or >>>>>>>> >>>>>>>> ```swift >>>>>>>> self.button.actions(forEvents: [.touchUpInside]) >>>>>>>> .debounce(interval: 3.0) >>>>>>>> .map(context: self) { (self, _) in >>>>>>>> return self.searchField.text >>>>>>>> } >>>>>>>> .distinct() >>>>>>>> .onUpdate(context: self) { (self, searchQuery) in >>>>>>>> self.performSearch(query: searchQuery) >>>>>>>> } >>>>>>>> ``` >>>>>>>> >>>>>>>> This code neither requires an addition of language features nor >>>>>>>> contains retain cycles. All closures will be released as soon as >>>>>>>> source or destination deallocates. >>>>>>> >>>>>>> This isn’t too bad but it does require manually threading the context. >>>>>>> This is more work for both the library and the client than necessary. >>>>>>> It also does not help users avoid an accidental strong reference in the >>>>>>> closure. It nudges them not to by offering to thread the context but >>>>>>> it doesn’t do anything to prevent it. You can still create a strong >>>>>>> reference (event to self) without specifying it in the capture list. >>>>>> >>>>>> You are correct. This will code will not help to avoid accidental strong >>>>>> reference. But it gives an opportunity to do things without explicit >>>>>> weak references just as guarded closures do. It also adds an ability to >>>>>> avoid an execution of pure (or just not bound to context) operations you >>>>>> depends on. Deallocation of context will lead to cancellation of full >>>>>> chain of operations and unsubscription from button event. >>>>>> >>>>>>> I think there is a place for a language solution here. >>>>>> >>>>>> The only language solution I expect is a static analyzer warning about >>>>>> retain cycle (like in ObjC). >>>>>> >>>>>> I’m starting to think that my solution is similar to yours. I’ve done >>>>>> these things with a library rather than with language support. I will >>>>>> definitely take advantage of guarded self in closures as soon as the >>>>>> proposal will be accepted. But I would prefer and suggest using my >>>>>> solution for now. >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> On Feb 22, 2017, at 22:57, Matthew Johnson via swift-evolution >>>>>>>>> <[email protected] <mailto:[email protected]>> wrote: >>>>>>>>> >>>>>>>>> Hi David, >>>>>>>>> >>>>>>>>> I just shared a draft proposal to introduce guarded closures last >>>>>>>>> week: >>>>>>>>> https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170213/032478.html >>>>>>>>> >>>>>>>>> <https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170213/032478.html>. >>>>>>>>> I think you would find it very interesting. >>>>>>>>> >>>>>>>>> I considered including a new capture list specifier `guard` in this >>>>>>>>> proposal but decided against it. Guarded behavior requires prefixing >>>>>>>>> the contents of the closure with a guard clause that returns >>>>>>>>> immediately if the guard is tripped. This is a property of the >>>>>>>>> closure as a whole, not of an individual capture. For that reason, I >>>>>>>>> decided that allowing a `guard` specifier for an individual capture >>>>>>>>> would be inappropriate. >>>>>>>>> >>>>>>>>> Instead, a guarded closure has a guarded by default capture behavior >>>>>>>>> which can be overridden with `weak`, `unowned` or `strong` in the >>>>>>>>> capture list. The thread on this proposal was relatively brief. I >>>>>>>>> plan to open a PR soon after making a few minor modifications. >>>>>>>>> >>>>>>>>> Matthew >>>>>>>>> >>>>>>>>>> On Feb 22, 2017, at 2:48 PM, David Hedbor via swift-evolution >>>>>>>>>> <[email protected] <mailto:[email protected]>> wrote: >>>>>>>>>> >>>>>>>>>> Hello, >>>>>>>>>> >>>>>>>>>> (apologies if this got sent twice - gmail and Apple mail seems to >>>>>>>>>> confused as to what account the first mail was sent from) >>>>>>>>>> >>>>>>>>>> I’m new to this mailing list, but have read some archived messages, >>>>>>>>>> and felt that this would be a reasonable subject to discuss. It’s >>>>>>>>>> somewhat related to the recent posts about @selfsafae/@guarded but >>>>>>>>>> distinctly different regardless. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Problem: >>>>>>>>>> >>>>>>>>>> It’s often desirable not to capture self in closures, but the syntax >>>>>>>>>> for doing so adds significant boilerplate code for [weak self] or us >>>>>>>>>> unsafe when used with [unowned self]. Typically you’d do something >>>>>>>>>> like this: >>>>>>>>>> >>>>>>>>>> { [weak self] in self?.execute() } >>>>>>>>>> >>>>>>>>>> This is simple enough but often doesn’t work: >>>>>>>>>> >>>>>>>>>> { [weak self] in self?.boolean = self?.calculateBoolean() ] >>>>>>>>>> >>>>>>>>>> This fails because boolean is not an optional. This in turn leads to >>>>>>>>>> code like this: >>>>>>>>>> >>>>>>>>>> { [weak self] in >>>>>>>>>> guard let strongSelf = self else { return } >>>>>>>>>> strongSelf.boolean = self.calculateBoolean() } >>>>>>>>>> >>>>>>>>>> And this is the boilerplate code. My suggestion is to add a syntax >>>>>>>>>> that works the same as the third syntax, yet doesn’t require the >>>>>>>>>> boilerplate code. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Solution: >>>>>>>>>> >>>>>>>>>> Instead of using unowned or weak, let’s use guard/guarded syntax: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> { [guard self] in >>>>>>>>>> self.isExecuted = self.onlyIfWeakSelfWasCaptured() >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> In essence, guarded self is equivalent to a weak self, that’s >>>>>>>>>> captured when the closure is executed. If it was already released at >>>>>>>>>> that point, the closure is simply not executed. It’s equivalent to: >>>>>>>>>> >>>>>>>>>> { [weak self] in >>>>>>>>>> guard let strongSelf = self else { return } >>>>>>>>>> strongSelf.isExecuted = strongSelf.onlyIfWeakSelfWasCaptured() >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> Except with a lot less boilerplate code, while not losing any >>>>>>>>>> clarify in what it does. >>>>>>>>>> >>>>>>>>>> Impact / compatibility: >>>>>>>>>> >>>>>>>>>> This is simply additive syntax, and wouldn’t affect any existing >>>>>>>>>> code. >>>>>>>>>> _______________________________________________ >>>>>>>>>> swift-evolution mailing list >>>>>>>>>> [email protected] <mailto:[email protected]> >>>>>>>>>> https://lists.swift.org/mailman/listinfo/swift-evolution >>>>>>>>>> <https://lists.swift.org/mailman/listinfo/swift-evolution> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> swift-evolution mailing list >>>>>>>>> [email protected] <mailto:[email protected]> >>>>>>>>> https://lists.swift.org/mailman/listinfo/swift-evolution >>>>>>>>> <https://lists.swift.org/mailman/listinfo/swift-evolution>
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
