SE-0025: Allow public members inside internal types. (#3404)

(and any other member with higher access control than its enclosing type)

There's no effect, but it is now considered legal and the compiler will
no longer warn about it. This allows an API author to prototype their
API with proper access levels and still limit the top-level type.

If the new getEffectiveAccess computation turns out to be expensive, we
can cache the result.

Note that the compiler will still warn when putting a public member
inside an extension explicitly marked internal, because the extended
type could be public and then including a public member would be valid.
It is also still an error to put a public member inside a constrained
extension of an internal type, though I think this one is safe to
relax later.

Progress on SE-0025 ('private' and 'fileprivate')

include/swift/AST/Decl.h

@@ -5942,24 +5942,6 @@ inline bool ValueDecl::isSettable(const DeclContext *UseDC,
     return false;
 }
 
-namespace impl {
-  bool isInternalDeclEffectivelyPublic(const ValueDecl *VD);
-}
-
-inline Accessibility ValueDecl::getEffectiveAccess() const {
-  switch (getFormalAccess()) {
-  case Accessibility::Public:
-    return Accessibility::Public;
-  case Accessibility::Internal:
-    if (impl::isInternalDeclEffectivelyPublic(this)) {
-      return Accessibility::Public;
-    }
-    return Accessibility::Internal;
-  case Accessibility::Private:
-    return Accessibility::Private;
-  }
-}
-
 inline Optional<VarDecl *>
 NominalTypeDecl::ToStoredProperty::operator()(Decl *decl) const {
   if (auto var = dyn_cast<VarDecl>(decl)) {

include/swift/AST/DiagnosticsSema.def

@@ -945,10 +945,6 @@ ERROR(access_control_setter_more,none,
       "%select{variable|property|subscript}1 cannot have "
       "%select{PRIVATE|an internal|a public}2 setter",
       (Accessibility, unsigned, Accessibility))
-WARNING(access_control_member_more,none,
-    "declaring %select{PRIVATE|an internal|a public}0 %1 for "
-    "%select{a private|an internal|PUBLIC}2 %3",
-    (Accessibility, DescriptiveDeclKind, Accessibility, DescriptiveDeclKind))
 WARNING(access_control_ext_member_more,none,
     "declaring %select{PRIVATE|an internal|a public}0 %1 in "
     "%select{a private|an internal|PUBLIC}2 extension",

lib/AST/Decl.cpp

@@ -44,23 +44,6 @@
 
 using namespace swift;
 
-bool impl::isInternalDeclEffectivelyPublic(const ValueDecl *VD) {
-  assert(VD->getFormalAccess() == Accessibility::Internal);
-
-  if (VD->getAttrs().hasAttribute<VersionedAttr>())
-    return true;
-
-  if (auto *fn = dyn_cast<FuncDecl>(VD))
-    if (auto *ASD = fn->getAccessorStorageDecl())
-      if (ASD->getAttrs().hasAttribute<VersionedAttr>())
-        return true;
-
-  if (VD->getModuleContext()->isTestingEnabled())
-    return true;
-
-  return false;
-}
-
 clang::SourceLocation ClangNode::getLocation() const {
   if (auto D = getAsDecl())
     return D->getLocation();
@@ -1810,6 +1793,64 @@ SourceLoc ValueDecl::getAttributeInsertionLoc(bool forModifier) const {
   return resultLoc.isValid() ? resultLoc : getStartLoc();
 }
 
+/// Returns true if \p VD needs to be treated as publicly-accessible at the SIL,
+/// LLVM, and machine levels.
+///
+/// The most common causes of this are -enable-testing and versioned non-public
+/// declarations.
+static bool isInternalDeclEffectivelyPublic(const ValueDecl *VD) {
+  assert(VD->getFormalAccess() == Accessibility::Internal);
+
+  if (VD->getAttrs().hasAttribute<VersionedAttr>())
+    return true;
+
+  if (auto *fn = dyn_cast<FuncDecl>(VD))
+    if (auto *ASD = fn->getAccessorStorageDecl())
+      if (ASD->getAttrs().hasAttribute<VersionedAttr>())
+        return true;
+
+  if (VD->getModuleContext()->isTestingEnabled())
+    return true;
+
+  return false;
+}
+
+Accessibility ValueDecl::getEffectiveAccess() const {
+  Accessibility effectiveAccess = getFormalAccess();
+
+  // Handle @testable.
+  switch (effectiveAccess) {
+  case Accessibility::Public:
+    break;
+  case Accessibility::Internal:
+    if (isInternalDeclEffectivelyPublic(this))
+      effectiveAccess = Accessibility::Public;
+    break;
+  case Accessibility::Private:
+    break;
+  }
+
+  if (auto enclosingNominal = dyn_cast<NominalTypeDecl>(getDeclContext())) {
+    effectiveAccess = std::min(effectiveAccess,
+                               enclosingNominal->getEffectiveAccess());
+
+  } else if (auto enclosingExt = dyn_cast<ExtensionDecl>(getDeclContext())) {
+    // Just check the base type. If it's a constrained extension, Sema should
+    // have already enforced access more strictly.
+    if (auto extendedTy = enclosingExt->getExtendedType()) {
+      if (auto nominal = extendedTy->getAnyNominal()) {
+        effectiveAccess = std::min(effectiveAccess,
+                                   nominal->getEffectiveAccess());
+      }
+    }
+
+  } else if (getDeclContext()->isLocalContext()) {
+    effectiveAccess = Accessibility::Private;
+  }
+
+  return effectiveAccess;
+}
+
 
 Type TypeDecl::getDeclaredType() const {
   if (auto TAD = dyn_cast<TypeAliasDecl>(this)) {

lib/Sema/TypeCheckAttr.cpp

@@ -683,7 +683,6 @@ public:
   void visitRequiredAttr(RequiredAttr *attr);
   void visitRethrowsAttr(RethrowsAttr *attr);
 
-  bool visitAbstractAccessibilityAttr(AbstractAccessibilityAttr *attr);
   void visitAccessibilityAttr(AccessibilityAttr *attr);
   void visitSetterAccessibilityAttr(SetterAccessibilityAttr *attr);
 
@@ -1267,25 +1266,6 @@ void AttributeChecker::visitRethrowsAttr(RethrowsAttr *attr) {
   attr->setInvalid();
 }
 
-bool AttributeChecker::visitAbstractAccessibilityAttr(
-    AbstractAccessibilityAttr *attr) {
-  DeclContext *dc = D->getDeclContext();
-  if (auto nominal = dc->getAsNominalTypeOrNominalTypeExtensionContext()) {
-    Accessibility typeAccess = nominal->getFormalAccess();
-    if (attr->getAccess() > typeAccess) {
-      auto diag = TC.diagnose(attr->getLocation(),
-                              diag::access_control_member_more,
-                              attr->getAccess(),
-                              D->getDescriptiveKind(),
-                              typeAccess,
-                              nominal->getDescriptiveKind());
-      swift::fixItAccessibility(diag, cast<ValueDecl>(D), typeAccess);
-      return true;
-    }
-  }
-  return false;
-}
-
 void AttributeChecker::visitAccessibilityAttr(AccessibilityAttr *attr) {
   if (auto extension = dyn_cast<ExtensionDecl>(D)) {
     Type extendedTy = extension->getExtendedType();
@@ -1327,8 +1307,6 @@ void AttributeChecker::visitAccessibilityAttr(AccessibilityAttr *attr) {
       return;
     }
   }
-
-  visitAbstractAccessibilityAttr(attr);
 }
 
 void
@@ -1352,8 +1330,6 @@ AttributeChecker::visitSetterAccessibilityAttr(SetterAccessibilityAttr *attr) {
     attr->setInvalid();
     return;
   }
-
-  visitAbstractAccessibilityAttr(attr);
 }
 
 /// Check that the @_specialize type list has the correct number of entries.

test/SILGen/accessibility_warnings.swift

@@ -1,73 +1,98 @@
 // RUN: %target-parse-verify-swift
-// RUN: %target-swift-frontend -emit-silgen -o /dev/null %s
+// RUN: %target-swift-frontend -emit-silgen %s | FileCheck %s
 
 // This file tests that the AST produced after fixing accessibility warnings
 // is valid according to SILGen and the verifiers.
 
 public struct PublicStruct {
+  // CHECK-DAG: sil{{( \[.+\])*}} @_TFV22accessibility_warnings12PublicStructg9publicVarSi
   public var publicVar = 0
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings12PublicStructCfT_S0_
 }
 
 internal struct InternalStruct {
-  public var publicVar = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  public var publicVar = 0
 
-  public private(set) var publicVarPrivateSet = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  public private(set) var publicVarPrivateSet = 0
 
-  public public(set) var publicVarPublicSet = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}} {{10-22=}}
+  public public(set) var publicVarPublicSet = 0
 
-  public var publicVarGetOnly: Int { return 0 } // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStructg16publicVarGetOnlySi
+  public var publicVarGetOnly: Int { return 0 }
 
-  public var publicVarGetSet: Int { get { return 0 } set {} } // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStructg15publicVarGetSetSi
+  public var publicVarGetSet: Int { get { return 0 } set {} }
+
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStructCfT_S0_
 }
 
 private struct PrivateStruct {
-  public var publicVar = 0 // expected-warning {{declaring a public var for a private struct}} {{3-9=private}}
+  public var publicVar = 0
+  // CHECK-DAG: sil private @_TFV22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0213PrivateStructCfT_S0_
 }
 
 
 extension PublicStruct {
+  // CHECK-DAG: sil @_TFV22accessibility_warnings12PublicStructCfT1xSi_S0_
   public init(x: Int) { self.init() }
 
+  // CHECK-DAG: sil @_TFV22accessibility_warnings12PublicStructg18publicVarExtensionSi
   public var publicVarExtension: Int { get { return 0 } set {} }
 }
 
 extension InternalStruct {
-  public init(x: Int) { self.init() } // expected-warning {{declaring a public initializer for an internal struct}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStructCfT1xSi_S0_
+  public init(x: Int) { self.init() }
 
-  public var publicVarExtension: Int { get { return 0 } set {} } // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStructg18publicVarExtensionSi
+  public var publicVarExtension: Int { get { return 0 } set {} }
 }
 
 extension PrivateStruct {
-  public init(x: Int) { self.init() } // expected-warning {{declaring a public initializer for a private struct}} {{3-9=private}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0213PrivateStructCfT1xSi_S0_
+  public init(x: Int) { self.init() }
 
-  public var publicVarExtension: Int { get { return 0 } set {} } // expected-warning {{declaring a public var for a private struct}} {{3-9=private}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0213PrivateStructg18publicVarExtensionSi
+  public var publicVarExtension: Int { get { return 0 } set {} }
 }
 
 public extension PublicStruct {
+  // CHECK-DAG: sil @_TFV22accessibility_warnings12PublicStruct15extMemberPublicfT_T_
   public func extMemberPublic() {}
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings12PublicStructP33_5D2F2E026754A901C0FF90C404896D0213extImplPublicfT_T_
   private func extImplPublic() {}
 }
 internal extension PublicStruct {
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings12PublicStruct17extMemberInternalfT_T_
   public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-9=internal}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings12PublicStructP33_5D2F2E026754A901C0FF90C404896D0215extImplInternalfT_T_
   private func extImplInternal() {}
 }
 private extension PublicStruct {
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings12PublicStructP33_5D2F2E026754A901C0FF90C404896D0216extMemberPrivatefT_T_
   public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-9=private}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings12PublicStructP33_5D2F2E026754A901C0FF90C404896D0214extImplPrivatefT_T_
   private func extImplPrivate() {}
 }
 
 internal extension InternalStruct {
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStruct17extMemberInternalfT_T_
   public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-9=internal}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings14InternalStructP33_5D2F2E026754A901C0FF90C404896D0215extImplInternalfT_T_
   private func extImplInternal() {}
 }
 private extension InternalStruct {
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings14InternalStructP33_5D2F2E026754A901C0FF90C404896D0216extMemberPrivatefT_T_
   public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-9=private}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings14InternalStructP33_5D2F2E026754A901C0FF90C404896D0214extImplPrivatefT_T_
   private func extImplPrivate() {}
 }
 
 
 private extension PrivateStruct {
+  // CHECK-DAG: sil private @_TFV22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0213PrivateStruct16extMemberPrivatefT_T_
   public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-9=private}}
