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/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..534c209b9e2aa 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,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() 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..08a01c5dde85e --- /dev/null +++ b/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 + } + } +}