Use 'fileprivate' for synthesized members of 'private' decls (#15980)

Since 'private' means "limit to the enclosing scope (and extensions
thereof)", putting it on a member means that the member can't be
accessed everywhere the type might show up. That's normally a good
thing, but it's not the desired effect for synthesized members used
for derived conformances, and when it comes to class initializers this
actually violates AST invariants.

rdar://problem/39478298

Author: jrose-apple

include/swift/AST/Decl.h

@@ -2304,8 +2304,14 @@ class ValueDecl : public Decl {
                        bool treatUsableFromInlineAsPublic = false) const;
 
 
-  /// Copy the formal access level and @usableFromInline attribute from source.
-  void copyFormalAccessFrom(ValueDecl *source);
+  /// Copy the formal access level and @usableFromInline attribute from
+  /// \p source.
+  ///
+  /// If \p sourceIsParentContext is true, an access level of \c private will
+  /// be copied as \c fileprivate, to ensure that this declaration will be
+  /// available everywhere \p source is.
+  void copyFormalAccessFrom(const ValueDecl *source,
+                            bool sourceIsParentContext = false);
 
   /// Returns the access level that actually controls how a declaration should
   /// be emitted and may be used.

lib/AST/Decl.cpp

@@ -2259,9 +2259,24 @@ ValueDecl::getFormalAccessScope(const DeclContext *useDC,
   llvm_unreachable("unknown access level");
 }
 
-void ValueDecl::copyFormalAccessFrom(ValueDecl *source) {
+void ValueDecl::copyFormalAccessFrom(const ValueDecl *source,
+                                     bool sourceIsParentContext) {
   if (!hasAccess()) {
-    setAccess(source->getFormalAccess());
+    AccessLevel access = source->getFormalAccess();
+
+    // To make something have the same access as a 'private' parent, it has to
+    // be 'fileprivate' or greater.
+    if (sourceIsParentContext && access == AccessLevel::Private)
+      access = AccessLevel::FilePrivate;
+
+    // Only certain declarations can be 'open'.
+    if (access == AccessLevel::Open && !isPotentiallyOverridable()) {
+      assert(!isa<ClassDecl>(this) &&
+             "copying 'open' onto a class has complications");
+      access = AccessLevel::Public;
+    }
+
+    setAccess(access);
   }
 
   // Inherit the @usableFromInline attribute.
@@ -2816,7 +2831,7 @@ void ClassDecl::addImplicitDestructor() {
   setHasDestructor();
 
   // Propagate access control and versioned-ness.
-  DD->copyFormalAccessFrom(this);
+  DD->copyFormalAccessFrom(this, /*sourceIsParentContext*/true);
 
   // Wire up generic environment of DD.
   DD->setGenericEnvironment(getGenericEnvironmentOfContext());

lib/Sema/DerivedConformanceCodable.cpp

@@ -767,7 +767,7 @@ static FuncDecl *deriveEncodable_encode(TypeChecker &tc, Decl *parentDecl,
 
   encodeDecl->setInterfaceType(interfaceType);
   encodeDecl->setValidationStarted();
-  encodeDecl->setAccess(target->getFormalAccess());
+  encodeDecl->copyFormalAccessFrom(target, /*sourceIsParentContext*/true);
 
   tc.Context.addSynthesizedDecl(encodeDecl);
 
@@ -1106,7 +1106,7 @@ static ValueDecl *deriveDecodable_init(TypeChecker &tc, Decl *parentDecl,
   initDecl->setInterfaceType(interfaceType);
   initDecl->setValidationStarted();
   initDecl->setInitializerInterfaceType(initializerType);
-  initDecl->setAccess(target->getFormalAccess());
+  initDecl->copyFormalAccessFrom(target, /*sourceIsParentContext*/true);
 
   tc.Context.addSynthesizedDecl(initDecl);
 

lib/Sema/DerivedConformanceEquatableHashable.cpp

@@ -655,7 +655,7 @@ deriveEquatable_eq(TypeChecker &tc, Decl *parentDecl, NominalTypeDecl *typeDecl,
                                     FunctionType::ExtInfo());
   }
   eqDecl->setInterfaceType(interfaceTy);
-  eqDecl->copyFormalAccessFrom(typeDecl);
+  eqDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);
   eqDecl->setValidationStarted();
 
   tc.Context.addSynthesizedDecl(eqDecl);
@@ -1044,15 +1044,15 @@ deriveHashable_hashValue(TypeChecker &tc, Decl *parentDecl,
 
   getterDecl->setInterfaceType(interfaceType);
   getterDecl->setValidationStarted();
-  getterDecl->copyFormalAccessFrom(typeDecl);
+  getterDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);
 
   // Finish creating the property.
   hashValueDecl->setImplicit();
   hashValueDecl->setInterfaceType(intType);
   hashValueDecl->setValidationStarted();
   hashValueDecl->makeComputed(SourceLoc(), getterDecl,
                               nullptr, nullptr, SourceLoc());
-  hashValueDecl->copyFormalAccessFrom(typeDecl);
+  hashValueDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);
 
   Pattern *hashValuePat = new (C) NamedPattern(hashValueDecl, /*implicit*/true);
   hashValuePat->setType(intType);

lib/Sema/DerivedConformanceRawRepresentable.cpp

@@ -340,7 +340,7 @@ static ConstructorDecl *deriveRawRepresentable_init(TypeChecker &tc,
   }
   initDecl->setInterfaceType(allocIfaceType);
   initDecl->setInitializerInterfaceType(initIfaceType);
-  initDecl->copyFormalAccessFrom(enumDecl);
+  initDecl->copyFormalAccessFrom(enumDecl, /*sourceIsParentContext*/true);
   initDecl->setValidationStarted();
 
   tc.Context.addSynthesizedDecl(initDecl);

lib/Sema/DerivedConformances.cpp

@@ -303,7 +303,7 @@ DerivedConformance::declareDerivedProperty(TypeChecker &tc, Decl *parentDecl,
                                       /*IsCaptureList*/false, SourceLoc(), name,
                                       propertyContextType, parentDC);
   propDecl->setImplicit();
-  propDecl->copyFormalAccessFrom(typeDecl);
+  propDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);
   propDecl->setInterfaceType(propertyInterfaceType);
   propDecl->setValidationStarted();
 

lib/Sema/TypeCheckDecl.cpp

@@ -7605,7 +7605,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
            && "Decl parsing must prevent destructors outside of types!");
 
     checkDeclAttributesEarly(DD);
