[Review] SE-0159: Fix Private Access Levels

[marco.masser]

I also add my voice on the -1 side and fully agree with Drew’s reasoning.
`private` and `fileprivate` have a very distinct use in the code I write and in my opinion both carry their weight.

I’m not opposed to renaming or changing access modifiers, but I don’t see the need for it now. I agree with what others wrote that this topic should be revisited in the context of submodules, whenever that will be. Taking away the functionality of the current `private` by a confusingly written proposal that does not address the reasons for its introduction (or support the claims in the proposal itself with facts, for that matter) seems like a bad idea.

Cheers,

Marco

···

On 2017-03-21, at 02:26, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

I disagree quite strongly with the proposal.

First, the document draws conclusions without apparent supporting evidence, e.g.

> Since the release of Swift 3, the access level change of SE–0025 was met with dissatisfaction by a substantial proportion of the general Swift community. Those changes can be viewed as actively harmful, the new requirement for syntax/API changes.
What is “dissatisfaction by a substantial proportion of the general Swift community”? How was this measured/determined?
What was done to control for the population happy with SE-0025 who would e.g. not be likely to take up pitchforks?
Who argues these changes are “actively harmful” and where were they during SE-0025?
> subtly encourages overuse of scoped access control and discourages the more reasonable default
Who claims that scoped access is “overused” and what is their argument for doing so?
Why is “fileprivate” the “more reasonable default”? In fact neither fileprivate *nor* private are default (reasonable or not!). Internal is the default. Nor does this proposal suggest we change that. So this seems a very strange statement.
> But is that distinction between private and fileprivate actively used by the larger community of Swift developers?
Yes. To cite some evidence, here are codebases I actively maintain:

> codebase | private # | fileprivate # | ratio |

>--------------------------------------------------------|-----------|---------------|-------|

> "M" (proprietary) | 486 | 249 | 2x |

> "N"(proprietary) | 179 | 59 | 3x |

> NaOH https://code.sealedabstract.com/drewcrawford/NaOH | 15 | 1 | 15x |

> atbuild GitHub - AnarchyTools/atbuild: The Anarchy Tools build tool | 54 | 5 | 11x |

So from my chair, not only is the distinction useful, but scoped access control (private) is overwhelmingly (2-15x) more useful than fileprivate.

> And if it were used pervasively, would it be worth the cognitive load and complexity of keeping two very similar access levels in the language? This proposal argues that answer to both questions is no

This proposal does not make any later argument about “cognitive load” or “complexity” I can identify. Did the proposal get truncated?

What is stated (without evidence) is that "it is extremely common to use several extensions within a file” and that use of “private” is annoying in that case. I now extend the above table

> codebase | private # | fileprivate # | ratio | # of extensions (>=3 extensions in file) |

>--------------------------------------------------------|-----------|---------------|-------|------------------------------------------|

> "M" (proprietary) | 486 | 249 | 2x | 48 |

> "N"(proprietary) | 179 | 59 | 3x | 84 |

> NaOH https://code.sealedabstract.com/drewcrawford/NaOH | 15 | 1 | 15x | 3 |

> atbuild GitHub - AnarchyTools/atbuild: The Anarchy Tools build tool | 54 | 5 | 11x | 6 |

in order to demonstrate in my corner of Swift this is not “extremely common”, and is actually less popular than language features the proposal alleges aren’t used.

My point here is that **different people in different corners of the community program Swift differently and use different styles**. I can definitely empathize with folks like the author who use extensions to group functions and are annoyed that their favorite visibility modifier grew four extra characters. Perhaps we can come up with a keyword that is more succint.

However, that is no reason to take away features from working codebases. A scoped access modifier is perhaps my favorite feature in Swift 3. Let’s not throw stuff away because it adds extra characters to one programming style.

Finally, SE-0025 establishes clear motivation for the scoped access modifier:

> Currently, the only reliable way to hide implementation details of a class is to put the code in a separate file and mark it as private. This is not ideal for the following reasons:

> It is not clear whether the implementation details are meant to be completely hidden or can be shared with some related code without the danger of misusing the APIs marked as private. If a file already has multiple classes, it is not clear if a particular API is meant to be hidden completely or can be shared with the other classes.

> It forces a one class per file structure, which is very limiting. Putting related APIs and/or related implementations in the same file helps ensure consistency and reduces the time to find a particular API or implementation. This does not mean that the classes in the same file need to share otherwise hidden APIs, but there is no way to express such sharability with the current access levels.

As far as I can see, the proposal does not actually address or acknowledge these problems at all, but cheerfully returns us to them. It would be a mistake to deprecate this feature without examining at all why we introduced it. And realistically we need new solutions to those problems before removing the existing one.

Drew

On March 20, 2017 at 6:54:55 PM, Douglas Gregor (dgregor@apple.com <mailto:dgregor@apple.com>) 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 swift-evolution-announce@swift.org <mailto:swift-evolution-announce@swift.org> https://lists.swift.org/mailman/listinfo/swift-evolution-announce_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

[Drew_Crawford1 (Drew Crawford)]

There are a couple of problems with the design which are causing Cognitive Dissonance:

1) Private & fileprivate require very different mental models to operate them
2) They are named very similarly to each other
3) There is a third concept of “private to the type” which requires a third mental model, but is spelled the same way in other languages

How would you respond to the allegation that supporting different "redundant" semantic models is actually a *design goal* of Swift? For example:
Value semantics vs reference semantics
Pass-by-owned vs pass-by-shared vs pass-by-reference parameters
dynamic dispatch vs static dispatch
associatedtype vs generics
"complete" types (e.g. Array<Int>) vs "incomplete" types (Sequence) and which one is permitted after a colon
strong vs weak vs unowned
I would argue that supporting whatever the programmer's chosen mental model is actually Swift's greatest strength. We could have a language with only reference types for example, it would be far, far simpler and easier to teach. I am glad that we don't have that language. I don't use all of these in every program, but I use all of them eventually, and each one is right for some situation.

