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

[John McCall]

> On Jun 23, 2016, at 6:03 PM, Jordan Rose via swift-evolution <swift-evolution at swift.org> wrote:
>> On Jun 15, 2016, at 18:47, Charles Srstka via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>> 
>>> On Jun 15, 2016, at 8:36 PM, Robert Widmann <devteam.codafi at gmail.com <mailto:devteam.codafi at gmail.com>> wrote:
>>> 
>>> Point 3 is *not* how member lookup applies access control levels to unannotated properties of outer structures (see https://github.com/CodaFi/swift/blob/fb9f9536a5760369457d0f9c49599415cbc36e07/lib/Sema/TypeCheckDecl.cpp#L1470 <https://github.com/CodaFi/swift/blob/fb9f9536a5760369457d0f9c49599415cbc36e07/lib/Sema/TypeCheckDecl.cpp#L1470>) and makes no sense.  They do not default to "internal" when unannotated, they default to the highest possible access level they can get given the decl they're in.  A private structure will necessarily have private members.  This is the whole point of me raising this issue.  If we were to break containment we would break the very motivation for this proposal.  And if we wish to do this to get this feature right, then the proposal needs to be amended to include that kind of caveat.
>> 
>> This isn’t correct. If the outer type is marked “public”, and its properties are not annotated, those properties will be internal, *not* public, and you will not be able to see them outside of the module.
>> 
>> The rule can basically be summed up as “internal by default, unless we can’t because our enclosing type is more restrictive than internal. Then, be as visible as the enclosing type is.”
> 
> Robert is correct in the current interpretation: the default access level of a members is the access level of the enclosing type, except that nothing is ever public by default, and so you get ‘internal’ if the type is public. This suggests a rule that the access level of a member is never greater than the access level for a type, and the compiler will warn if you break this rule.
> 
> (Citation: I wrote both the implementation and the original proposal <https://github.com/apple/swift/blob/master/docs/AccessControl.rst> for Swift access control. Please let’s not continue talking about the current (pre-SE-0025) behavior or its intent.)
> 
> —
> 
> As everyone has pointed out, this does not produce desirable behavior in the presence of the new ‘private’.
> 
> Here is my understanding of the problem:
> 
> - Blindly applying the current rules, the members of a ‘private' type default to being ‘private'.
> - Blindly applying the current rules, the members of a ‘private' type cannot be marked as anything but ‘private’ or the compiler will warn.
> 
> (Thank you Robert for explaining the problem to me in person.)
> 
> —

Right.  I think the basic problem is that a rule mandating things by lexical nesting cannot interwork successfully with an access level that is also based on lexical nesting.

> I think Brent has done a great job of summing up the options with regard to ‘private’ on types:
> 
>> 1. Having an extra, unutterable access modifier that floats relative to the other access levels.
>> 
>> 2. Having an extra access modifiers that floats relative to the other access levels, but has a name (like `default`).
>> 
>> 3. Having an extra, unutterable access modifier between `fileprivate` and `private`.
>> 
>> 4. Using `internal` whenever there's no access modifier, and not warning if the effective scope is smaller.
>> 
>> 5. Having an extra access modifier between `fileprivate` and `private`, but which has a name (like `inheritprivate`) 
>> 
>> 6. Changing the definition of `fileprivate` to be within the surrounding private type if there is one, without changing the keyword.
>> 
>> 7. Changing the definition of `fileprivate` to be within the surrounding private type if there is one, and changing the keyword to match.
> 
> 
> (The order is Brent’s preferred order, with higher numbers being more preferred.)

Have you considered just removing this no-greater-access rule?  It seems kind of low-value to me, certainly a worthy check for a linter or maybe a warning but an unnecessarily pedantic check to be a hard error.  Just let the actual visibility of a member be limited by the visibility of its context; you might think that would regress diagnostics, but QoI here is pretty easy, e.g. "cannot access member of a private extension from a different file" instead of "cannot access private member from a different file".  As it is, you can't just drop the visibility of a type without fixing all of its members.

Otherwise I agree that 6 and 4 are reasonable ways of talking about it.

John.

> 
> If the discussion on SE-0025 is any indication, it’s that Naming Is Hard and that I really don’t want to come up with a new name at this point. Therefore, my preferred solution is (6), followed by (4). The concrete change to the rules I outlined above would be as follows:
> 
> - The default level of members is ‘fileprivate’ for a private or fileprivate type, and ‘internal’ for an internal or public type.
> - Any rule where something must be “as visible as the type” (such as minimum access for required initializers) must be ‘fileprivate’ for a private type (rather than private).
> - As a consequence, the compiler will warn when the access level of a member is “too high”; that is, where the next level down wouldn’t change anything.
> 
> Note that there don’t actually need to be any changes to access rules to make this work; when a type is private, there won’t be any uses of it outside of the enclosing scope, and so nobody elsewhere in the file will be able to call one of those fileprivate members.
> 
> —
> 
> Ilya hasn’t been active on swift-evolution lately, so if no one has serious objections to (6) besides “I don’t like the name” Robert and I will write up an official amendment to SE-0025 and take it to Doug or Chris. If they think this is not a good idea to just move forward with, I’ll ask them whether they’d prefer a partial implementation that disallows ‘private’ on nominal types, or whether they want to kick the whole proposal back for review (and possibly slipping out of Swift 3).
> 
> I am not core team, but I do sit near them and have been able to convince them of things. :-) Hopefully we can get this resolved soon. Thanks again to Robert for bringing it up and everyone else for bringing their minds to bear on the problem!
> 
> Jordan
> 
> _______________________________________________
> 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/20160623/c0dc0f48/attachment.html>