SE-0025: Allow public members inside internal types.

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

@@ -941,10 +941,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();
@@ -1804,6 +1787,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 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   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
 
-  public private(set) var publicVarPrivateSet = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}}
+  public public(set) var publicVarPublicSet = 0
 
-  public public(set) var publicVarPublicSet = 0 // expected-warning {{declaring a public var for an internal struct}} {{3-9=internal}} {{10-22=}}
+  // CHECK-DAG: sil hidden @_TFV22accessibility_warnings14InternalStructg16publicVarGetOnlySi
+  public var publicVarGetOnly: Int { return 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_warnings14InternalStructg15publicVarGetSetSi
+  public var publicVarGetSet: Int { get { return 0 } set {} }
 
-  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_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_
 }