Many of us don't know half these models half as as well as we'd like. But we also like less than half of them half as well as they deserve. Visibility is in line with the trend here.

Of the three, I am personally most supportive of file-based

My concern about dropping down to file-based is it doesn't allow any nesting, which isn't powerful enough for several examples posted in this discussion. Unless by "file-based" we mean "directory-based" which is another proposal that has seen strong opposition from the "it's too complicated" wing.

···

On March 24, 2017 at 5:00:33 AM, Jonathan Hull via swift-evolution (swift-evolution@swift.org) wrote:

Several people have asked: What is the harm it causes?

I would like to provide a (hopefully) clear answer to that.

There are a couple of problems with the design which are causing Cognitive Dissonance:

1) Private & fileprivate require very different mental models to operate them
2) They are named very similarly to each other
3) There is a third concept of “private to the type” which requires a third mental model, but is spelled the same way in other languages

Any one of these would cause confusion. All of them together mean that even those of us who understand how they work are bothered by them on a subconscious level (similar to how misaligned graphics will give everyone a subconscious feeling that a graphic design is “off” even if they can’t discern why consciously). Even many of those who are arguing to keep private have argued to rename things or to make it work in a more type-based way by extending visibility to extensions. I don’t think anyone is denying there is a problem here with the status quo.

There is a general rule in design that things that look similar (or in this case are named similarly) need to behave similarly to avoid causing confusion (and conversely things that behave differently *need* to look different).

**This is not something that will go away simply by learning the behavior. It will continue to be a rough spot in the language that causes discomfort until the underlying design issue is fixed.**

The ideal solution here would be to choose a single model and make everything coherent under that. We choose either file-based, scope-based, or type-based access control and unify under that.

For file-based, that means that our sub-modules, when we add them, would need to be file-based as well.

If we choose Scope-based, then it really cries out for some sort of parameterized private. That is you only have private and public, but for private you would be able to name a scope to widen the visibility to the named scope. Submodules would then need to be scope based, and you would probably use the parameterized private with the submodule name to share data within it.

Type-based is very common in other languages, but I fear it would not play nicely with the way Swift has grown to work. The pros are that you could access private variables from extensions, but getting types to work together with the ease they do now would require a lot of design work (and may not even be quite possible). You also have additional complexities around subtypes, etc...

We really need to choose a single direction and not mix and match.

Of the three, I am personally most supportive of file-based because it is the closest to where we are now, and it seems to simplify many of the concerns about “friend” types by having visibility be dependent on proximity… which seems very intuitive to me.

As I said before, my preference would be to tentatively accept the change, but delay implementation until we design submodules for Swift 5. That way the design can be more cohesive as we can tackle it all at once. If submodules won’t be in scope until after Swift 5, then we should implement the change now.

I hope that explanation was at least a bit helpful…

Thanks,
Jon

Technical note: For the curious, basically we have made a design which is "locally consistent, but globally inconsistent". That is why the effect doesn’t go away after learning the concepts involved. Basically we have made the programming equivalent of the following image:

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Jon_Hull]

My goal was to explain the cognitive psychology of why this is an issue so we can work together on solving the underlying causes (as opposed to making random changes until something feels right)

As I said above, the names are not the only issue causing the problem. It is how the concepts interact in your brain. They are locally consistent, but globally inconsistent. Basically, you look at one of them, and it makes sense, but when you try to consider both of them at once, it doesn’t quite fit. The fact that it is close makes it worse, because your brain wants to make it fit (just as it keeps trying and failing with the optical illusion I posted that has the same issue). The end result is that it causes cognitive dissonance, or a subconscious uneasy feeling caused by two incompatible mental models.

It isn’t just a naming or education issue. That is why, when we were bikeshedding names, we couldn’t find any set of names that felt right. Solving only issue 2 by renaming them to be different would still help a little, but it isn’t ideal because it only solves one of the underlying issues.

The reason picking one model and declaring it the winner is ideal, is that it gets rid of the actual cause of our problem.

Even with this proposal, we would still have some negative transfer from other languages (expecting ‘private’ to behave based on how it has been experienced in other languages). Luckily, negative transfer is something that can be solved with training/education… even if it means learning is more difficult. We could potentially get rid of all 3 issues by getting rid of the word ‘private’ entirely as well, and calling fileprivate ‘local’ or something like that. Local would have the connotation that things nearby could see it.

Thanks,
Jon

···

On Mar 24, 2017, at 6:07 AM, Goffredo Marocchi <panajev@gmail.com> wrote:
On Fri, Mar 24, 2017 at 9:59 AM, Jonathan Hull via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Several people have asked: What is the harm it causes?

I would like to provide a (hopefully) clear answer to that.

There are a couple of problems with the design which are causing Cognitive Dissonance:

1) Private & fileprivate require very different mental models to operate them
2) They are named very similarly to each other
3) There is a third concept of “private to the type” which requires a third mental model, but is spelled the same way in other languages

Any one of these would cause confusion. All of them together mean that even those of us who understand how they work are bothered by them on a subconscious level (similar to how misaligned graphics will give everyone a subconscious feeling that a graphic design is “off” even if they can’t discern why consciously). Even many of those who are arguing to keep private have argued to rename things or to make it work in a more type-based way by extending visibility to extensions. I don’t think anyone is denying there is a problem here with the status quo.

There is a general rule in design that things that look similar (or in this case are named similarly) need to behave similarly to avoid causing confusion (and conversely things that behave differently *need* to look different).

**This is not something that will go away simply by learning the behavior. It will continue to be a rough spot in the language that causes discomfort until the underlying design issue is fixed.**

The ideal solution here would be to choose a single model and make everything coherent under that. We choose either file-based, scope-based, or type-based access control and unify under that.

Why would this be the ideal solution? I tend to disagree and I do not think there is real agreement or even anything approaching consensus that it would be ideal... it feels quite restrictive.

[ark_dan]

• What is your evaluation of the proposal?
+1

• Is the problem being addressed significant enough to warrant a change to Swift?

