Private members may not satisfy protocol requirements, ever. (#3842)

* Private members may not satisfy protocol requirements, ever.

...because by construction they can be invoked from outside of the
type.

Finishing up SE-0025 ('private' and 'fileprivate').

* Update docs and mark SE-0025 ('private' and 'fileprivate') as done!

There's still improvements we can make (see 508e825), but the feature
is in place and should be working correctly.

Author: jrose-apple

CHANGELOG.md

@@ -3,6 +3,8 @@ Note: This is in reverse chronological order, so newer entries are added to the
 Swift 3.0
 ---------
 
+* [SE-0025](https://github.com/apple/swift-evolution/blob/master/proposals/0025-scoped-access-level.md): A declaration marked as `private` can now only be accessed within the lexical scope it is declared in (essentially the enclosing curly braces `{}`). A `private` declaration at the top level of a file can be accessed anywhere in that file, as in Swift 2. The access level formerly known as `private` is now called `fileprivate`.
+
 * [SE-0131](https://github.com/apple/swift-evolution/blob/master/proposals/0131-anyhashable.md):
   The standard library provides a new type `AnyHashable` for use in heterogenous
   hashed collections. Untyped `NSDictionary` and `NSSet` APIs from Objective-C

docs/AccessControl.rst

@@ -7,32 +7,38 @@ The general guiding principle of Swift access control:
   **No entity can be defined in terms of another entity that has a lower
   access level.**
 
-There are three levels of access: "private", "internal", and "public".
-Private entities can only be accessed from within the source file where they
-are defined. Internal entities can be accessed anywhere within the module they
-are defined. Public entities can be accessed from anywhere within the module
-and from any other context that imports the current module.
+There are four levels of access: "private", "fileprivate", "internal", and
+"public". Private entities can only be accessed from within the lexical scope
+where they are defined. File-private entities can only be accessed from within
+the source file where they are defined. Internal entities can be accessed
+anywhere within the module they are defined. Public entities can be accessed
+from anywhere within the module and from any other context that imports the
+current module.
 
 The names ``public`` and ``private`` have precedent in many languages;
-``internal`` comes from C#. In the future, ``public`` may be used for both API
-and SPI, at which point we may design additional annotations to distinguish the
-two.
+``internal`` comes from C# and ``fileprivate`` from the Swift community. In the
+future, ``public`` may be used for both API and SPI, at which point we may
+design additional annotations to distinguish the two.
 
 By default, most entities in a source file have ``internal`` access.
 This optimizes for the most common case—a single-target application
 project—while not accidentally revealing entities to clients of a framework
 module.
 
+.. warning:: This document has not yet been updated for SE-0117, which adds the 
+  "open" level of access.
+
+
 .. contents:: :local:
 
 Rules
 ======
 
 Access to a particular entity is considered relative to the current
-*access context.* The access context of an entity is the current
-file (if ``private``), the current module (if ``internal``), or the current
-program (if ``public``). A reference to an entity may only be written within
-the entity's access context.
+*access scope.* The access scope of an entity is its immediate lexical scope
+(if ``private``), the current file (if ``fileprivate``), the current module (if
+``internal``), or the current program (if ``public``). A reference to an entity
+may only be written within the entity's access scope.
 
 If a particular entity is not accessible, it does not appear in name lookup,
 unlike in C++. However, access control does not restrict access to members via
@@ -43,9 +49,13 @@ visibility of symbols in a linked binary.
 Globals and Members
 -------------------
 
-A global function, constant, or variable may have any access level less than
-or equal to the access level of its type. That is, a ``private`` constant can
-have ``public`` type, but not the other way around.
+All globals and members have a default access level of ``internal``, except
+within extensions (as described below).
+
+A declaration may have any access level less than or equal to the access level
+of its type. That is, a ``private`` constant can have ``public`` type, but not
+the other way around. It is legal for a member to have greater access than its
+enclosing type, but this has no effect.
 
 Accessors for variables have the same access level as their associated variable.
 The setter may be explicitly annotated with an access level less than or equal
@@ -56,10 +66,6 @@ An initializer, method, subscript, or property may have any access level less
 than or equal to the access level of its type (including the implicit 'Self'
 type), with a few additional rules:
 
-- If the type's access level is ``private``, the access level of members
-  defaults to ``private``. If the type's access level is ``internal`` or
-  ``public``, the access level of members defaults to ``internal``.
-
 - If a member is used to satisfy a protocol requirement, its access level must
   be at least as high as the protocol conformance's; see :ref:`Protocols` below.
 
@@ -130,10 +136,13 @@ struct, enum, or class may be extended whenever it is accessible.
 A class may be subclassed whenever it is accessible. A class may have any
 access level less than or equal to the access level of its superclass.
 
-Members in an extension have the same default access level as members declared
-within the extended type. However, an extension may be marked with an explicit
-access modifier (e.g. ``private extension``), in which case the default
-access level of members within the extension is changed to match.
+Members within constrained extensions must have access less than or equal to
+the access level of the types used in the constraints.
+
+An extension may be marked with an explicit access modifier (e.g. ``private
+extension``), in which case the default access level of members within the
+extension is changed to match. No member within such an extension may have
+broader access than the new default.
 
 Extensions with explicit access modifiers may not add new protocol
 conformances, since Swift does not support private protocol conformances
@@ -167,9 +176,9 @@ or other extensions from outside the module. Therefore, members of a subclass
 or extension will not conflict with or inadvertently be considered to override
 non-accessible members of the superclass.
 
-Both ``private`` and ``internal`` increase opportunities for devirtualization,
+Access levels lower than ``public`` increase opportunities for devirtualization,
 though it is still possible to put a subclass of a ``private`` class within the
-same file.
+same scope.
 
 Most information about a non-``public`` entity still has to be put into a
 module file for now, since we don't have resilience implemented. This can be
@@ -186,9 +195,10 @@ selector for members, everything can be inspected at runtime, and even a
 private member can cause selector conflicts. In this case, access control is
 only useful for discipline purposes.
 
-Members explicitly marked ``private`` are *not* exposed to Objective-C unless
-they are also marked ``@objc`` (or ``@IBAction`` or similar), even if declared
-within a class implicitly or explicitly marked ``@objc``.
+Members explicitly marked ``private`` or ``fileprivate`` are *not* exposed to
+Objective-C unless they are also marked ``@objc`` (or ``@IBAction`` or
+similar), even if declared within a class implicitly or explicitly marked
+``@objc``.
 
 Any ``public`` entities will be included in the generated header. In an
 application or unit test target, ``internal`` entities will be exposed as well.
@@ -201,7 +211,7 @@ This proposal omits two forms of access control commonly found in other
 languages, a "class-implementation-only" access (often called "private"), and a
 "class and any subclasses" access (often called "protected"). We chose not to
 include these levels of access control because they do not add useful
-functionality beyond ``private``, ``internal``, and ``public``.
+functionality beyond ``private``, ``fileprivate``, ``internal``, and ``public``.
 
 "class-only"
   If "class-only" includes extensions of the class, it is clear that it
@@ -211,10 +221,9 @@ functionality beyond ``private``, ``internal``, and ``public``.
   limit forces code to be declared within the class that might otherwise
   naturally be a top-level helper or an extension method on another type.
 
-  ``private`` serves the proper use case of limiting access to the
+  ``private`` and ``fileprivate`` serve the use case of limiting access to the
   implementation details of a class (even from the rest of the module!) while
-  not requiring that all of those implementation details be written lexically
-  inside the class.
+  not tying access to the notion of type.
 
 "protected"
   "protected" access provides no guarantees of information hiding, since any
@@ -235,7 +244,7 @@ Potential Future Directions
 ===========================
 
 - Allowing ``private`` or ``internal`` protocol conformances, which are only
-  accessible at compile-time from a particular access context.
+  accessible at compile-time from a particular access scope.
 
 - Limiting particular capabilities, such as marking something ``final(public)``
   to restrict subclassing or overriding outside of the current module.

docs/OptimizationTips.rst

@@ -134,17 +134,17 @@ in the following ``C.array1`` and ``D.array1`` will be accessed directly
      d.array2[i] = ... // Will access D.array2 through dynamic dispatch.
   }
 
-Advice: Use 'private' when declaration does not need to be accessed outside of file
------------------------------------------------------------------------------------
-
-Applying the ``private`` keyword to a declaration restricts the visibility of
-the declaration to the file in which it is declared. This allows the compiler to
-be able to ascertain all other potentially overriding declarations. Thus the
-absence of any such declarations enables the compiler to infer the ``final``
-keyword automatically and remove indirect calls for methods and field accesses
-accordingly. For instance in the following, ``e.doSomething()`` and
-``f.myPrivateVar``, will be able to be accessed directly assuming ``E``, ``F``
-do not have any overriding declarations in the same file:
+Advice: Use 'private' and 'fileprivate' when declaration does not need to be accessed outside of file
+-----------------------------------------------------------------------------------------------------
+
+Applying the ``private`` or ``fileprivate`` keywords to a declaration restricts
+the visibility of the declaration to the file in which it is declared. This
+allows the compiler to be able to ascertain all other potentially overriding
+declarations. Thus the absence of any such declarations enables the compiler to
+infer the ``final`` keyword automatically and remove indirect calls for methods
+and field accesses accordingly. For instance in the following,
+``e.doSomething()`` and ``f.myPrivateVar``, will be able to be accessed directly
+assuming ``E``, ``F`` do not have any overriding declarations in the same file:
 
 ::
 
@@ -153,7 +153,7 @@ do not have any overriding declarations in the same file:
   }
 
   class F {
-    private var myPrivateVar : Int
+    fileprivate var myPrivateVar : Int
   }
 
   func usingE(_ e: E) {

lib/Sema/TypeCheckProtocol.cpp

@@ -1220,11 +1220,9 @@ checkWitnessAccessibility(Accessibility *requiredAccess,
                           bool *isSetter) {
   *isSetter = false;
 
-  // FIXME: Handle "private(set)" requirements.
   *requiredAccess = std::min(Proto->getFormalAccess(), *requiredAccess);
-
-  if (*requiredAccess == Accessibility::Private)
-    return false;
+  if (TC.getLangOpts().EnableSwift3Private)
+    *requiredAccess = std::max(*requiredAccess, Accessibility::FilePrivate);
 
   Accessibility witnessAccess = witness->getFormalAccess(DC);
 
@@ -1242,9 +1240,20 @@ checkWitnessAccessibility(Accessibility *requiredAccess,
     *isSetter = true;
 
     auto ASD = cast<AbstractStorageDecl>(witness);
-    const DeclContext *accessDC = nullptr;
-    if (*requiredAccess == Accessibility::Internal)
+    const DeclContext *accessDC;
+    switch (*requiredAccess) {
+    case Accessibility::Public:
+      accessDC = nullptr;
+      break;
+    case Accessibility::Internal:
       accessDC = DC->getParentModule();
+      break;
+    case Accessibility::FilePrivate:
+    case Accessibility::Private:
+      accessDC = DC->getModuleScopeContext();
+      break;
+    }
+
     if (!ASD->isSetterAccessibleFrom(accessDC))
       return true;
   }

stdlib/public/SDK/Foundation/IndexSet.swift

@@ -792,7 +792,7 @@ extension IndexSet : CustomStringConvertible, CustomDebugStringConvertible, Cust
 
 /// Iterate two index sets on the boundaries of their ranges. This is where all of the interesting stuff happens for exclusive or, intersect, etc.
 private struct IndexSetBoundaryIterator : IteratorProtocol {
-    private typealias Element = IndexSet.Element
+    typealias Element = IndexSet.Element
 
     private var i1 : IndexSet.RangeView.Iterator
     private var i2 : IndexSet.RangeView.Iterator

test/NameBinding/accessibility.swift

@@ -144,7 +144,11 @@ struct ConformerByLocalType : TypeProto {
 }
 
 private struct PrivateConformerByLocalType : TypeProto {
-  private struct TheType {} // okay
+  struct TheType {} // okay
+}
+
+private struct PrivateConformerByLocalTypeBad : TypeProto {
+  private struct TheType {} // expected-error {{struct 'TheType' must be as accessible as its enclosing type because it matches a requirement in protocol 'TypeProto'}} {{3-10=fileprivate}}
 }
 #endif
 

test/SILOptimizer/basic-callee-printer.sil

@@ -392,7 +392,7 @@ private protocol private_proto_4 {
 }
 
 public class private_proto_public_class_private_method : private_proto_4 {
-  private func theMethod()
+  fileprivate func theMethod()
 }
 
 private func call_through_private_proto_4<T : private_proto_4>(x: T)

test/SILOptimizer/function_order.sil

@@ -340,7 +340,7 @@ private protocol private_proto_4 {
 }
 
 public class private_proto_public_class_private_method : private_proto_4 {
-  private func theMethod()
+  fileprivate func theMethod()
 }
 
 private func call_through_private_proto_4<T : private_proto_4>(x: T)

test/Sema/accessibility.swift

@@ -22,7 +22,7 @@ public struct PublicStruct: PublicProto, InternalProto, FilePrivateProto, Privat
   private func publicReq() {} // expected-error {{method 'publicReq()' must be declared public because it matches a requirement in public protocol 'PublicProto'}} {{3-10=public}}
   private func internalReq() {} // expected-error {{method 'internalReq()' must be declared internal because it matches a requirement in internal protocol 'InternalProto'}} {{3-10=internal}}
   private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
-  private func privateReq() {}
+  private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
 
   public var publicVar = 0
 }
@@ -32,7 +32,7 @@ internal struct InternalStruct: PublicProto, InternalProto, FilePrivateProto, Pr
   private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-10=internal}}
   private func internalReq() {} // expected-error {{method 'internalReq()' must be declared internal because it matches a requirement in internal protocol 'InternalProto'}} {{3-10=internal}}
   private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
-  private func privateReq() {}
+  private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
 
   public var publicVar = 0
 }
