[cxx-interop] Fix access check for nested private C++ enums (swiftlang#80366)

This patch fixes the access check for nested private C++ enums to look for the SWIFT_PRIVATE_FILEID of the enclosing C++ class, if any. Previously, the check was looking at for SWIFT_PRIVATE_FILEID on the enum decl itself (which is meaningless); that prevented nested private enum members from being accessible in Swift.

This patch also specializes the type signature of getPrivateFileIDAttrs to clarify the fact that SWIFT_PRIVATE_FILEID is not a meaningful annotation on anything other than CXXRecordDecl, because that is the only kind of decl that can assign access specifiers to its members.

rdar://148081340

Author: j-hui

include/swift/ClangImporter/ClangImporter.h

@@ -763,7 +763,7 @@ AccessLevel convertClangAccess(clang::AccessSpecifier access);
 /// The returned fileIDs may not be of a valid format (e.g., missing a '/'),
 /// and should be parsed using swift::SourceFile::FileIDStr::parse().
 SmallVector<std::pair<StringRef, clang::SourceLocation>, 1>
-getPrivateFileIDAttrs(const clang::Decl *decl);
+getPrivateFileIDAttrs(const clang::CXXRecordDecl *decl);
 
 /// Use some heuristics to determine whether the clang::Decl associated with
 /// \a decl would not exist without C++ interop.

lib/AST/DeclContext.cpp

@@ -1490,21 +1490,34 @@ bool AccessScope::allowsPrivateAccess(const DeclContext *useDC, const DeclContex
   // Do not allow access if the sourceDC is in a different file
   auto *useSF = useDC->getOutermostParentSourceFile();
   if (useSF != sourceDC->getOutermostParentSourceFile()) {
-    // This might be a C++ declaration with a SWIFT_PRIVATE_FILEID
-    // attribute, which asks us to treat it as if it were defined in the file
+    // sourceDC might be a C++ record with a SWIFT_PRIVATE_FILEID attribute,
+    // which asks us to treat it as if it were defined in the file
     // with the specified FileID.
-
     auto clangDecl = sourceNTD->getDecl()->getClangDecl();
-    if (!clangDecl)
+
+    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();
+    }
+
+    if (!isa_and_nonnull<clang::CXXRecordDecl>(clangDecl))
       return false;
 
+    auto recordDecl = cast<clang::CXXRecordDecl>(clangDecl);
+
     // Diagnostics should enforce that there is at most SWIFT_PRIVATE_FILEID,
     // but this handles the case where there is more than anyway (whether that
     // is a feature or a bug). Allow access check to proceed if useSF is blessed
     // by any of the SWIFT_PRIVATE_FILEID annotations (i.e., disallow private
     // access if none of them bless useSF).
     if (!llvm::any_of(
-            importer::getPrivateFileIDAttrs(clangDecl), [&](auto &blessed) {
+            importer::getPrivateFileIDAttrs(recordDecl), [&](auto &blessed) {
               auto blessedFileID = SourceFile::FileIDStr::parse(blessed.first);
               return blessedFileID && blessedFileID->matches(useSF);
             })) {

lib/ClangImporter/ClangImporter.cpp

@@ -6285,9 +6285,11 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
   // to import all non-public members by default; if that is disabled, we only
   // import non-public members annotated with SWIFT_PRIVATE_FILEID (since those
   // are the only classes that need non-public members.)
+  auto *cxxRecordDecl =
+      dyn_cast<clang::CXXRecordDecl>(inheritingDecl->getClangDecl());
   auto skipIfNonPublic =
       !ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers) &&
-      importer::getPrivateFileIDAttrs(inheritingDecl->getClangDecl()).empty();
+      cxxRecordDecl && importer::getPrivateFileIDAttrs(cxxRecordDecl).empty();
 
   auto directResults = evaluateOrDefault(
       ctx.evaluator,
@@ -8758,9 +8760,8 @@ void ClangInheritanceInfo::setUnavailableIfNecessary(
 }
 
 SmallVector<std::pair<StringRef, clang::SourceLocation>, 1>
-importer::getPrivateFileIDAttrs(const clang::Decl *decl) {
+importer::getPrivateFileIDAttrs(const clang::CXXRecordDecl *decl) {
   llvm::SmallVector<std::pair<StringRef, clang::SourceLocation>, 1> files;
-
   constexpr auto prefix = StringRef("private_fileid:");
 
   if (decl->hasAttrs()) {

lib/ClangImporter/ImportDecl.cpp

@@ -10056,10 +10056,11 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
   // to import all non-public members by default; if that is disabled, we only
   // import non-public members annotated with SWIFT_PRIVATE_FILEID (since those
   // are the only classes that need non-public members.)
-  auto skipIfNonPublic =
-      !swiftDecl->getASTContext().LangOpts.hasFeature(
-          Feature::ImportNonPublicCxxMembers) &&
-      importer::getPrivateFileIDAttrs(swiftDecl->getClangDecl()).empty();
+  auto *baseRecord = dyn_cast<clang::CXXRecordDecl>(swiftDecl->getClangDecl());
+  auto skipIfNonPublic = !swiftDecl->getASTContext().LangOpts.hasFeature(
+                             Feature::ImportNonPublicCxxMembers) &&
+                         baseRecord &&
+                         importer::getPrivateFileIDAttrs(baseRecord).empty();
 
   // Import all of the members.
   llvm::SmallVector<Decl *, 16> members;

test/Interop/Cxx/class/access/Inputs/non-public.h

@@ -27,14 +27,14 @@ __attribute__((__swift_attr__("private_fileid:main/blessed.swift"))) MyClass {
   typedef int publTypedef;
   struct publStruct {};
 
-  enum publEnum { publEnumValue1 };
-  enum class publEnumClass { publEnumClassValue1 };
+  enum publEnum { variantPublEnum };
   enum { publEnumAnonValue1 };
+  enum class publEnumClass { variantPublEnumClass };
   enum publEnumClosed {
-    publEnumClosedValue1
+    variantPublEnumClosed
   } __attribute__((enum_extensibility(closed)));
   enum publEnumOpen {
-    publEnumOpenValue1
+    variantPublEnumOpen
   } __attribute__((enum_extensibility(open)));
   enum publEnumFlag {} __attribute__((flag_enum));
 
@@ -48,14 +48,14 @@ __attribute__((__swift_attr__("private_fileid:main/blessed.swift"))) MyClass {
   typedef int privTypedef;
   struct privStruct {};
 
-  enum privEnum { privEnumValue1 };
-  enum class privEnumClass { privEnumClassValue1 };
+  enum privEnum { variantPrivEnum };
   enum { privEnumAnonValue1 };
+  enum class privEnumClass { variantPrivEnumClass };
   enum privEnumClosed {
-    privEnumClosedValue1
+    variantPrivEnumClosed
   } __attribute__((enum_extensibility(closed)));
   enum privEnumOpen {
-    privEnumOpenValue1
+    variantPrivEnumOpen
   } __attribute__((enum_extensibility(open)));
   enum privEnumFlag {} __attribute__((flag_enum));
 };

test/Interop/Cxx/class/access/private-fileid-typecheck.swift

@@ -104,20 +104,20 @@ extension MyClass {
         let _: privEnumFlag
 
         // TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
-        // let _ = publEnum.publEnumValue1
-        // let _ = privEnum.privEnumValue1
+        // let _ = variantPublEnum
+        // let _ = variantPrivEnum
         //
-        // let _ = publEnumClass.publEnumClassValue1
-        // let _ = privEnumClass.privEnumClassValue1
-        //
-        // let _ = publEnumAnonValue1
-        // let _ = privEnumAnonValue1
-        //
-        // let _ = publEnumClosed.Value1
-        // let _ = privEnumClosed.Value1
-        //
-        // let _ = publEnumOpen.Value1
-        // let _ = privEnumOpen.Value1
+        // let _ = publEnumAnonVariant
+        // let _ = privEnumAnonVariant
+
+        let _ = publEnumClass.variantPublEnumClass
+        let _ = privEnumClass.variantPrivEnumClass
+
+        let _ = publEnumClosed.variantPublEnumClosed
+        let _ = privEnumClosed.variantPrivEnumClosed
+
+        let _ = publEnumOpen.variantPublEnumOpen
+        let _ = privEnumOpen.variantPrivEnumOpen
     }
 
     func fcutd(_ _: publTypedef) { }
@@ -162,20 +162,21 @@ func notExt(_ c: inout MyClass) {
     let _: MyClass.privEnumFlag // expected-error {{'privEnumFlag' is inaccessible due to 'private' protection level}}
 
     // TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
-    // let _ = MyClass.publEnum.publEnumValue1
-    // let _ = MyClass.privEnum.privEnumValue1
     //
-    // let _ = MyClass.publEnumClass.publEnumClassValue1
-    // let _ = MyClass.privEnumClass.privEnumClassValue1
+    // let _ = MyClass.variantPublEnum
+    // let _ = MyClass.variantPrivEnum // TODO-error {{'variantPrivEnum' is inaccessible due to 'private' protection level}}
     //
-    // let _ = MyClass.publEnumAnonValue1
-    // let _ = MyClass.privEnumAnonValue1
-    //
-    // let _ = MyClass.publEnumClosed.Value1
-    // let _ = MyClass.privEnumClosed.Value1
-    //
-    // let _ = MyClass.publEnumOpen.Value1
-    // let _ = MyClass.privEnumOpen.Value1
+    // let _ = MyClass.publEnumAnonVariant
+    // let _ = MyClass.privEnumAnonVariant // TODO-error {{'privEnumAnonVariant' is inaccessible due to 'private' protection level}}
+
+    let _ = MyClass.publEnumClass.variantPublEnumClass
+    let _ = MyClass.privEnumClass.variantPrivEnumClass // expected-error {{'privEnumClass' is inaccessible due to 'private' protection level}}
+
+    let _ = MyClass.publEnumClosed.variantPublEnumClosed
+    let _ = MyClass.privEnumClosed.variantPrivEnumClosed // expected-error {{'privEnumClosed' is inaccessible due to 'private' protection level}}
+
+    let _ = MyClass.publEnumOpen.variantPublEnumOpen
+    let _ = MyClass.privEnumOpen.variantPrivEnumOpen // expected-error {{'privEnumOpen' is inaccessible due to 'private' protection level}}
 }
 
 //--- cursed.swift
@@ -228,20 +229,20 @@ extension MyClass {
         let _: privEnumFlag // expected-error {{'privEnumFlag' is inaccessible due to 'private' protection level}}
 
         // TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
-        // let _ = publEnum.publEnumValue1
-        // let _ = privEnum.privEnumValue1
-        //
-        // let _ = publEnumClass.publEnumClassValue1
-        // let _ = privEnumClass.privEnumClassValue1
+        // let _ = variantPublEnum
+        // let _ = variantPrivEnum // TODO-error {{'variantPrivEnum' is inaccessible due to 'private' protection level}}
         //
-        // let _ = publEnumAnonValue1
-        // let _ = privEnumAnonValue1
-        //
-        // let _ = publEnumClosed.Value1
-        // let _ = privEnumClosed.Value1
-        //
-        // let _ = publEnumOpen.Value1
-        // let _ = privEnumOpen.Value1
+        // let _ = publEnumAnonVariant
+        // let _ = privEnumAnonVariant // TODO-error {{'privEnumAnonVariant' is inaccessible due to 'private' protection level}}
+
+        let _ = publEnumClass.variantPublEnumClass
+        let _ = privEnumClass.variantPrivEnumClass// expected-error {{'privEnumClass' is inaccessible due to 'private' protection level}}
+
+        let _ = publEnumClosed.variantPublEnumClosed
+        let _ = privEnumClosed.variantPrivEnumClosed // expected-error {{'privEnumClosed' is inaccessible due to 'private' protection level}}
+
+        let _ = publEnumOpen.variantPublEnumOpen
+        let _ = privEnumOpen.variantPrivEnumOpen // expected-error {{'privEnumOpen' is inaccessible due to 'private' protection level}}
     }
 }
 
@@ -283,18 +284,18 @@ func notExt(_ c: inout MyClass) {
     let _: MyClass.privEnumFlag // expected-error {{'privEnumFlag' is inaccessible due to 'private' protection level}}
 
     // TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
-    // let _ = MyClass.publEnum.publEnumValue1
-    // let _ = MyClass.privEnum.privEnumValue1
-    //
-    // let _ = MyClass.publEnumClass.publEnumClassValue1
-    // let _ = MyClass.privEnumClass.privEnumClassValue1
+    // let _ = MyClass.variantPublEnum
+    // let _ = MyClass.variantPrivEnum // TODO-error {{'variantPrivEnum' is inaccessible due to 'private' protection level}}
     //
-    // let _ = MyClass.publEnumAnonValue1
-    // let _ = MyClass.privEnumAnonValue1
-    //
-    // let _ = MyClass.publEnumClosed.Value1
-    // let _ = MyClass.privEnumClosed.Value1
-    //
-    // let _ = MyClass.publEnumOpen.Value1
-    // let _ = MyClass.privEnumOpen.Value1
+    // let _ = MyClass.publEnumAnonVariant
+    // let _ = MyClass.privEnumAnonVariant // TODO-error {{'privEnumAnonVariant' is inaccessible due to 'private' protection level}}
+
+    let _ = MyClass.publEnumClass.variantPublEnumClass
+    let _ = MyClass.privEnumClass.variantPrivEnumClass// expected-error {{'privEnumClass' is inaccessible due to 'private' protection level}}
+
+    let _ = MyClass.publEnumClosed.variantPublEnumClosed
+    let _ = MyClass.privEnumClosed.variantPrivEnumClosed // expected-error {{'privEnumClosed' is inaccessible due to 'private' protection level}}
+
+    let _ = MyClass.publEnumOpen.variantPublEnumOpen
+    let _ = MyClass.privEnumOpen.variantPrivEnumOpen // expected-error {{'privEnumOpen' is inaccessible due to 'private' protection level}}
 }