Yes. For some strange reason, i can’t bring myself to write `fileprivate` instead of `private` by default - if i don’t specifically think of it, i end up with privates everywhere, and then have to go back and change to `fileprivate`. At times, it seems a bit annoying.

• Does this proposal fit well with the feel and direction of Swift?

Oh yes. fileprivate is too long a word for access level that i pretty much use as a default; and definitely not intuitive.

I write as much framework code, as application logic. Since Swift 3 i don’t remember many cases where i had to use `private`. I do, however, have many `private` members in my code - solely because `private` is way more intuitive to write.

~99% of privates in my code are there by mistake - but thats what i end up with. That means, there is no need for a mixed code like that 99% of the time:

    private(set) var iosAudio: CMSampleBuffer?
    private(set) var iosVideo: CMSampleBuffer?
    fileprivate(set) var mic: CMSampleBuffer?
    fileprivate(set) var serialNumber: String
    fileprivate(set) var name: String

It also means, the 1% where i want to use `private`, the readers/users of my code may think it's a mistake too.

My point of view, the number of times i mean "this variable or function can only be accessed from within tis file" dwarfs the number when i mean "it can only be accessed by subclasses". They don't even compare.

If enough people need access based on class hierarchy, we should pick a keyword specifically for that - like "classprivate" if you will. But private within the file simply makes more sense.

• If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to
those?
N/A

• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I have strong opinion based on how i use Swift, which i love deep from the heart.

Thanks David & Doug for giving hope this may actually be reverted.

[Rien]

> What is your evaluation of the proposal?

Very strong -1. Drew has said it all.

A few extra points:

* If you split a class into a bunch of extensions, chances are it's a large class, and fileprivate keyword provides crucial documentation on which methods and fields are accessed from outside the scope. Even if someone blindly uses private until compiler complains, then switches to fileprivate without thinking, there's still value when reading the code. (This has been mentioned as one of the points against scoped private.)

it should, but fileprivate only means that something can be done, not that something is done.
And unfortunately or not, people will use this keyword in the wrong way, forget to update access levels after changes etc.
So while this should be true in theory, in praxis it is almost never the case.

Rien.

···

On 26 Mar 2017, at 13:23, Andrey Tarantsov via swift-evolution <swift-evolution@swift.org> wrote:

* Encapsulation is in the spirit of Swift's safety goals, we want to encourage it.

* // MARK is perfectly fine for tightly-coupled class bodies, so if you find yourself using tons of fileprivate, consider switching to a single body with marks

* I really like the point that we should uphold the original decision unless we find new compelling arguments that were not considered initially.

> Is the problem being addressed significant enough to warrant a change to Swift?

No. I haven't personally seen any complains about this feature in my circles; everyone is happy about tighter access controls.

> Does this proposal fit well with the feel and direction of Swift?

It feels actively against the safety and readability goals of Swift.

> If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

This has been beaten to death in the original proposal.

> How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thorough reading of the first half of this discussion, and active participation in the original discussion (as well as writing and shipping lots of Swift code since Swift 0.x).

A.

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Tino]

I am now absolutely thrilled to create a filter to Mark As Read anything else arising from this thread. Good luck.

That might be a good idea — after more than 200 messages, and a quite circular discussion with an unhealthy amount of ignorance for the opposing side ;-).

To fight the latter, I just tried to take the position that "new private" is really important, and this imho leads to interesting consequences...
This access modifier really doesn't solve a problem, like "let" does (unless the problem you want to solve is having a language with private access).
Have a look at this:

public struct SeperateConcerns {
  private var foo: Int = 0
  public mutating func updateFoo(_ value: Int) {
    print("The only allowed way to change foo was invoked")
    foo = value
  }

  private var bar: Int = 0
  public mutating func updateBar(_ value: Int) {
    print("The only allowed way to change bar was invoked")
    bar = value
  }

  private var foobar: Int = 0
  public mutating func updateFoobar(_ value: Int) {
    print("The only allowed way to change foobar was invoked")
    foobar = value
  }
}

You can protect foo from being changed by code in other files, and from extensions in the same file — and if the latter is a concern, there should also be a way to limit access to foo to specific function in scope.
Afaik, somebody proposed "partial" type declarations, but without them, the meaning of private is rather arbitrary, and the feature is only useful for a tiny special case.
If we had partial types, the situation would be different, and if would be possible to declare extensions inside a partial declaration of another type, we could even remove fileprivate without an replacement (I guess I should write a separate mail for this thought…)

[xwu (Xiaodi Wu)]

I agree with critiques that the proposal text itself could use some
fleshing out. There are a number of statements made that could use more
context; while it is true that a lot of that context already exists in the
form of messages to this list, in an ideal world someone would have time to
collate into the proposal the most convincingly argued points on both sides
of the debate.

That said, the proposed solution itself is succinct, unambiguous, and
easily understood. It does not argue that the new access level in Swift 3
has no merit, but rather that it has defects sufficient to merit a reversal
to what we had before. The migration path is clear and unlikely to cause
major problems. Therefore, as I agree with the arguments, I support the
proposal.

As to Drew's excellent analysis--

The statements about defaults is not strange. The core team itself, in its
rationale for accepting SE-0025, said that the design "makes 'private' the
'safe default' for cases where you don't think about which one [i.e.
`fileprivate` vs. `private`] you want to use." The current proposal is
simply engaging with the language and thinking of the core team.

The data presented are very helpful. But, I interpret the data differently.
Based on the extended table, I see that `fileprivate` becomes increasingly
indispensable the more you code in a style that uses extensions. In
codebases that have a very small number of extensions, the ratio of
`fileprivate` to `private` is heavily skewed in favor of the latter.
However, in codebases that have dozens of extensions, the ratio of
`fileprivate` to `private` draws closer to 1:3 or 1:2. This goes to a very
important basis on which the original proposal (SE-0025) was decided: the
core team predicted that `fileprivate` would be rarely used in idiomatic
Swift with the introduction of a new `private`. As I understand it, the
idea was that most users would not have to contend with four access
modifiers (now five) to become proficient in Swift but rather only three.
`fileprivate`, therefore, could be named awkwardly based on the expectation
that it would be rarely seen.

