From 508c63c40cd1e3f4a41d706053621f5600c6669a Mon Sep 17 00:00:00 2001 From: John Hui Date: Thu, 25 Sep 2025 16:49:00 -0700 Subject: [PATCH 1/4] [cxx-interop] Make imported enum members inherit the access of their parent 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 --- lib/AST/DeclContext.cpp | 34 ++++-- .../non-public-nested-enum-executable.swift | 111 ++++++++++++++++++ ...-public-nested-enum-module-interface.swift | 43 +++++++ .../non-public-nested-enum-typecheck.swift | 107 +++++++++++++++++ 4 files changed, 287 insertions(+), 8 deletions(-) create mode 100644 test/Interop/Cxx/class/access/non-public-nested-enum-executable.swift create mode 100644 test/Interop/Cxx/class/access/non-public-nested-enum-module-interface.swift create mode 100644 test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp index dc49d15e24e8b..6b0b3a9257f0e 100644 --- a/lib/AST/DeclContext.cpp +++ b/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(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(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(clangDecl->getDeclContext()); } if (!isa_and_nonnull(clangDecl)) diff --git a/test/Interop/Cxx/class/access/non-public-nested-enum-executable.swift b/test/Interop/Cxx/class/access/non-public-nested-enum-executable.swift new file mode 100644 index 0000000000000..0cfcedfd3b627 --- /dev/null +++ b/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() diff --git a/test/Interop/Cxx/class/access/non-public-nested-enum-module-interface.swift b/test/Interop/Cxx/class/access/non-public-nested-enum-module-interface.swift new file mode 100644 index 0000000000000..11cb0ee1bf902 --- /dev/null +++ b/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: } diff --git a/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift b/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift new file mode 100644 index 0000000000000..e472e2e3db039 --- /dev/null +++ b/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift @@ -0,0 +1,107 @@ +// RUN: split-file %s %t + +// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs -cxx-interoperability-mode=default -module-name main %t/base.swift +// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs -cxx-interoperability-mode=default -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 .Bar: return // somehow this is OK + case .Foo: return // somehow this is OK + } + switch d.makeEnum() { + case .Bar: return // somehow this is OK + case .Foo: return // somehow this is OK + } +} + +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 _: 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 + } + } +} From 5650da392afc3526149fc80a579abb184799ba3c Mon Sep 17 00:00:00 2001 From: John Hui Date: Fri, 26 Sep 2025 00:15:49 -0700 Subject: [PATCH 2/4] Fix tests --- .../access/access-inversion-executable.swift | 2 +- .../access/access-inversion-typechecker.swift | 16 +++++++--------- .../non-public-nested-enum-typecheck.swift | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/Interop/Cxx/class/access/access-inversion-executable.swift b/test/Interop/Cxx/class/access/access-inversion-executable.swift index 175afde0997bc..9d686e1e32b16 100644 --- a/test/Interop/Cxx/class/access/access-inversion-executable.swift +++ b/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) } diff --git a/test/Interop/Cxx/class/access/access-inversion-typechecker.swift b/test/Interop/Cxx/class/access/access-inversion-typechecker.swift index 4159abde52d80..e5ff1e50ec804 100644 --- a/test/Interop/Cxx/class/access/access-inversion-typechecker.swift +++ b/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,15 @@ 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 + // FIXME: ^this should be accessible let rv0 = Leaky.AliasToPrivateEnumClass(rawValue: 0)! let _ = Leaky.PrivateEnumClass(rawValue: 0)! @@ -298,7 +297,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() diff --git a/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift b/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift index e472e2e3db039..6ac7110ce3b6a 100644 --- a/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift +++ b/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift @@ -1,7 +1,7 @@ // RUN: split-file %s %t -// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs -cxx-interoperability-mode=default -module-name main %t/base.swift -// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs -cxx-interoperability-mode=default -module-name main %t/derived.swift +// 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 From 4303e02c9848f68b0e346502f6d8d03d61c18c42 Mon Sep 17 00:00:00 2001 From: John Hui Date: Fri, 26 Sep 2025 13:18:58 -0700 Subject: [PATCH 3/4] Clarify test --- .../non-public-nested-enum-typecheck.swift | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift b/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift index 6ac7110ce3b6a..08a01c5dde85e 100644 --- a/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift +++ b/test/Interop/Cxx/class/access/non-public-nested-enum-typecheck.swift @@ -52,12 +52,20 @@ func switches() { default: return // this is OK } switch b.makeEnum() { - case .Bar: return // somehow this is OK - case .Foo: return // somehow this is OK + 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 // somehow this is OK - case .Foo: return // somehow this is OK + 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) } } @@ -91,6 +99,7 @@ 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 From dfe7c4794cbe0f72474e7152e133e3e3825717f6 Mon Sep 17 00:00:00 2001 From: John Hui Date: Fri, 26 Sep 2025 13:20:45 -0700 Subject: [PATCH 4/4] Fix previous typo in test --- .../Cxx/class/access/access-inversion-typechecker.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/Interop/Cxx/class/access/access-inversion-typechecker.swift b/test/Interop/Cxx/class/access/access-inversion-typechecker.swift index e5ff1e50ec804..534c209b9e2aa 100644 --- a/test/Interop/Cxx/class/access/access-inversion-typechecker.swift +++ b/test/Interop/Cxx/class/access/access-inversion-typechecker.swift @@ -282,9 +282,7 @@ func usePrivateEnumClass(a: inout Leaky.AliasToPrivateEnumClass) -> Leaky.AliasT let _ = Leaky.AliasToPrivateEnumClass.privateEnumClassMember 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'}} - // FIXME: ^this should be accessible + let _: Leaky.AliasToPrivateEnumClass = .privateEnumClassMember let rv0 = Leaky.AliasToPrivateEnumClass(rawValue: 0)! let _ = Leaky.PrivateEnumClass(rawValue: 0)!