-    DD->copyFormalAccessFrom(enclosingClass);
+    DD->copyFormalAccessFrom(enclosingClass, /*sourceIsParentContext*/true);
 
     configureImplicitSelf(*this, DD);
 

test/Sema/struct_equatable_hashable_access.swift

@@ -0,0 +1,10 @@
+// RUN: %target-swift-frontend -print-ast %s 2>&1 | %FileCheck %s
+
+// Check that synthesized members show up as 'fileprivate', not 'private.
+
+// CHECK-LABEL: private struct PrivateConformer : Hashable {
+private struct PrivateConformer: Hashable {
+  var value: Int
+  // CHECK-DAG: fileprivate var hashValue: Int { get }
+  // CHECK-DAG: @_implements(Equatable, ==(_:_:)) fileprivate static func __derived_struct_equals
+}

test/decl/protocol/special/coding/class_codable_inheritance.swift

@@ -50,3 +50,30 @@ let _ = SimpleChildClass.encode(to:)
 // The synthesized CodingKeys type should not be accessible from outside the
 // class.
 let _ = SimpleChildClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}
+
+// Check access level issues around 'private'.
+private class PrivateClass: Codable {
+  var x: Int = 1
+}
+
+private class PrivateClassChild: PrivateClass {}
+
+_ = PrivateClass.init(from:)
+_ = PrivateClass.encode(to:)
+_ = PrivateClassChild.init(from:)
+_ = PrivateClassChild.encode(to:)
+
+_ = PrivateClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}
+
+// Check access level issues around 'open'.
+open class OpenClass: Codable {
+  var x: Int = 1
+}
+open class OpenClassChild: OpenClass {}
+
+_ = OpenClass.init(from:)
+_ = OpenClass.encode(to:)
+_ = OpenClassChild.init(from:)
+_ = OpenClassChild.encode(to:)
+
+_ = OpenClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}