However, real-world experience now demonstrates that `fileprivate` is *not*
rarely seen. In fact, as the data above bear out, it is very hard not to
use `fileprivate` when writing code in at least one very common Swift style
(i.e. using same-file extensions). The point is well taken--but in the end
it isn't germane to the argument--that *some* styles of Swift code do not
have to use `fileprivate`. The point is that the core team's acceptance of
the design was based on a prediction that did not turn out to be true:
namely, that idiomatic Swift would rarely need `fileprivate`. As a
consequence, more users now have to contend with more access modifiers than
anticipated.

I disagree that there needs to be much more said about "complexity"--it is
self-evident that four access modifiers are more complex than three. (And
yes, I agree with previous arguments on this list that only two access
modifiers are really necessary at all: internal and public.)

Charles Srstka's added comment, while intriguing, poses a problem in
argumentation. One of the points being made above about the major advantage
of new `private` over `fileprivate` is precisely that new `private` is
invisible to extensions. If one "solves" the problem of having to use
`fileprivate` by making `private` visible to extensions, it may well be the
case that `fileprivate` is no longer commonly necessary--but one has also
reverted one of the major arguments in favor of new `private` in the first
place. And if `fileprivate` becomes uncommon but still strictly necessary
(as it would be a broader level than any newly invented `private` that is
exposed to extensions, we would still have to keep both access modifiers
around since the former cannot be migrated to the latter), thus juggling
access modifiers without simplifying anything.

···

On Mon, Mar 20, 2017 at 11:31 PM, Rob Mayoff via swift-evolution < swift-evolution@swift.org> wrote:

I also disagree with the proposal, and Drew's explanation is spot on.

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Slava_Pestov]

-1 on this as well for similar reasons. Places where I use fileprivate (aside from what was automatically migrated by Xcode) can be counted on fingers of one or two hands.

I feel that this proposal is reverting something without offering an alternative solution.

In what situations do you use Swift 3 private where fileprivate is not an acceptable replacement?

Slava

···

On Mar 20, 2017, at 11:07 PM, Charlie Monroe via swift-evolution <swift-evolution@swift.org> wrote:

On Mar 21, 2017, at 3:33 AM, Greg Parker via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Mar 20, 2017, at 4:54 PM, Douglas Gregor <dgregor@apple.com <mailto:dgregor@apple.com>> 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

-1. I yield the remainder of my time to Drew Crawford who satisfactorily explained my concerns.

--
Greg Parker gparker@apple.com <mailto:gparker@apple.com> Runtime Wrangler

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Brent_Royal-Gordon]

I think that many advocates of scoped private misunderstand critics' concerns about extensions.

One of the nice things about file-based `private` is that it allows access from extensions not only on the same type, but also on *other* types. For example, suppose I have a Book type and I want to allow them to be added to bookshelves:

  class Bookshelf {
    // I'm using file-based private here
    private(set) var books: [Book]
    
    func addBook(_ book: Book) {
      book.bookshelf = self
      books.append(book)
    }
  }

I also want to have a removal method, but—much like `UIView` and `NSView`'s `removeFromSuperivew`—I want that method to be on `Book`, not `Bookshelf`. With file-based private, that's no problem—just drop the code in an extension in the same file:

  extension Book {
    func removeFromBookshelf() {
      guard let shelf = bookshelf else { return }
      shelf.remove(at: shelf.books.index(of: self)!)
      bookshelf = nil
    }
  }

When we talk about file-based private being "better for extensions", we're *not*—or at least not necessarily—talking about it being better for the pattern of splitting a type into several different extensions to group related code together. We're talking about the pattern of splitting a *concern* across several different *types* by extending all of them, taking advantage of Swift's flexible scoping to encapsulate details even when the type hierarchy doesn't match the encapsulation boundaries you want.

So allowing `private` members to be seen by extensions anywhere in a project doesn't address this issue; in fact, it actively and aggressively undermines it, making it impossible to encapsulate any implementation details at a scope smaller than a single type. For myself and others like me, it is the exact opposite of what we want.

···

On Mar 20, 2017, at 8:26 PM, Charles Srstka via swift-evolution <swift-evolution@swift.org> wrote:

the concern with extensions could be easily solved simply by giving extensions access to private members of the extended type as long as those extensions are in the same module/submodule (or file, if you prefer, but I don’t). This would probably eliminate almost all of the use cases for fileprivate, as well as freeing us from the sometimes clumsy situation where we’re rather artificially forced to use files as a scope, instead enabling us to organize code into files as is appropriate for the project.

--
Brent Royal-Gordon
Architechies

[Shawn_Erickson]

I am still on the -1 side of this. I leverage the swift 3 enhanced
encapsulation aspects of private fairly heavily and would rather see any
additional efforts take place on needs of exposing things for subclasses to
work with while hiding them from clients of those classes (better then
protected like thing), submodules and things related. Also I would like to
avoid forcing file organization on folks to control access to things if it
can be done sensibly in other ways.

I don't see strong enough supportive reasoning in this proposal to warrant
a change nor does aspect of the talk about it being harmful, etc. The
default is internal, if you have no care for access controls just omit it
and things will work great for new users, etc of swift. If you want things
to be restricted for better encapsulation (or limiting auto completion) use
private. If you find a need to relax thing some consider fileprivate.

-Shawn

···

On Mon, Mar 20, 2017 at 8:26 PM Charles Srstka via swift-evolution < swift-evolution@swift.org> wrote:

On Mar 20, 2017, at 9:33 PM, Greg Parker via swift-evolution < swift-evolution@swift.org> wrote:

On Mar 20, 2017, at 4:54 PM, Douglas Gregor <dgregor@apple.com> 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

-1. I yield the remainder of my time to Drew Crawford who satisfactorily
explained my concerns.

