[swift-evolution] [Discussion] A Problem With SE-0025?

Robert Widmann devteam.codafi at gmail.com
Thu Jun 16 10:41:43 CDT 2016


The changes in that patch were the minimum required to get it to build.  There are definitely refinements to be had here, but I believe this gets us 90% of the way there (except corelibs-foundation which I will try to audit today after a rebase).  I was only able to leave explicitly marked private members alone, but SPM more often than not couples outer and inner types together by declaring the inner classes private which made them wholly inaccessible.  For enums there's nothing I could do because you can't re-apply access control to individual cases, those all had to be fileprivate.  typealiases and globals at the top level could generally stay private consistently.  

One of the reasons I brought this up in the first place was I noticed about 4 (not sure if some are the same) new crashes in and around SILGen because we were blowing away full-private types inside SwiftPM and causing problems down the rest of the optimization passes into codegen.

~Robert Widmann

2016/06/16 8:24、Matthew Johnson <matthew at anandabits.com> のメッセージ:

> 
>> On Jun 16, 2016, at 10:20 AM, Robert Widmann <devteam.codafi at gmail.com> wrote:
>> 
>> 
>> 
>> ~Robert Widmann
>> 
>> 2016/06/16 8:18、Matthew Johnson <matthew at anandabits.com> のメッセージ:
>> 
>>> 
>>>> On Jun 16, 2016, at 10:12 AM, Robert Widmann <devteam.codafi at gmail.com> wrote:
>>>> 
>>>> The Swift PM case is actually the one that causes me to sound the alarm bells ;) I migrated that one by hand as did @modocache for XCTest.
>>> 
>>> What I mean is that you just applied the semantic-preserving transformation of replacing `private` with `fileprivate` rather than reviewing whether the new `private` might be more appropriate given the intent of a specific piece of code.  That is reasonable for moving the implementation forward.
>> 
>> No, I actually sat for 4 hours and hand-migrated individual failing test cases across 3 repositories over the weekend.  This was no simple sed job!
> 
> I certainly appreciate the effort you applied here!  So you did leave some things as `private` when they were not required to be more visible then?  I apologize if I misunderstood.
> 
>> 
>>> 
>>> However, the discussion on the PR is because the Swift PM team wants to take advantage of the new semantics of `private` rather than just preserving the existing semantics which were looser than desired.  That is also reasonable.
>>> 
>>> I understand that this discussion is what prompted the realization that the proposal is not explicit enough and therefore requires clarification from the core team.  Hopefully Doug (review manager for the proposal) will have a chance to chime in after WWDC wraps up.
>> 
>> Cheers.  I'm at WWDC along with them staffing the Swift labs today if anybody wants to come find me and talk about this!
> 
> I would love to, but unfortunately didn’t get a ticket.  If you have a chance to discuss with Doug or other core team members please report back to the list!
> 
>> 
>>> 
>>>> 
>>>> ~Robert Widmann
>>>> 
>>>> 2016/06/16 8:04、Matthew Johnson <matthew at anandabits.com> のメッセージ:
>>>> 
>>>>> 
>>>>>> On Jun 16, 2016, at 9:49 AM, Robert Widmann <devteam.codafi at gmail.com> wrote:
>>>>>> 
>>>>>> Can you not see the links to the rest of the corelibs changes in the discussion?  Then I'll reproduce them here
>>>>> 
>>>>> Thanks.  I don’t see anything unexpected here.  The Swift PM case is one where the team wishes to take advantage of SE-0025 to tighten visibility and express their intent more explicitly.  
>>>>> 
>>>>> The automated find / replace migration you applied is correct, but maybe they want to slow down and review the changes so the results match the semantics they desire.  That seems reasonable to me.
>>>>> 
>>>>> 
>>>>>> 
>>>>>> - SwiftPM
>>>>>> 
>>>>>> https://github.com/apple/swift-package-manager/pull/410
>>>>>> 
>>>>>> - XCTest
>>>>>> 
>>>>>> https://github.com/apple/swift-corelibs-xctest/pull/124
>>>>>> 
>>>>>> - Foundation
>>>>>> 
>>>>>> https://github.com/apple/swift-corelibs-foundation/pull/413
>>>>>> 
>>>>>> 
>>>>>> ~Robert Widmann
>>>>>> 
>>>>>> 2016/06/16 7:35、Matthew Johnson <matthew at anandabits.com> のメッセージ:
>>>>>> 
>>>>>>> 
>>>>>>>> On Jun 16, 2016, at 9:30 AM, Robert Widmann <devteam.codafi at gmail.com> wrote:
>>>>>>>> 
>>>>>>>> Find it under our own pull requests on apple/swift#3000
>>>>>>> 
>>>>>>> You mean this one: https://github.com/apple/swift/pull/3000 right?
>>>>>>> 
>>>>>>> What specifically did you want me to look at here?  Of course this proposal is going to require a lot of changes to existing code (changing `private` to `fileprivate`).  That was vetted and accepted during the review process.  I don’t see how this is relevant to the current thread.  Is there some other part of the discussion I am not seeing?
>>>>>>> 
>>>>>>>> 
>>>>>>>> ~Robert Widmann
>>>>>>>> 
>>>>>>>> 2016/06/16 7:28、Matthew Johnson <matthew at anandabits.com> のメッセージ:
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>> On Jun 16, 2016, at 9:23 AM, Robert Widmann <devteam.codafi at gmail.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> Go checkout my branch!  And see the discussion there for how your proposal has impacted corelibs.
>>>>>>>>> 
>>>>>>>>> I’ll be happy to.  Can you please provide a link to the branch and discussion?
>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> ~Robert Widmann
>>>>>>>>>> 
>>>>>>>>>> 2016/06/16 5:50、Matthew Johnson <matthew at anandabits.com> のメッセージ:
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> Sent from my iPad
>>>>>>>>>>> 
>>>>>>>>>>> On Jun 16, 2016, at 5:20 AM, Brent Royal-Gordon via swift-evolution <swift-evolution at swift.org> wrote:
>>>>>>>>>>> 
>>>>>>>>>>>>> 6. With the core team tied up at WWDC, you may want to temporarily forbid the use of `private` on a type and revisit the matter when people are less busy; if necessary, we could even ship Swift 3 that way. Or you may want to consider making a guess as to a good implementation model to apply. Two suggestions for alternate implementation models:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> a. Introduce a `parent` access level, meaning "visible in scopes within this file where the parent is visible", which is between `fileprivate` and `private`. Just as `internal` is the maximum inherited access level, `parent` is the minimum, so the members of a `private` type would inherit `parent` visibility. `parent` might be an entirely compiler-internal concept, with no utterable access control keyword.
>>>>>>>>>>>> 
>>>>>>>>>>>> Thinking about this more, I notice that `fileprivate` as currently defined doesn't actually make any sense to say inside a `private` type: if your parent type has less-than-file-wide visibility, nothing in the file that's outside its scope can see you anyway. Therefore, we could redefine `fileprivate` thusly:
>>>>>>>>>>>> 
>>>>>>>>>>>> 1. A member with `fileprivate` visibility is visible within the scope in which the nearest containing `private` type is visible.
>>>>>>>>>>>> 2. If there are no containing `private` types, it is visible within the file containing it.
>>>>>>>>>>>> 3. Just as the members of a `public` type are `internal`, so the members of a `private` type are `fileprivate`.
>>>>>>>>>>>> 
>>>>>>>>>>>> This kind of suggests that we ought to rename `fileprivate` to something that, y'know, doesn't say "file" in it. However, I can scarcely imagine the results of a round of bikeshedding without parental supervision from the core team, so I don't dare make any suggestions.
>>>>>>>>>>> 
>>>>>>>>>>> I am not convinced this is necessary.  If there *is* a containing 'private' scope you can just leave the member unannotated to get this behavior.  If there isn't you can use 'fileprivate' as it is already defined.  Why is that not sufficient?
>>>>>>>>>>> 
>>>>>>>>>>> If you really want a second, more nuanced and complex scope-dependent access control mechanism I think you'll need to submit a proposal for it.  A simple renaming to 'fileprivate' is what has been accepted thus far.  
>>>>>>>>>>> 
>>>>>>>>>>> The main argument for what you suggest is that it would provide a way to ensure visibility of the member is*never* more than the file, but is as visible as possible within the file, while being less sensitive to changes in visibility of surrounding scopes.  IMO we need to get some experience with SE-0025 in real code before we know whether this is a problem that needs solving or not.
>>>>>>>>>>> 
>>>>>>>>>>> -Matthew
>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> -- 
>>>>>>>>>>>> Brent Royal-Gordon
>>>>>>>>>>>> Architechies
>>>>>>>>>>>> 
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> swift-evolution mailing list
>>>>>>>>>>>> swift-evolution at swift.org
>>>>>>>>>>>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160616/b241b998/attachment.html>


More information about the swift-evolution mailing list