[cxx-interop] Make imported enum members inherit the access of their parent (#84522)

This patch relaxes the access check for members of imported inherited
nested enums. Without it, those members become impossible to inherit,
which largely defeats the purpose of importing them in the first place.

rdar://160803362

Author: j-hui

lib/AST/DeclContext.cpp

@@ -1499,14 +1499,32 @@ bool AccessScope::allowsPrivateAccess(const DeclContext *useDC, const DeclContex
     auto clangDecl = sourceNTD->getDecl()->getClangDecl();
 
     if (isa_and_nonnull<clang::EnumDecl>(clangDecl)) {
-      // Consider:  class C { private: enum class E { M }; };
-      // If sourceDC is a C++ enum (e.g, E), then we are looking at one of its
-      // members (e.g., E.M). If this is the case, then we should consider
-      // the SWIFT_PRIVATE_FILEID of its enclosing class decl (e.g., C), if any.
-      // Doing so allows the nested private enum's members to inherit the access
-      // of the nested enum type itself.
-      clangDecl = dyn_cast<clang::CXXRecordDecl>(clangDecl->getDeclContext());
-      sourceDC = sourceNTD->getDeclContext();
+      // C/C++ enums are an odd case for access checking because they can only
+      // contain variants as members, and those variants cannot be assigned
+      // access specifiers. Instead, those variants should simply inherit the
+      // access of their parent enum, if any. For instance:
+      //
+      //   class Base { protected: enum class E { M }; };
+      //   class Derived : public Base {};
+      //
+      // In C++, E::M should be accessible within Base and Derived; we should
+      // follow suit in Swift.
+      //
+      // When we check the access of something like E.M, clangDecl will point to
+      // enum class E (the DeclContext of M), and if we've gotten this far, we
+      // can infer that E was accessible in useDC, so its members should be too.
+      //
+      // This is technically unsound (i.e., encapsulation-breaking), because it
+      // is possible to extend imported Clang enums private in Swift extensions.
+      // With this hack, those private members would be accessible everywhere
+      // even though they shouldn't. But right here, when we're checking the E
+      // of E.M, there's no way to tell whether the M is something we imported
+      // from Clang (which we should allow) versus something we defined in Swift
+      // (which we should disallow), without over-complicating the access check.
+      // To limit the encapsulate the breakage, we do an additional check to
+      // ensure we're checking an imported enum that is *nested* in a Clang
+      // record (which is the only way this enum can be imported as private).
+      return isa_and_nonnull<clang::CXXRecordDecl>(clangDecl->getDeclContext());
     }
 
     if (!isa_and_nonnull<clang::CXXRecordDecl>(clangDecl))

test/Interop/Cxx/class/access/access-inversion-executable.swift

@@ -28,7 +28,7 @@ AccessInversionTestSuite.test("usePrivateRec") {
 }
 
 AccessInversionTestSuite.test("usePrivateEnum") {
-    let e = Leaky.AliasToPrivateEnum(rawValue: 0)!
+    let e = Leaky.AliasToPrivateEnum(rawValue: 0)
     expectEqual(e.rawValue, 0)
 }
 

test/Interop/Cxx/class/access/access-inversion-typechecker.swift

@@ -208,10 +208,10 @@ func usePrivateEnum(a: inout Leaky.AliasToPrivateEnum) -> Leaky.AliasToPrivateEn
     // Constructing and reading PrivateEnum
     //
 
-    // TODO: nested enum members are not being imported (#54905)
-    // let _ = Leaky.privateEnumMember
-    let rv0 = Leaky.AliasToPrivateEnum(rawValue: 0)!
-    let _ = Leaky.PrivateEnum(rawValue: 0)!
+    let _ = Leaky.privateEnumMember // FIXME: nested enum members are not being imported (#54905)
+    // expected-error@-1 {{type 'Leaky' has no member 'privateEnumMember'}}
+    let rv0 = Leaky.AliasToPrivateEnum(rawValue: 0)
+    let _ = Leaky.PrivateEnum(rawValue: 0)
     // expected-error@-1 {{'PrivateEnum' is inaccessible due to 'private' protection level}}
 
     let _ = rv0.rawValue
@@ -276,16 +276,13 @@ func usePrivateEnumClass(a: inout Leaky.AliasToPrivateEnumClass) -> Leaky.AliasT
     // Constructing and reading PrivateEnumClass
     //
 
-    // NOTE: private enum class members are not accessible even if we can access
-    // instances of the private enum class via
+    // NOTE: private enum class members are accessible if we can access
+    // their parent enum class decl
 
     let _ = Leaky.AliasToPrivateEnumClass.privateEnumClassMember
-    // expected-error@-1 {{'privateEnumClassMember' is inaccessible due to 'private' protection level}}
     let _ = Leaky.PrivateEnumClass.privateEnumClassMember
     // expected-error@-1 {{'PrivateEnumClass' is inaccessible due to 'private' protection level}}
-    let _: Leaky.AliasToPrivateEnum = .privateEnumClassMember
-    // expected-error@-1 {{type 'Leaky.AliasToPrivateEnum' (aka 'Leaky.PrivateEnum') has no member 'privateEnumClassMember'}}
-    // TODO: ^this is not really the right error message
+    let _: Leaky.AliasToPrivateEnumClass = .privateEnumClassMember
 
     let rv0 = Leaky.AliasToPrivateEnumClass(rawValue: 0)!
     let _ = Leaky.PrivateEnumClass(rawValue: 0)!
@@ -298,7 +295,6 @@ func usePrivateEnumClass(a: inout Leaky.AliasToPrivateEnumClass) -> Leaky.AliasT
 
     switch rv0 {
     case .privateEnumClassMember:
-    // expected-error@-1 {{'privateEnumClassMember' is inaccessible due to 'private' protection level}}
       doSomething()
     default:
       doSomething()

test/Interop/Cxx/class/access/non-public-nested-enum-executable.swift

@@ -0,0 +1,111 @@
+// RUN: split-file %s %t
+
+// RUN: %target-build-swift -module-name main %t/base.swift -I %t/Inputs -o %t/base -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
+// RUN: %target-codesign %t/base
+// RUN: %target-run %t/base
+
+// RUN: %target-build-swift -module-name main %t/not-base.swift -I %t/Inputs -o %t/not-base -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
+// RUN: %target-codesign %t/not-base
+// RUN: %target-run %t/not-base
+
+// RUN: %target-build-swift -module-name main %t/derived.swift -I %t/Inputs -o %t/derived -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
+// RUN: %target-codesign %t/derived
+// RUN: %target-run %t/derived
+
+// RUN: %target-build-swift -module-name main %t/not-derived.swift -I %t/Inputs -o %t/not-derived -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
+// RUN: %target-codesign %t/not-derived
+// RUN: %target-run %t/not-derived
+
+// REQUIRES: executable_test
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
+
+//--- Inputs/module.modulemap
+module CxxModule {
+    requires cplusplus
+    header "cxx-header.h"
+}
+
+//--- Inputs/cxx-header.h
+#pragma once
+
+class __attribute__((__swift_attr__("private_fileid:main/base.swift")))
+Base {
+protected:
+  enum class Enum { Foo, Bar };
+public:
+  Enum makeEnum(void) const { return Enum::Bar; }
+};
+
+class __attribute__((__swift_attr__("private_fileid:main/derived.swift")))
+Derived : public Base {};
+
+//--- base.swift
+import StdlibUnittest
+import CxxModule
+
+var Suite = TestSuite("BlessedForBase")
+extension Base {
+  static func makeEnums() {
+    let e1: Enum = Enum.Bar
+    let e2: Enum = Enum.Foo
+    expectEqual(e1, Enum.Bar)
+    expectNotEqual(e1, e2)
+    switch e2 {
+    case .Bar:
+      expectTrue(false, "foo is bar")
+    case .Foo:
+      expectTrue(true, "foo is foo")
+    }
+  }
+}
+Suite.test("Use private nested enum in base class") { Base.makeEnums() }
+runAllTests()
+
+//--- not-base.swift
+import StdlibUnittest
+import CxxModule
+
+var Suite = TestSuite("NotBlessedForBase")
+Suite.test("Use private nested enum in base class") {
+  let b = Base()
+  let e1 = b.makeEnum()
+  let e2 = b.makeEnum()
+  expectEqual(e1, e2)
+}
+runAllTests()
+
+
+//--- derived.swift
+import StdlibUnittest
+import CxxModule
+
+var Suite = TestSuite("BlessedForDerived")
+extension Derived {
+  static func makeEnums() {
+    let e1: Enum = Enum.Bar
+    let e2: Enum = Enum.Foo
+    expectEqual(e1, Enum.Bar)
+    expectNotEqual(e1, e2)
+    switch e2 {
+    case .Bar:
+      expectTrue(false, "foo is bar")
+    case .Foo:
+      expectTrue(true, "foo is foo")
+    }
+  }
+}
+Suite.test("Use private nested enum inherited from base class") { Derived.makeEnums() }
+runAllTests()
+
+//--- not-derived.swift
+import StdlibUnittest
+import CxxModule
+
+var Suite = TestSuite("NotBlessedForDerive")
+Suite.test("Use private nested enum inherited from base class") {
+  let d = Derived()
+  let e1 = d.makeEnum()
+  let e2 = d.makeEnum()
+  expectEqual(e1, e2)
+}
+runAllTests()

test/Interop/Cxx/class/access/non-public-nested-enum-module-interface.swift

@@ -0,0 +1,43 @@
+// RUN: split-file %s %t
+
+// RUN: %target-swift-ide-test -print-module -module-to-print=CxxModule -print-access -I %t/Inputs -source-filename=x -enable-experimental-cxx-interop -enable-experimental-feature ImportNonPublicCxxMembers | %FileCheck %s
+
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
+
+//--- Inputs/module.modulemap
+module CxxModule {
+    requires cplusplus
+    header "cxx-header.h"
+}
+
+//--- Inputs/cxx-header.h
+#pragma once
+
+class __attribute__((__swift_attr__("private_fileid:main/base.swift")))
+Base {
+protected:
+  enum class Enum { Foo, Bar };
+public:
+  Enum makeEnum(void) const { return Enum::Bar; }
+};
+
+class __attribute__((__swift_attr__("private_fileid:main/derived.swift")))
+Derived : public Base {};
+
+// CHECK:      public struct Base {
+// CHECK-NEXT:   public init()
+// CHECK-NEXT:   private enum Enum : [[ENUM_UNDERLYING_TYPE:Int32|UInt32]] {
+// CHECK-NEXT:     private init?(rawValue: [[ENUM_UNDERLYING_TYPE]])
+// CHECK-NEXT:     private var rawValue: [[ENUM_UNDERLYING_TYPE]] { get }
+// CHECK-NEXT:     private typealias RawValue = [[ENUM_UNDERLYING_TYPE]]
+// CHECK-NEXT:     case Foo
+// CHECK-NEXT:     case Bar
+// CHECK-NEXT:   }
+// CHECK-NEXT:   public func makeEnum() -> Base.Enum
+// CHECK-NEXT: }
+
+// CHECK-NEXT: public struct Derived {
+// CHECK-NEXT:   public init()
+// CHECK-NEXT:   private typealias Enum = Base.Enum
+// CHECK-NEXT:   public func makeEnum() -> Base.Enum
+// CHECK-NEXT: }

test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift

@@ -0,0 +1,116 @@
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/base.swift
+// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/derived.swift
+
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
+
+//--- Inputs/module.modulemap
+module CxxModule {
+    requires cplusplus
+    header "cxx-header.h"
+}
+
+//--- Inputs/cxx-header.h
+#pragma once
+
+class __attribute__((__swift_attr__("private_fileid:main/base.swift")))
+Base {
+protected:
+  enum class Enum { Foo, Bar };
+public:
+  Enum makeEnum(void) const { return Enum::Bar; }
+};
+
+class __attribute__((__swift_attr__("private_fileid:main/derived.swift")))
+Derived : public Base {};
+
+//--- base.swift
+import CxxModule
+
+extension Base {
+  private static func inside(e: Enum) {
+    let _: Enum = Enum.Bar
+    let _: Base.Enum = Base.Enum.Bar
+    let _: Enum = Base.Enum.Foo
+    let _: Base.Enum = Enum.Foo
+
+    switch e {
+      case .Bar: return
+      case .Foo: return
+    }
+  }
+}
+
+func switches() {
+  let b = Base()
+  let d = Derived()
+  switch b.makeEnum() {
+    default: return   // this is OK
+  }
+  switch d.makeEnum() {
+    default: return   // this is OK
+  }
+  switch b.makeEnum() {
+    case Base.Enum.Bar: return // expected-error {{'Enum' is inaccessible due to 'private' protection level}}
+    case Base.Enum.Foo: return // expected-error {{'Enum' is inaccessible due to 'private' protection level}}
+  }
+  switch b.makeEnum() {
+    case .Bar: return // OK as long as the user does not refer to Enum itself
+    case .Foo: return // (NOTE: arguably this should not be allowed)
+  }
+  switch d.makeEnum() {
+    case Derived.Enum.Bar: return // expected-error {{'Enum' is inaccessible due to 'private' protection level}}
+    case Derived.Enum.Foo: return // expected-error {{'Enum' is inaccessible due to 'private' protection level}}
+  }
+  switch d.makeEnum() {
+    case .Bar: return // OK as long as the user does not refer to Enum itself
+    case .Foo: return // (NOTE: arguably this should not be allowed)
+  }
+}
+
+func outside() {
+  let b = Base()
+  let d = Derived()
+  let _ = b.makeEnum()  // This is OK as long as we don't name the type
+  let _ = d.makeEnum()  // This is OK as long as we don't name the type
+
+  let _: Derived.Enum = b.makeEnum() // expected-error {{'Enum' is inaccessible due to 'private' protection level}}
+  let _: Derived.Enum = d.makeEnum() // expected-error {{'Enum' is inaccessible due to 'private' protection level}}
+
+  // The types in the base and derived types should be considered the same type
+  var x = b.makeEnum()
+  x = d.makeEnum()
+  var y = d.makeEnum()
+  y = b.makeEnum()
+}
+
+//--- derived.swift
+import CxxModule
+
+extension Base {
+  private static func inside(e: Enum) { // expected-error    {{'Enum' is inaccessible due to 'private' protection level}}
+    let _: Enum = Enum.Bar              // expected-error    {{'Enum' is inaccessible due to 'private' protection level}}
+                                        // expected-error@-1 {{'Enum' is inaccessible due to 'private' protection level}}
+  }
+}
+
+extension Derived {
+  private static func inside(e: Enum) {
+    let b = Base()
+    let _: Enum = Enum.Bar
+    let _: Derived.Enum = Derived.Enum.Bar
+    let _: Enum = b.makeEnum()
+
+    // It would be nice to make these work but they do not
+    let _: Base.Enum = Base.Enum.Bar // expected-error    {{'Enum' is inaccessible due to 'private' protection level}}
+                                     // expected-error@-1 {{'Enum' is inaccessible due to 'private' protection level}}
+    let _: Enum = Base.Enum.Foo      // expected-error    {{'Enum' is inaccessible due to 'private' protection level}}
+    let _: Base.Enum = Enum.Foo      // expected-error    {{'Enum' is inaccessible due to 'private' protection level}}
+
+    switch e {
+      case .Bar: return
+      case .Foo: return
+    }
+  }
+}