I’ll second the -1 and the reference to Drew’s post, with the added comment
that the concern with extensions could be easily solved simply by giving
extensions access to private members of the extended type as long as those
extensions are in the same module/submodule (or file, if you prefer, but I
don’t). This would probably eliminate almost all of the use cases for
fileprivate, as well as freeing us from the sometimes clumsy situation
where we’re rather artificially forced to use files as a scope, instead
enabling us to organize code into files as is appropriate for the project.

Charles

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Charlie_Monroe1 (Charlie Monroe)]

-1 on this as well for similar reasons. Places where I use fileprivate (aside from what was automatically migrated by Xcode) can be counted on fingers of one or two hands.

I feel that this proposal is reverting something without offering an alternative solution.

In what situations do you use Swift 3 private where fileprivate is not an acceptable replacement?

In my usage, I very rarely use fileprivate. To back this with numbers, a recent project I developed (i.e. without Xcode's automatic migration of private -> fileprivate; about 6KLOC) has 2 uses of "fileprivate" and 160 uses of "private".

This is definitely a personal opinion, but I tend to limit access to members as much as possible to discourage/prevent their usage out of the scope they are intended for and to better design API that should be used out of the scope (public/internal). I feel this is a good thing for newcomers to a codebase (and myself coming back to a codebase after a year or two) as Xcode's autocompletion won't offer members that are not supposed to be called out of the scope (scope-private).

As I've mentioned this during the discussion, I feel that current state of things encourages huge files (1000+ lines of code) as you can't define "protected" level access and migrate some of the code that implements e.g. accessibility to a different file without potentially exposing internal details to the rest of the module (if the extension in the other file needs access to private details, which is often the case).

I feel that if there should be a change to the access levels, it should address this as well. The current solution based on reverting the original proposal only unnecessarily exposes implementational details to the rest of the file - at least in my codebase.

···

On Mar 21, 2017, at 8:26 AM, Slava Pestov <spestov@apple.com> wrote:

On Mar 20, 2017, at 11:07 PM, Charlie Monroe via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Slava

On Mar 21, 2017, at 3:33 AM, Greg Parker via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Mar 20, 2017, at 4:54 PM, Douglas Gregor <dgregor@apple.com <mailto:dgregor@apple.com>> 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

-1. I yield the remainder of my time to Drew Crawford who satisfactorily explained my concerns.

--
Greg Parker gparker@apple.com <mailto:gparker@apple.com> Runtime Wrangler

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

[Drew_Crawford1 (Drew Crawford)]

If private is really more useful, should we remove fileprivate instead?

We don’t have to remove any access modifier. They are all useful.

Fact is, you can replace every occurrence of "private" with "fileprivate", and your source would compile as before, whereas fileprivate saves us from a "friend"-keyword.

This is certainly a source-breaking change. Consider:

struct Foo {

fileprivate func foo\(\)\->String \{return "foo" \}

private func foo\(\)\->Int \{return 2\}   

}

print("\(Foo().foo())")

Replacing “private” with “fileprivate” here will introduce a compile error.

···

On March 21, 2017 at 7:00:32 AM, Tino Heth (2th@gmx.de) wrote:

[Rien]

+1

Big Yes, collect the other stuff on access levels and modules too before implementing anything.

···

On 21 Mar 2017, at 10:15, Jonathan Hull via swift-evolution <swift-evolution@swift.org> wrote:

I wonder if there is a way to basically accept that this is what we want to do, but delay implementing it until we have other changes to make to the access system (e.g. submodules) at the same time?

If things like that are scoped for Swift 5, then I would say delay implementing this until then. If it will be longer than that, we may as well fix it now.

On Mar 21, 2017, at 2:07 AM, Brent Royal-Gordon via swift-evolution <swift-evolution@swift.org> wrote:

On Mar 20, 2017, at 4:54 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0159-fix-private-access-levels.md

  • What is your evaluation of the proposal?

I'm torn. During the SE-0025 review, I argued against scoped private. I still think it was a mistake to add it. But we did, it's out there, and I don't want to introduce churn unnecessarily.

Long ago, judges realized the problems caused by re-litigating old disputes and created a doctrine called "stare decisis": standing by things decided. That doesn't mean they won't correct obvious mistakes, but it does mean that they default to upholding the precedent they've already set. I think that would be a wise course here.

I personally would prefer to have Swift behave as SE-0159 proposes. But if the core team thinks this is going to come out 50/50—that is, this change will help about as many people as it hurts—I think they should reject this proposal and keep the status quo. I really don't want to write another review next year for SE-0289 "Reintroduce Scoped Private".

--
Brent Royal-Gordon
Architechies

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Drew_Crawford1 (Drew Crawford)]

I’m not arguing that it is less or more than a majority. I’m just saying that we’ve seen a lot of talk against the original change.

This proposal asks us to balance the convenience of one group (extension-writers) against the existence of another (scoped-access users). To do that, we need a clear idea of the composition of both groups.

“A lot of talk” is not the evidentiary standard to remove a feature. It was not good enough when we introduced the feature, that required argument and clear use-cases.

By default, I did not mean the syntactic default of the language but the access modifier users will use “by default” when trying to restrict visibility. In most languages, that keyword is “private” so its valid to say that newcomers to the language will “default” to using that one.

Apologies, but I do not understand the argument:

A user wants to restrict visibility (e.g. they are dissatisfied with “internal”)
The user *chooses* private because of familiarity from another language
The user is then surprised that their choice of private indeed restricted the visibility, thus achieving their goal?
What language does the user come from in which “private” is file-visible? It isn’t Java, C++, or PHP. C#’s “partial” is the closest I can think of, and it isn’t at all close.

A user who wants a middle-ground visibility would “default” to “protected”, “friend”, “partial”, or similar. After that does not compile, they will use google to find a middle-road visibility keyword, for which the only candidate is “fileprivate”. But they will not choose “private”, it’s just not a reasonable expectation of what the keyword means to a new Swift developer.

The popularity of private “as a default” is simply because many users prefer to hide their implementation details as a matter of routine code hygiene. Redefining private in order to thwart their code hygiene goal seems extreme.