+  // CHECK-DAG: sil private @_TFV22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0213PrivateStruct14extImplPrivatefT_T_
   private func extImplPrivate() {}
 }
 
@@ -77,7 +102,10 @@ public protocol PublicReadOnlyOperations {
 }
 
 internal struct PrivateSettersForReadOnlyInternal : PublicReadOnlyOperations {
-  public private(set) var size = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden{{( \[.+\])*}} @_TFV22accessibility_warnings33PrivateSettersForReadOnlyInternalg4sizeSi
+  public private(set) var size = 0
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings33PrivateSettersForReadOnlyInternalg9subscriptFSiSi
+  // CHECK-DAG: sil private @_TFV22accessibility_warnings33PrivateSettersForReadOnlyInternals9subscriptFSiSi
   internal private(set) subscript (_: Int) -> Int { // no-warning
     get { return 42 }
     set {}
@@ -86,22 +114,32 @@ internal struct PrivateSettersForReadOnlyInternal : PublicReadOnlyOperations {
 
 
 public class PublicClass {
+  // CHECK-DAG: sil{{( \[.+\])*}} @_TFC22accessibility_warnings11PublicClassg9publicVarSi
   public var publicVar = 0
+  // CHECK-DAG: sil hidden @_TFC22accessibility_warnings11PublicClasscfT_S0_
 }
 
 internal class InternalClass {
-  public var publicVar = 0 // expected-warning {{declaring a public var for an internal class}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden{{( \[.+\])*}} @_TFC22accessibility_warnings13InternalClassg9publicVarSi
+  public var publicVar = 0
 
-  public private(set) var publicVarPrivateSet = 0 // expected-warning {{declaring a public var for an internal class}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden [transparent] @_TFC22accessibility_warnings13InternalClassg19publicVarPrivateSetSi
+  public private(set) var publicVarPrivateSet = 0
 
-  public public(set) var publicVarPublicSet = 0 // expected-warning {{declaring a public var for an internal class}} {{3-9=internal}}
+  public public(set) var publicVarPublicSet = 0
 
-  public var publicVarGetOnly: Int { return 0 } // expected-warning {{declaring a public var for an internal class}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden @_TFC22accessibility_warnings13InternalClassg16publicVarGetOnlySi
+  public var publicVarGetOnly: Int { return 0 }
 
-  public var publicVarGetSet: Int { get { return 0 } set {} } // expected-warning {{declaring a public var for an internal class}} {{3-9=internal}}
+  // CHECK-DAG: sil hidden @_TFC22accessibility_warnings13InternalClassg15publicVarGetSetSi
+  public var publicVarGetSet: Int { get { return 0 } set {} }
+
+  // CHECK-DAG: sil hidden @_TFC22accessibility_warnings13InternalClasscfT_S0_
 }
 
 private class PrivateClass {
-  public var publicVar = 0 // expected-warning {{declaring a public var for a private class}} {{3-9=private}}
+  // CHECK-DAG: sil private{{( \[.+\])*}} @_TFC22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0212PrivateClassg9publicVarSi
+  public var publicVar = 0
+  // CHECK-DAG: sil private @_TFC22accessibility_warningsP33_5D2F2E026754A901C0FF90C404896D0212PrivateClasscfT_S0_
 }
 

test/Sema/accessibility.swift

@@ -34,7 +34,7 @@ internal struct InternalStruct: PublicProto, InternalProto, FilePrivateProto, Pr
   private func filePrivateReq() {}
   private func privateReq() {}
 
-  public var publicVar = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  public var publicVar = 0
 }
 
 // expected-note@+1 * {{type declared here}}
@@ -44,7 +44,7 @@ fileprivate struct FilePrivateStruct: PublicProto, InternalProto, FilePrivatePro
   private func filePrivateReq() {}
   private func privateReq() {}
 
-  public var publicVar = 0 // expected-warning {{declaring a public var for a private struct}} {{3-9=private}}
+  public var publicVar = 0
 }
 
 // expected-note@+1 * {{type declared here}}
@@ -54,7 +54,7 @@ private struct PrivateStruct: PublicProto, InternalProto, FilePrivateProto, Priv
   private func filePrivateReq() {}
   private func privateReq() {}
 
-  public var publicVar = 0 // expected-warning {{declaring a public var for a private struct}} {{3-9=private}}
+  public var publicVar = 0
 }
 
 extension PublicStruct {
@@ -62,15 +62,15 @@ extension PublicStruct {
 }
 
 extension InternalStruct {
-  public init(x: Int) { self.init() } // expected-warning {{declaring a public initializer for an internal struct}} {{3-9=internal}}
+  public init(x: Int) { self.init() }
 }
 
 extension FilePrivateStruct {
-  public init(x: Int) { self.init() } // expected-warning {{declaring a public initializer for a private struct}} {{3-9=private}}
+  public init(x: Int) { self.init() }
 }
 
 extension PrivateStruct {
-  public init(x: Int) { self.init() } // expected-warning {{declaring a public initializer for a private struct}} {{3-9=private}}
+  public init(x: Int) { self.init() }
 }
 
 public extension PublicStruct {
@@ -90,7 +90,7 @@ fileprivate extension PublicStruct {
   private func extImplFilePrivate() {}
 }
 public extension InternalStruct { // expected-error {{extension of internal struct cannot be declared public}} {{1-8=}}
-  public func extMemberPublic() {} // expected-warning {{declaring a public instance method for an internal struct}} {{3-9=internal}}
+  public func extMemberPublic() {}
   private func extImplPublic() {}
 }
 internal extension InternalStruct {
@@ -106,7 +106,7 @@ private extension InternalStruct {
   private func extImplPrivate() {}
 }
 public extension FilePrivateStruct { // expected-error {{extension of private struct cannot be declared public}} {{1-8=}}
-  public func extMemberPublic() {} // expected-warning {{declaring a public instance method for a private struct}} {{3-9=private}}
+  public func extMemberPublic() {}
   private func extImplPublic() {}
 }
 internal extension FilePrivateStruct { // expected-error {{extension of private struct cannot be declared internal}} {{1-10=}}
@@ -122,7 +122,7 @@ private extension FilePrivateStruct {
   private func extImplPrivate() {}
 }
 public extension PrivateStruct { // expected-error {{extension of private struct cannot be declared public}} {{1-8=}}
-  public func extMemberPublic() {} // expected-warning {{declaring a public instance method for a private struct}} {{3-9=private}}
+  public func extMemberPublic() {}
   private func extImplPublic() {}
 }
 internal extension PrivateStruct { // expected-error {{extension of private struct cannot be declared internal}} {{1-10=}}
@@ -422,7 +422,7 @@ public protocol PublicReadOnlyOperations {
 }
 
 internal struct PrivateSettersForReadOnlyInternal : PublicReadOnlyOperations {
-  public private(set) var size = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  public private(set) var size = 0
   internal private(set) subscript (_: Int) -> Int { // no-warning
     get { return 42 }
     set {}

test/attr/accessibility.swift

@@ -186,7 +186,7 @@ extension PublicProto where Assoc == InternalStruct {
   public func foo() {} // expected-error {{cannot declare a public instance method in an extension with internal requirements}} {{3-9=internal}}
 }
 extension InternalProto {
-  public func foo() {} // expected-warning {{declaring a public instance method for an internal protocol}} {{3-9=internal}}
+  public func foo() {} // no effect, but no warning
 }
 extension InternalProto where Assoc == PublicStruct {
   public func foo() {} // expected-error {{cannot declare a public instance method in an extension with internal requirements}} {{3-9=internal}}