@@ -42,17 +42,17 @@ fileprivate struct FilePrivateStruct: PublicProto, InternalProto, FilePrivatePro
   private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-10=fileprivate}}
   private func internalReq() {} // expected-error {{method 'internalReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'InternalProto'}} {{3-10=fileprivate}}
   private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
-  private func privateReq() {}
+  private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
 
   public var publicVar = 0
 }
 
 // expected-note@+1 * {{type declared here}}
 private struct PrivateStruct: PublicProto, InternalProto, FilePrivateProto, PrivateProto {
-  private func publicReq() {}
-  private func internalReq() {}
-  private func filePrivateReq() {}
-  private func privateReq() {}
+  private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-10=fileprivate}}
+  private func internalReq() {} // expected-error {{method 'internalReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'InternalProto'}} {{3-10=fileprivate}}
+  private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
+  private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
 
   public var publicVar = 0
 }

test/Sema/accessibility_private.swift

@@ -110,3 +110,38 @@ class Sub : Container {
   var subInner: PrivateInner? // FIXME expected-error {{use of undeclared type 'PrivateInner'}}
   var subInnerQualified: Container.PrivateInner? // FIXME expected-error {{'PrivateInner' is not a member type of 'Container'}}
 }
+
+
+protocol VeryImportantProto {
+  associatedtype Assoc
+  var value: Int { get set } // expected-note {{protocol requires property 'value' with type 'Int'; do you want to add a stub?}}
+}
+
+private struct VIPPrivateType : VeryImportantProto {
+  private typealias Assoc = Int // expected-error {{type alias 'Assoc' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}}
+  var value: Int
+}
+
+private struct VIPPrivateProp : VeryImportantProto {
+  typealias Assoc = Int
+  private var value: Int // expected-error {{property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{3-10=fileprivate}}
+}
+
+private struct VIPPrivateSetProp : VeryImportantProto {
+  typealias Assoc = Int
+  private(set) var value: Int // expected-error {{setter for property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{3-10=fileprivate}}
+}
+
+private class VIPPrivateSetBase {
+  private var value: Int = 0
+}
+private class VIPPrivateSetSub : VIPPrivateSetBase, VeryImportantProto { // expected-error {{type 'VIPPrivateSetSub' does not conform to protocol 'VeryImportantProto'}}
+  typealias Assoc = Int
+}
+
+private class VIPPrivateSetPropBase {
+  private(set) var value: Int = 0 // expected-error {{setter for property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{3-10=fileprivate}}
+}
+private class VIPPrivateSetPropSub : VIPPrivateSetPropBase, VeryImportantProto {
+  typealias Assoc = Int
+}