I agree with several here (as I did in SE-0025) that our access modifiers are not well-named. However, that’s not the proposal in front of us.

My own statistics in my projects show the contrary. At best, this shows how divisive this feature is.

This *may* show that, if contrary statistics were presented, but that hasn’t occurred.

In old code, statistics could be biased by the migrator having replaced all previous instances of private by fileprivate.
If the migrator migrated code to private, and it *worked* (e.g. did not introduce visibility errors) this is not bias, this is a correct use of the feature.

I'm just arguing that the additional scope-based access modifier does not provide enough differentiation to be worth that complexity.

The only argument I have seen so far around “complexity” boils down to: “some people do not use it”. But some people *do* use it, and anyway if we are going to remove all the features “not enough people” use then we are in for a ride.

Swift 3 shipped, so what we are discussing now is yanking a keyword without replacement. There is code written that uses private to enforce its threading or security invariants. There is code written that uses private in order to shadow another declaration. There is code that will not compile after migration. We need more than a vague fear of complexity generally to throw a brick through all those windows. That brick will introduce quite a bit of complexity itself.

Concerning the one-class-per-file argument, I would suggest this counter-argument: when working in large projects, I believe it's a good thing if the language encourages (forces is too strong a word for my taste) a one class per file structure, it's good practice.

The form of the argument is invalid. Suppose I argued: "it’s a good thing for the language to encourage one definition per class (no extensions), it’s good practice. So we do not need fileprivate.” That would be very silly (although it has already been advanced as a straw-man position elsewhere in this thread). The argument that we do not need private because nobody should put multiple classes in a file is equally silly. There are reasons to do so, in fact one motivation was given in SE-0025:

Putting related APIs and/or related implementations in the same file helps ensure consistency and reduces the time to find a particular API or implementation.

These concerns are not resolved by arguments of the form “just don’t do that”.

I empathize with the Swift2 programmer who got through two releases without a scoped access modifier and is annoyed by change. However, removing the feature now is more change, not less, so it makes their problem worse, not better.

Perhaps it was a mistake, but I purposefully did not go into too much detail in the proposal because I think this debate is purely a question of philosophy on Swift and its language features. I did not want to add un-necessary bloat that would have added little rationalisation. Let me try to explain the holes in the proposal by answering your review:

I disagree quite strongly with the proposal.

First, the document draws conclusions without apparent supporting evidence, e.g.

Since the release of Swift 3, the access level change of SE–0025 was met with dissatisfaction by a substantial proportion of the general Swift community. Those changes can be viewed as actively harmful, the new requirement for syntax/API changes.

What is “dissatisfaction by a substantial proportion of the general Swift community”? How was this measured/determined?
It’s not feasible to measure precisely the feeling of a whole community. But we get a feeling for it by following the mailing-list, by talking to colleagues, by reading twitter, etc… And it think we all agree that the debate is highly divisive and that a “substantial proportion” of the community was dissatisfied: I’m not arguing that it is less or more than a majority. I’m just saying that we’ve seen a lot of talk against the original change.
What was done to control for the population happy with SE-0025 who would e.g. not be likely to take up pitchforks?
That’s why its important we have this debate now.
Who argues these changes are “actively harmful” and where were they during SE-0025?
The proposal makes the argument that the changes are actively harmful. It’s now up to debate. By the way, even if several people (including me) were already against this proposal during the review, I don’t see why anybody would not have the right to change his mind, especially after several months of production usage and argue differently now.

subtly encourages overuse of scoped access control and discourages the more reasonable default

Who claims that scoped access is “overused” and what is their argument for doing so?
Why is “fileprivate” the “more reasonable default”? In fact neither fileprivate *nor* private are default (reasonable or not!). Internal is the default. Nor does this proposal suggest we change that. So this seems a very strange statement.
By default, I did not mean the syntactic default of the language but the access modifier users will use “by default” when trying to restrict visibility. In most languages, that keyword is “private” so its valid to say that newcomers to the language will “default” to using that one. If the proposal is accepted, file-scoped private will regain that status.

But is that distinction between private and fileprivate actively used by the larger community of Swift developers?

Yes. To cite some evidence, here are codebases I actively maintain:

codebase | private # | fileprivate # | ratio |

--------------------------------------------------------|-----------|---------------|-------|

"M" (proprietary) | 486 | 249 | 2x |

"N"(proprietary) | 179 | 59 | 3x |

NaOH https://code.sealedabstract.com/drewcrawford/NaOH | 15 | 1 | 15x |

atbuild GitHub - AnarchyTools/atbuild: The Anarchy Tools build tool | 54 | 5 | 11x |

So from my chair, not only is the distinction useful, but scoped access control (private) is overwhelmingly (2-15x) more useful than fileprivate.

My own statistics in my projects show the contrary. At best, this shows how divisive this feature is. During the discussion of this proposal, it was argued that making decisions based upon project statistics would be dangerous:

In old code, statistics could be biased by the migrator having replaced all previous instances of private by fileprivate.
In new code, satistics could be biased by people using private because of it being the “soft-default”, regardless of proper semantics.

And if it were used pervasively, would it be worth the cognitive load and complexity of keeping two very similar access levels in the language? This proposal argues that answer to both questions is no

This proposal does not make any later argument about “cognitive load” or “complexity” I can identify. Did the proposal get truncated?

Sorry if I did not state it explicitly, but I see any feature/keyword added to the language as “additional complexity”. And that complexity is completely worth it when the feature adds significant expressivity. I'm just arguing that the additional scope-based access modifier does not provide enough differentiation to be worth that complexity.
What is stated (without evidence) is that "it is extremely common to use several extensions within a file” and that use of “private” is annoying in that case. I now extend the above table

codebase | private # | fileprivate # | ratio | # of extensions (>=3 extensions in file) |

--------------------------------------------------------|-----------|---------------|-------|------------------------------------------|

"M" (proprietary) | 486 | 249 | 2x | 48 |

