[swift-evolution] Fix "private extension" (was "Public Access Modifier Respected in Type Definition")
Tony Allevato
tony.allevato at gmail.com
Fri Oct 6 22:01:10 CDT 2017
On Fri, Oct 6, 2017 at 7:07 PM Jose Cheyo Jimenez <cheyo at masters3d.com>
wrote:
> On Oct 6, 2017, at 11:07 AM, Tony Allevato <tony.allevato at gmail.com>
> wrote:
>
> On Fri, Oct 6, 2017 at 10:16 AM Jose Cheyo Jimenez via swift-evolution <
> swift-evolution at swift.org> wrote:
>
>> On Oct 6, 2017, at 7:10 AM, Vladimir.S <svabox at gmail.com> wrote:
>>
>> On 05.10.2017 20:52, Jose Cheyo Jimenez via swift-evolution wrote:
>>
>> On Oct 5, 2017, at 4:32 AM, David Hart <david at hartbit.com<
>> mailto:david at hartbit.com <david at hartbit.com>>> wrote:
>>
>>
>> On 5 Oct 2017, at 07:34, Jose Cheyo Jimenez via swift-evolution <
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>> wrote:
>>
>> I appreciate the enthusiasm but this is not a bug. This was a deliberate
>> change in swift 3 to make `private extension` usable. If this was a bug
>> then during swift 3 we should have disallowed `private extension` and only
>> allowed `fileprivate extension` but that is not what happened. `private
>> extension` has worked the same since swift 1. I’ve always used `private
>> extension` when I want to add methods to String or other build in types.
>>
>>
>> It’s not a bug, but its unfortunate the behaviour wasn’t changed at the
>> same time as SE-0169, and it now is very inconsistent. I also don’t have to
>> rehash previous discussions, but if a Core Team member (Chris) is okay with
>> going ahead with this, perhaps we should consider it.
>>
>> This could have not been part of 169 because it would've required to
>> lower the visibility of the private extension modifier.
>> “No migration will be necessary as this proposal merely broadens the
>> visibility of|private|.”
>> There was a corner case mentioned when dealing with functions with the
>> same name and that was understandable.
>> private extension is consistent to the way the private scope rules work.
>> The word private is explicit at the top level because extensions can only
>> be declared at top level. Top level private is always fileprivate. The
>> inconsistency is that we have 1 scope ALC and the rest are not. An explicit
>> declaration should always take precedence when declaring something like an
>> access level override.
>>
>>
>> FWIW, I can't agree with this. 'private extension' is a real point of
>> additional confusion for access levels in Swift.
>> Extension *by itself* has no access level, we only can specify the
>> *default* (and the top most) access level for inner methods.
>> I.e. 'private' access modifier for extension has not the same meaning as
>> 'private' func/type/variable at file scope.
>> (Yes, I also believe we should disallow 'private' keyword at file level
>> and allow it only for extensions, so 'fileprivate' should be used
>> explicitly if one needs this. At least warning should be raised. This is
>> the *root* of the problem we discuss now. But unfortunately I don't expect
>> this could be supported.)
>>
>>
>> Wouldn't that just add a *special* rule to extensions? :)
>>
>> The latter is 'direct' access level for the func/type/variable and here
>> we apply the standard rule for scoped private, so 'private' for file scope
>> --> 'fileprivate'.
>>
>> The former means 'the default(and top most) modifier that will be
>> auto-inserted by compiler for all nested methods in extension'. This
>> relatively simple rule should not be complicated by additional rule as ",
>> but if it is private extension, result access level will be fileprivate,
>> you can't have extensions with private methods”
>>
>>
>> Private as it exist in swift now is the scope access control label. The
>> compiler does not insert the modifier without having to first compute what
>> access control level would be applied to the members of the extension.
>> Doing it the other way would be counterintuitive for an scope access label.
>> In my code base I disallow top level fileprivate because private top level
>> is fileprivate. This is a matter of taste and a linter here would help like
>> a mentioned up thread.
>>
>
> This is the sticking point, which is why there are two possible
> interpretations of "private extension":
>
> Choice 1) Attach-then-evaluate. "ACL extension { ... }" is a syntactic
> shortcut for "extension { ACL ... }". Under that definition, the ACL is
> evaluated as if it were attached to each declaration, so "private
> extension" would expand to "private" in front of each decl.
>
> Choice 2) Evaluate-then-attach. "ACL extension { ... }" is evaluated such
> that "ACL" takes on the meaning based on its scope; since it's equivalent
> to "fileprivate" there, that is what is attached to each declaration inside
> the extension.
>
>
> Yep. This is the issue. Nice summary!
>
>
> The phrasing in the official Swift language guide doesn't specifically
> state it, but I think most readers would interpret the following as #1:
>
>
> "Alternatively, you can mark an extension with an explicit access-level
> modifier (for example, `private extension`) to set a new default access
> level for all members defined within the extension."
>
> I personally find that choice to be the clearer interpretation of the
> rule, because it's based entirely on what words are in the source file and
> not about how they interact in special edge cases.
>
>
> Documentation is hard to keep in sync.
>
> https://github.com/apple/swift-evolution/blob/master/proposals/0025-scoped-access-level.md#complications-with-private-types
>
>
>
At the time SE-0025 was accepted, "private extension" would have been
meaningless if it did not mean "fileprivate" because it predated the
SE-0169 behavior extending "private" to extensions in the same file. The
very issue being debated here is whether the oversight that SE-0169 did not
consider extensions—now that "private extension" *could* have a meaningful
use separate from "fileprivate extension"—is something that is worth
correcting.
If the documentation is out-of-date and needs to be updated to list
describe unintuitive special behavior, why not use the opportunity to make
the behavior intuitive and consistent instead?
> I also think it's hard to rationalize "private extension" working like #2
> because compared to #1, it's both duplicative ("private extension" and
> "fileprivate extension" are awkwardly the same) _and_ it is strictly less
> flexible (there is _no_ way using that syntax to define an extension whose
> members are private, which is an inconsistent hole in the language).
>
>
> This compiles on an Xcode 9 playground. SE-0025 doesn't only affect
> extensions.
> ``MyClass.myFunc()` looks like (Choice 2) to me. Same as `private
> extension MyClass2`.
>
>
>
> private class MyClass {
> static func myFunc(){
> print(“Acts likes fileprivate")
> }
> }
>
> private class MyClass2 {
> }
>
> private extension MyClass2{
> static func myFunc2(){
> print("Same as MyClass.myFunc")
> }
> }
>
>
> MyClass.myFunc() // Acts likes fileprivate
> MyClass2.myFunc2() // Same as MyClass.myFunc
>
>
>
This example is somewhat irrelevant to what's being discussed here—it's
purely about SE-0025 behavior rather than, as the original message said,
the extremely narrow specific case of access control for extensions.
>
>
>>
>> And, as was already said, this inconsistency leads to *relaxed* access
>> level, which can lead to bugs. If one expects 'private extension' means
>> 'fileprivate extension' - compiler will show(with error) that this
>> assumption is wrong just after the first attempt to access methods from
>> outside of the extended type.
>> But if one expects true 'private' access level - the methods from private
>> extension could be called from any other code in the same file(by mistake,
>> or because code was written a long time ago, or by another developer) and
>> this clearly could produce complex bugs.
>>
>> Also, isn't it a strange code below:
>>
>> private extension MyType {
>> func foo() {}
>> private bar() {}
>> fileprivate baz() {} // note that "usually" fileprivate is 'wider'
>> access level
>> }
>>
>>
>> This is also strange too :)
>>
>> fileprivate class MyType {
>> open func foo(){} // Is this open or fileprivate?
>> public func bar(){}
>> }
>>
>> open class MyType2 {
>> }
>>
>> open extension MyType2 { // Error: Extensions cannot use 'open' as their
>> default access; use 'public'
>> func baz(){}
>> }
>>
>>
>>
>> but it has *currently* a sense - 'foo' is fileprivate, and 'bar' is
>> 'true' private.
>> Yes, currently we have a warning about 'baz': "warning: declaring a
>> fileprivate instance method in a private extension", but then we have a
>> question "Why?", as private at top level == fileprivate. and this does not
>> produce any warnings:
>> fileprivate extension MyType {
>> fileprivate func foo() {}
>> }
>>
>> Even more, someone can think "why we need 'private' declaration in
>> private extension, probably this is a mistake i.e. unnecessary duplication
>> of code, I'll refactor this and delete this explicit 'private' because
>> extension is already private' and so will open doors for future problems.
>>
>> So I do believe we really need to remove that ugly inconsistency and make
>> Swift better to write, understand and support the code.
>>
>>
>> This is matter of taste. For example I think fileprivate is ugly and
>> having both private and fileprivate makes the code hard to understand.
>>
>>
>> Vladimir.
>>
>>
>> private is different because it is scoped so because of that it is also
>> different when dealing with extensions. Top level private is always the
>> same as fileprivate thanks to its scoped nature.
>>
>> Making private the scope ACL was a mistake but that ship has sailed and
>> so has this one imo.
>>
>>
>>
>> On Oct 4, 2017, at 10:05 PM, Tony Allevato <tony.allevato at gmail.com <
>> mailto:tony.allevato at gmail.com <tony.allevato at gmail.com>>> wrote:
>>
>> Trust me, I'm the last person who wants to rehash access levels in Swift
>> again. But that's not what's happening here, IMO, and fixing bugs is not
>> just "a change for the sake of changing."
>>
>> The current behavior of "private extension" is *incorrect*, because it's
>> entirely inconsistent with how access levels on extensions are documented
>> to behave and it's inconsistent with how other access levels apply to
>> extensions.
>>
>> Can anyone think of a reason—other than "it's too late to change it"—why
>> "private extension" and "fileprivate extension" should behave the same, and
>> why "X extension { decl }" should be identical to "extension { X decl }"
>> for all X *except* "private"?
>>
>> Yes, it's absolutely unfortunate that this oversight was not addressed
>> when the other access level changes were made. But we shouldn't have to
>> live with bugs in the language because we're afraid of some unknown amount
>> of churn among code that is already written incorrectly. Nor is fixing this
>> bug declaring open season on other, unrelated access level debates. Do you
>> have data that shows that the amount of code broken because it's using
>> "private" when it really should be saying "fileprivate" is high enough that
>> we should just leave the bug there?
>>
>> On Wed, Oct 4, 2017 at 9:51 PM Jose Cheyo Jimenez via swift-evolution <
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>> wrote:
>>
>> There was a high bar for breaking changes in swift 4 and is even
>> higher for swift 5. se-110 was approved and
>> implemented on the premises that it was not a big change but it was
>> breaking code so it got reverted. Sure the
>> migrator was making this easier but the result was a usability
>> regression. I think this is a change just for the
>> sake of changing. This will cause unnecessary churn. Let’s leave ACLs
>> alone for the next few versions of swift
>> unless we have a way better system.
>>
>>
>> https://lists.swift.org/pipermail/swift-evolution-announce/2017-June/000386.html
>>
>>
>>
>>
>>
>> On Oct 4, 2017, at 8:47 PM, BJ Homer <bjhomer at gmail.com<
>> mailto:bjhomer at gmail.com <bjhomer at gmail.com>>> wrote:
>>
>> It certainly could break *some* code. But it only breaks code written
>> by an author who wrote ‘private
>> extension’ knowing that ‘fileprivate extension’ was also an option,
>> but still intended it to be shared with the
>> whole file. (If that code was from Swift 2, it would have already been
>> migrated to ‘fileprivate extension’ by
>> the 2->3 migrator.)
>>
>> So existing code that says ‘private extension’ was written in a Swift
>> 3 or 4 era when ‘fileprivate’ was an
>> option. If the goal was specifically to share it with the whole file,
>> it seems likely that most authors would
>> have used ‘fileprivate extension’ instead of ‘private extension’, as
>> that better communicates the intention.
>> Regardless, though, we could check against the Swift source
>> compatibility test suite to see how widespread that is.
>>
>> Regardless, I think this change makes Swift a better language, and I’m
>> in favor of it.
>>
>> -BJ
>>
>> On Oct 4, 2017, at 9:10 PM, Jose Cheyo Jimenez via swift-evolution <
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>> wrote:
>>
>>
>>
>> On Oct 2, 2017, at 9:59 PM, David Hart via swift-evolution <
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>> wrote:
>>
>>
>>
>> On 3 Oct 2017, at 05:12, Xiaodi Wu via swift-evolution <
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>> wrote:
>>
>> On Mon, Oct 2, 2017 at 9:16 PM, Matthew Johnson via swift-evolution<
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>>wrote:
>>
>>
>>
>> Sent from my iPad
>>
>> On Oct 2, 2017, at 7:33 PM, Jordan Rose via swift-evolution <
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>>
>> wrote:
>>
>>
>>
>> On Oct 2, 2017, at 03:25, Vladimir.S via swift-evolution <
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>>
>> wrote:
>>
>> On 01.10.2017 1:18, Chris Lattner wrote:
>>
>> On Sep 29, 2017, at 10:42 AM, Xiaodi Wu via swift-evolution <
>> swift-evolution at swift.org
>> <mailto:swift-evolution at swift.org <swift-evolution at swift.org>>>
>> wrote:
>>
>> Vladimir, I agree with you on that change, but it’s a separate
>> topic from this one.
>>
>> Tony is absolutely correct that this topic has already been
>> discussed. It is a deliberate design
>> decision that public types do not automatically expose members
>> without explicit access modifiers;
>> this has been brought up on this list, and it is clearly not in
>> scope for discussion as no new
>> insight can arise this late in the game. The inconsistency with
>> public extensions was brought up,
>> the proposed solution was to remove modifiers for extensions, but
>> this proposal was rejected. So,
>> the final design is what we have.
>>
>> Agreed. The core team would only consider a refinement or change
>> to access control if there were
>> something actively broken that mattered for ABI stability.
>>
>>
>> So we have to live with *protected* extension inconsistency for
>> very long time just because core team
>> don't want to even discuss _this particular_ inconsistency(when
>> access level in *private extension*
>> must be private, not fileprivate)?
>>
>> Yes, we decided that access level for extension will mean a
>> default and top most access level for
>> nested methods, OK. But even in this rule, which already differ
>> from access modifiers for types, we
>> have another one special case for 'private extension'.
>>
>> Don't you think this is not normal situation and actually there
>> IMO can't be any reason to keep this
>> bug-producing inconsistency in Swift? (especially given Swift 5
>> seems like is a last moment to fix this)
>>
>>
>> I hate to say it but I'm inclined to agree with Vladimir on this.
>> "private extension" has a useful
>> meaning now distinct from "fileprivate extension", and it was an
>> oversight that SE-0169
>> <
>> https://github.com/apple/swift-evolution/blob/master/proposals/0169-improve-interaction-between-private-declarations-and-extensions.md>
>> didn't
>> include a fix here. On this/very narrow, very specific/access
>> control issue I think it would still be
>> worth discussing; like Xiaodi said it's not related to James'
>> original thread-starter.
>>
>>
>> I agree with this in principle but would not want to see it become
>> a slippery slope back into extremely
>> long access control discussions.
>>
>>
>> As I've said elsewhere, I too agree with this in principle. I agree
>> with Jordan that the current state of
>> things is justifiable but the alternative would be somewhat superior,
>> agree that in a vacuum this very
>> narrow and specific discussion might be warranted, and agree also that
>> this could be a very slippery slide
>> down a very steep slope.
>>
>>
>> Same here. It’s the only grudge I have left with the current access
>> control situation. I remember Doug Gregor
>> and John McCall discussing this during the last access control
>> proposal. And I wouldn’t mind having a very
>> narrow discussion about only this.
>>
>> I organize my types into extensions for each conformance and for each
>> access control. I can currently
>> implicitly apply public or fileprivate to all members of an extension
>> but I have no way of doing the same for
>> private. That’s why I think it should be fixed.
>>
>>
>> This will break a bunch of code because `private extension`
>> has_always_meant `fileprivate extension`.Even
>> Swift 3 had this same behavior. Lowering the access level of the
>> extension members will hide a bunch of code
>> that was visible to the file.
>>
>> 169 was not a breaking change but this “fix” would have made it a
>> breaking change. I doubt 169 would had been
>> accepted if it was a breaking change. I don’t think it’s worth it.
>>
>>
>>
>> https://github.com/apple/swift-evolution/blob/master/proposals/0169-improve-interaction-between-private-declarations-and-extensions.md
>>
>>
>>
>>
>>
>> (I maintain that the current model does/not/ include a special
>> case; it simply means the 'private' is
>> resolved at the level of the extension rather than the level of
>> its members. But that isn't what people
>> expect and it's not as useful.)
>>
>>
>> I agree that changing the behavior of/all/ access modifiers on
>> extensions is out of scope. (I also
>> agree that it is a bad idea. Sorry, James, but wanting 'pubic'
>> here indicates that your mental model of
>> extensions does not match what Swift is actually doing, and that
>> could get you into trouble.)
>>
>> Jordan
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org
>> <swift-evolution at swift.org>>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>> _______________________________________________
>> 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/20171007/34af5e6d/attachment.html>
More information about the swift-evolution
mailing list