It has been expressed in various ways "does anybody actually use scoped
visibility in the wild" and "what real benefit does it provide to production
code".
The below file or some estranged stepchild of it appears at least 5
repositories (that I know of; this is practically a samizdat around here). The
scoped access modifier is a significant improvement to the safety of this code
and by extension the many projects that contain it.
Apologies if this listing is rather tiring. Many of the "solutions" proposed
by the anti-private camp sound great in a sentence but fall apart at scale.
But it is not obvious why this is so to people who do not use the keyword, so I
can understand why they keep suggesting poor solutions. Perhaps with a more
involved listing, it will become more obvious why many of these suggestions do
not work. The original is a lot longer; I did reduce it to only the parts that
seem relevant to this thread.
import Foundation
/**
This code demonstrates one of the usecases for 'private'. Adapted from real
production code, this file exports a threading primitive to the rest of the
program.
To state it briefly, private is necessary here because we need the following
visibility nesting to achieve our objective:
┌───────────────────────────────────────────┐
│PROGRAM/internal │
│ │
│ │
│ ┌───────────────────────────────────┐ │
│ │ ThreadsafeWrapperNotifyChanged │ │
│ │ │ │
│ │ ┌──────────────────────┐ │ │
│ │ │ ThreadsafeWrapper │ │ │
│ │ │ │ │ │
│ │ │ │ │ │
│ │ │ value │ │ │
│ │ │ │ │ │
│ │ │ │ │ │
│ │ └──────────────────────┘ │ │
│ │ │ │
│ └───────────────────────────────────┘ │
└───────────────────────────────────────────┘
In particular:
1. value a.k.a. "t" must be protected against all potential unsafe access.
This file is hundreds of lines, auditing the whole thing is very tedious.
2. ThreadsafeWrapper is an implementation detail of
ThreadsafeWrapperNotifyChanged which is not intended for use by other callers.
To avoid exposing the class to other callers, it must appear in this file.
3. This file cannot be made an entire module, because it's actually used as
part of several projects that are shipped as frameworks, and apparently some
developers get really annoyed when they have to drag too many frameworks into
their Xcode project.
4. The use of `private` here reduces the "maybe not threadsafe" part of the
code from 196 lines to 47 lines (a reduction of buggy code of 76%). In the
production file from which this example is derived, the reduction is from 423
lines to 33 lines, or 92%. A scoped access variable significantly improves the
threadsafety of this code.
*/
//Define an interface common to both major components
private protocol Threadsafe : class {
///the type of the value we are protecting
associatedtype T
///Access the underlying value.
///- parameter block: The block that will be passed the protected value.
The block acts as an exclusive lock; while you're in it, no other consumers
will be accessing the value.
///- complexity: Coalescing multiple operations into a single block
improves performance.
func accessT<K>(_ block: @escaping (T) throws -> K) throws -> K
///Mutate the underlying value.
///- parameter block: The block that will be passed the protected value.
The block acts as an exclusive lock; while you're in it, no other consumers
will be accessing the value.
///- complexity: Coalescing multiple operations into a single block
improves performance.
func mutateT<K>(_ block: @escaping (inout T) throws -> K) throws -> K
}
///Some convenience accessors for callers that do not need a block-based API to
get lock semantics / operation coalescing
extension Threadsafe {
var value : T {
get {
var t: T! = nil
try! accessT({ lt in
t = lt
})
return t
}
set {
try! mutateT({ (lt:inout T) -> () in
lt = newValue
})
}
}
}
///The core synchronization primitive. This is a private implementation detail
of ThreadsafeWrapperNotifyChanged.
//MARK: audit this area begin
private final class ThreadsafeWrapper<T> : Threadsafe {
/**The value we are protecting. This value needs to be protected against
unsafe access from
1. This type, if a scoped keyword is available (as shown)
2. The entire file, if a scoped keyword is removed.
Only access this value on the synchronizationQueue.
*/
private var t: T
///The queue that is used to synchronize the value, only access the value
from the queue.
///- note: fileprivate is used here to allow the wrapped object to access
the queue. Demonstrating that both are legitimately useful modifiers.
fileprivate let synchronizationQueue: DispatchQueue
internal init (t: T, queueDescription: String) {
self.synchronizationQueue = DispatchQueue(label: "foo")
self.t = t
}
//MARK: implement our Threadsafe protocol
func accessT<K>(_ block: @escaping (T) throws -> K) throws -> K {
var k : K!
var err: Error?
synchronizationQueue.sync() {[unowned self] () -> Void in
do { try k = block(self.t) }
catch { err = error }
}
if let err = err { throw err }
return k
}
func mutateT<K>(_ block: @escaping (inout T) throws -> K) throws -> K {
var k : K!
var err: Error?
synchronizationQueue.sync() {[unowned self] () -> Void in
do { k = try self.fastMutateT(block) }
catch { err = error }
}
if let err = err { throw err }
return k
}
///An alternate mutation function that can only be used when inside a block
already.
///- note: Calling this function from the wrong queue is NOT thread-unsafe,
it will merely crash the program. So exposing this API to the file may
introduce bugs, but none of them are a threadsafety concern.
func fastMutateT<K>(_ block: @escaping (inout T) throws -> K) throws -> K {
dispatchPrecondition(condition: .onQueue(synchronizationQueue))
return try block(&t)
}
}
//MARK: audit area end
/**Like ThreadsafeWrapper, but also allows us to find out when the wrapped
object changes.
For that reason, it has a little more overhead than ThreadsafeWrapper, and
requires the wrapped type to be equatable */
final class ThreadsafeWrapperNotifyChanged<T: Equatable> : Threadsafe {
///Hold the value and a list of semaphores
private let tsw: ThreadsafeWrapper<(T, [DispatchSemaphore])>
internal init (t: T, queueDescription: String) {
self.tsw = ThreadsafeWrapper(t: (t, []), queueDescription: "foo")
}
//MARK: implement our Threadsafe protocol
func mutateT<K>(_ block: @escaping (inout T) throws -> K) throws -> K {
var k : K!
try tsw.mutateT { v in
defer {
for sema in v.1 {
sema.signal()
}
}
try self.tsw.fastMutateT({ v in
try block(&v.0)
})
}
return k
}
func accessT<K>(_ block: @escaping (T) throws -> K) throws -> K {
return try tsw.accessT({ v -> K in
return try block(v.0)
})
}
/**Notify when the value passed in has changed or the timeout has expired.
By passing a particular value, we can avoid many race conditions.*/
func waitForChange(oldValue: T, timeOut: TimeInterval) throws {
var sema : DispatchSemaphore! = nil
if oldValue != tsw.value.0 { return } //fastpath
//slowpath
try accessT {[unowned self] (tee) -> () in
if oldValue != tee { return }
sema = DispatchSemaphore(value: 0)
try self.tsw.fastMutateT({ v in
v.1.append(sema)
})
}
if sema == nil { return }
//clean up semaphore again
defer {
try! tsw.mutateT { v in
v.1.removeItemMatchingReference(sema)
}
}
let time = DispatchTime.now() + timeOut
if sema.wait(timeout: time) == .timedOut { throw
Errors.DeadlineExceeded }
//now, did we change?
let changed = try accessT { (val) -> Bool in
return val != oldValue
}
if changed { return }
try waitForChange(oldValue: oldValue, timeOut: timeOut) //🐞
}
}
//MARK: utility
enum Errors: Error {
case DeadlineExceeded
}
extension RangeReplaceableCollection where Iterator.Element : AnyObject {
/// Remove first colleciton element that matches the given reference
mutating func removeItemMatchingReference(_ object : Iterator.Element) {
if let index = self.index(where: {$0 === object}) {
self.remove(at: index)
}
}
}
On March 20, 2017 at 6:54:55 PM, Douglas Gregor ([email protected]) wrote:
Hello Swift community,
The review of SE-0159 "Fix Private Access Levels" begins now and runs through
March 27, 2017. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposals/0159-fix-private-access-levels.md
Reviews are an important part of the Swift evolution process. All reviews
should be sent to the swift-evolution mailing list at
https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review
manager. When replying, please try to keep the proposal link at the top of the
message:
Proposal link:
https://github.com/apple/swift-evolution/blob/master/proposals/0159-fix-private-access-levels.md
Reply text
Other replies
What goes into a review?
The goal of the review process is to improve the proposal under review through
constructive criticism and, eventually, determine the direction of Swift. When
writing your review, here are some questions you might want to answer in your
review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do
you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an
in-depth study?
More information about the Swift evolution process is available at
https://github.com/apple/swift-evolution/blob/master/process.md
Thank you,
-Doug
Review Manager
_______________________________________________
swift-evolution-announce mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution-announce
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