"N"(proprietary) | 179 | 59 | 3x | 84 |

NaOH https://code.sealedabstract.com/drewcrawford/NaOH | 15 | 1 | 15x | 3 |

atbuild GitHub - AnarchyTools/atbuild: The Anarchy Tools build tool | 54 | 5 | 11x | 6 |

in order to demonstrate in my corner of Swift this is not “extremely common”, and is actually less popular than language features the proposal alleges aren’t used.

My point here is that **different people in different corners of the community program Swift differently and use different styles**. I can definitely empathize with folks like the author who use extensions to group functions and are annoyed that their favorite visibility modifier grew four extra characters. Perhaps we can come up with a keyword that is more succint.

I agree that different people in different corners use different styles. But you could use that argument to validate many features which would make a group of users happy; but all those feature together would just add bloat to the language. Swift has been known to be a very opinionated language, to keep the language simple yet expressive.
However, that is no reason to take away features from working codebases. A scoped access modifier is perhaps my favorite feature in Swift 3. Let’s not throw stuff away because it adds extra characters to one programming style.

Finally, SE-0025 establishes clear motivation for the scoped access modifier:

Currently, the only reliable way to hide implementation details of a class is to put the code in a separate file and mark it as private. This is not ideal for the following reasons:

It is not clear whether the implementation details are meant to be completely hidden or can be shared with some related code without the danger of misusing the APIs marked as private. If a file already has multiple classes, it is not clear if a particular API is meant to be hidden completely or can be shared with the other classes.

It forces a one class per file structure, which is very limiting. Putting related APIs and/or related implementations in the same file helps ensure consistency and reduces the time to find a particular API or implementation. This does not mean that the classes in the same file need to share otherwise hidden APIs, but there is no way to express such sharability with the current access levels.

As far as I can see, the proposal does not actually address or acknowledge these problems at all, but cheerfully returns us to them. It would be a mistake to deprecate this feature without examining at all why we introduced it. And realistically we need new solutions to those problems before removing the existing one.

Drew

···

On March 21, 2017 at 2:17:40 AM, David Hart (david@hartbit.com) wrote:
On 21 Mar 2017, at 02:26, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

On March 20, 2017 at 6:54:55 PM, Douglas Gregor (dgregor@apple.com) 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 swift-evolution-announce@swift.org https://lists.swift.org/mailman/listinfo/swift-evolution-announce

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Tino]

> Fact is, you can replace every occurrence of "private" with "fileprivate", and your source would compile as before, whereas fileprivate saves us from a "friend"-keyword.

This is certainly a source-breaking change. Consider:

struct Foo {

    fileprivate func foo()->String {return "foo" }

    private func foo()->Int {return 2}

}

print("\(Foo().foo())")

Replacing “private” with “fileprivate” here will introduce a compile error.

True indeed… but can we agree that this is just an hypothetic example, and no issue that is likely to happen in productive code?
Or is this actually taken from one of the projects you measured?

[Panajev (Goffredo Marocchi)]

One point which was raised by a couple of different people on this thread (Brent Royal-Gordon, Jonathan Hull), which gave me some hesitation in voting in favor of this proposal, is that it might make more sense to leave things alone for the time being, with the understanding that scoped access will be solved in some more general way via submodules in the future.

For what it's worth, I don't really agree with Jonathan Hull on this. If we're going to remove the band-aid, we might as well rip it off now; there's no reason to wait for people to write more code for us to break.

Fair point, but I honestly do not see how the stated very strong burden of proof to revert previous proposals has been met. The new proposal does seem to do anything really to address the needs of those that have a legitimate use for scoped access and the current state of things does not make life impossible for people who want to use file based visibility only or mostly. I really want to avoid having another proposal like the one that removed argument labels from blocks (burying them from the type param) that is incomplete and supposed to get addressed, but still has not been almost a year later.

This proposal as it is does not feel complete and I do not think this helps it satisfy the burden of proof needed for this release.

···

Sent from my iPhone

On 22 Mar 2017, at 09:43, Brent Royal-Gordon via swift-evolution <swift-evolution@swift.org> wrote:

On Mar 21, 2017, at 11:29 PM, Matt Whiteside via swift-evolution <swift-evolution@swift.org> wrote:

My point was simply that the burden of proof is now on those proposing to revert SE-0025, and the core team shouldn't accept this proposal unless they are satisfied that the burden has clearly been met.

--
Brent Royal-Gordon
Architechies

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Drew_Crawford1 (Drew Crawford)]

This fails to convince me.
I understand that, but not why.

I can't go into detail in public, but I can say that we did a postmortem on a large lost sale and the customer specifically cited the number of frameworks in our product as an integration barrier for them. Most iOS SDKs are distributed as a single framework and so with that backdrop the friction makes more sense.

As a result of that I have about 5 bugs open on how to reduce our framework footprint so our tools are easier for our users to integrate. There are a variety of solutions we use on that, what you see here is one of the saner ones, believe it or not.

Whether or not the technical requirement makes sense to you, the business case is very clear. So clear that if scoped were removed we would almost certainly keep the file and its potential threading bugs, over promoting a new framework. Sales >> code, unfortunately.

Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

That is a lovely tale but it is not the proposal in front of us. Perhaps when we get submodules that solve these visibility problems we can revisit.

···

On March 23, 2017 at 1:47:20 AM, David Hart (david@hartbit.com) wrote:

Hi Drew Crawford,

I understand that scoped private can be of value, and that code is a good example. But I never argued that private was useless (it would never have been proposed and accepted in the first case if it was). I just argue that the advantage is not worth its presence in the language. Here’s my take:

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.

This fails to convince me. If this piece of code is really used in several projects, I think it belongs in its own module and I would have implemented it by moving ThreadsafeWrapper to its own file, thus keeping the same thread-safety guarantees and hiding of your current example. The feels to me like this piece of code is using private only because it is shying away from using module-scope. Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

David.

On 23 Mar 2017, at 06:17, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

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 (dgregor@apple.com) 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:

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:

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

apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[Drew_Crawford1 (Drew Crawford)]

I should add for completeness, leaving aside the # of frameworks issue, there is another issue with promoting this to a module as those exist in Swift today: it would necessarily become a public API (by this I mean: API visible to our customers) and one of them would try to use it.

···

On March 23, 2017 at 2:10:48 AM, Drew Crawford (drew@sealedabstract.com) wrote:

This fails to convince me.
I understand that, but not why.

I can't go into detail in public, but I can say that we did a postmortem on a large lost sale and the customer specifically cited the number of frameworks in our product as an integration barrier for them. Most iOS SDKs are distributed as a single framework and so with that backdrop the friction makes more sense.

As a result of that I have about 5 bugs open on how to reduce our framework footprint so our tools are easier for our users to integrate. There are a variety of solutions we use on that, what you see here is one of the saner ones, believe it or not.

Whether or not the technical requirement makes sense to you, the business case is very clear. So clear that if scoped were removed we would almost certainly keep the file and its potential threading bugs, over promoting a new framework. Sales >> code, unfortunately.

Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

That is a lovely tale but it is not the proposal in front of us. Perhaps when we get submodules that solve these visibility problems we can revisit.

On March 23, 2017 at 1:47:20 AM, David Hart (david@hartbit.com) wrote:

Hi Drew Crawford,

I understand that scoped private can be of value, and that code is a good example. But I never argued that private was useless (it would never have been proposed and accepted in the first case if it was). I just argue that the advantage is not worth its presence in the language. Here’s my take:

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.

This fails to convince me. If this piece of code is really used in several projects, I think it belongs in its own module and I would have implemented it by moving ThreadsafeWrapper to its own file, thus keeping the same thread-safety guarantees and hiding of your current example. The feels to me like this piece of code is using private only because it is shying away from using module-scope. Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

David.

On 23 Mar 2017, at 06:17, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

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 (dgregor@apple.com) 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:

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:

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

apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[hartbit (David Hart)]

This fails to convince me.

I understand that, but not why.

I can't go into detail in public, but I can say that we did a postmortem on a large lost sale and the customer specifically cited the number of frameworks in our product as an integration barrier for them. Most iOS SDKs are distributed as a single framework and so with that backdrop the friction makes more sense.

Thats a different problem. We will get static linking at some point in the near future.

As a result of that I have about 5 bugs open on how to reduce our framework footprint so our tools are easier for our users to integrate. There are a variety of solutions we use on that, what you see here is one of the saner ones, believe it or not.

Whether or not the technical requirement makes sense to you, the business case is very clear. So clear that if scoped were removed we would almost certainly keep the file and its potential threading bugs, over promoting a new framework. Sales >> code, unfortunately.

The business case makes it clear you need static linking, not a scoped private to wiggle around your requirement to avoid modules.

Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

That is a lovely tale but it is not the proposal in front of us. Perhaps when we get submodules that solve these visibility problems we can revisit.

That's the problem. If we wait until we get submodules, we won't be able to revisit. This is probably our last chance to "remove" a feature. Submodules can always add features down the way.

···

On 23 Mar 2017, at 08:10, Drew Crawford <drew@sealedabstract.com> wrote:

On March 23, 2017 at 1:47:20 AM, David Hart (david@hartbit.com) wrote:

Hi Drew Crawford,

I understand that scoped private can be of value, and that code is a good example. But I never argued that private was useless (it would never have been proposed and accepted in the first case if it was). I just argue that the advantage is not worth its presence in the language. Here’s my take:

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.

This fails to convince me. If this piece of code is really used in several projects, I think it belongs in its own module and I would have implemented it by moving ThreadsafeWrapper to its own file, thus keeping the same thread-safety guarantees and hiding of your current example. The feels to me like this piece of code is using private only because it is shying away from using module-scope. Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

David.

On 23 Mar 2017, at 06:17, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

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 (dgregor@apple.com) 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
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[hartbit (David Hart)]

I should add for completeness, leaving aside the # of frameworks issue, there is another issue with promoting this to a module as those exist in Swift today: it would necessarily become a public API (by this I mean: API visible to our customers) and one of them would try to use it.

Submodules will most probably fix that. It's one of the prime reasons people want submodules.

···

On 23 Mar 2017, at 08:17, Drew Crawford <drew@sealedabstract.com> wrote:

On March 23, 2017 at 2:10:48 AM, Drew Crawford (drew@sealedabstract.com) wrote:

This fails to convince me.

I understand that, but not why.

I can't go into detail in public, but I can say that we did a postmortem on a large lost sale and the customer specifically cited the number of frameworks in our product as an integration barrier for them. Most iOS SDKs are distributed as a single framework and so with that backdrop the friction makes more sense.

As a result of that I have about 5 bugs open on how to reduce our framework footprint so our tools are easier for our users to integrate. There are a variety of solutions we use on that, what you see here is one of the saner ones, believe it or not.

Whether or not the technical requirement makes sense to you, the business case is very clear. So clear that if scoped were removed we would almost certainly keep the file and its potential threading bugs, over promoting a new framework. Sales >> code, unfortunately.

Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

That is a lovely tale but it is not the proposal in front of us. Perhaps when we get submodules that solve these visibility problems we can revisit.

On March 23, 2017 at 1:47:20 AM, David Hart (david@hartbit.com) wrote:

Hi Drew Crawford,

I understand that scoped private can be of value, and that code is a good example. But I never argued that private was useless (it would never have been proposed and accepted in the first case if it was). I just argue that the advantage is not worth its presence in the language. Here’s my take:

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.

This fails to convince me. If this piece of code is really used in several projects, I think it belongs in its own module and I would have implemented it by moving ThreadsafeWrapper to its own file, thus keeping the same thread-safety guarantees and hiding of your current example. The feels to me like this piece of code is using private only because it is shying away from using module-scope. Once we gain submodules in the future, it would be even easier to move both classes to their own sub-module.

David.

On 23 Mar 2017, at 06:17, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

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 (dgregor@apple.com) 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
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution