[cxx-interop] Make experimental flag ImportNonPublicCxxMembers (swiftlang#79728)

ClangImporter can now import non-public members as of be73254 and 66c2e2c, but doing so triggers some latent ClangImporter bugs in projects that don't use or need those non-public members.

This patch introduces a new experimental feature flag, ImportNonPublicCxxMembers, that guards against the importation of non-public members while we iron out those latent issues. Adopters of the SWIFT_PRIVATE_FILEID feature introduced in bdf2294 can enable this flag to opt into importing private members they wish to access from Swift.

rdar://145569473

Author: j-hui

include/swift/Basic/Features.def

@@ -423,6 +423,9 @@ EXPERIMENTAL_FEATURE(SafeInteropWrappers, false)
 /// Ignore resilience errors due to C++ types.
 EXPERIMENTAL_FEATURE(AssumeResilientCxxTypes, true)
 
+/// Import inherited non-public members when importing C++ classes.
+EXPERIMENTAL_FEATURE(ImportNonPublicCxxMembers, true)
+
 // Isolated deinit
 SUPPRESSIBLE_LANGUAGE_FEATURE(IsolatedDeinit, 371, "isolated deinit")
 

lib/AST/FeatureSet.cpp

@@ -386,6 +386,7 @@ static bool usesFeatureMemorySafetyAttributes(Decl *decl) {
 UNINTERESTING_FEATURE(StrictMemorySafety)
 UNINTERESTING_FEATURE(SafeInteropWrappers)
 UNINTERESTING_FEATURE(AssumeResilientCxxTypes)
+UNINTERESTING_FEATURE(ImportNonPublicCxxMembers)
 UNINTERESTING_FEATURE(CoroutineAccessorsUnwindOnCallerError)
 UNINTERESTING_FEATURE(CoroutineAccessorsAllocateInCallee)
 

lib/ClangImporter/ClangImporter.cpp

@@ -6193,21 +6193,6 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
   DeclName name = desc.name;
   ClangInheritanceInfo inheritance = desc.inheritance;
 
-  // HACK: the inherited, synthesized, private 'pointee' property used in MSVC's
-  // std::optional implementation causes problems when conforming it to
-  // CxxOptional (see conformToCxxOptionalIfNeeded()), since it clashes with the
-  // public 'pointee' property synthesized from using _Mybase::operator* (where
-  // _Mybase is _Optional_construct_base). The root cause seems to be the
-  // cloned member cache's inability to manage special decls synthesized from
-  // operators.
-  if (auto *decl =
-          dyn_cast_or_null<clang::CXXRecordDecl>(recordDecl->getClangDecl())) {
-    if (decl->isInStdNamespace() && decl->getIdentifier() &&
-        decl->getName() == "_Optional_construct_base" &&
-        desc.name.getBaseName() == "pointee")
-      return {};
-  }
-
   auto &ctx = recordDecl->getASTContext();
   auto directResults = evaluateOrDefault(
       ctx.evaluator,
@@ -6223,6 +6208,17 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
         recordDecl->getClangDecl())
       continue;
 
+    if (!ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers)) {
+      auto access = found->getAccess();
+      if ((access == clang::AS_private || access == clang::AS_protected) &&
+          (inheritance || !isa<clang::FieldDecl>(found)))
+        // 'found' is a non-public member and ImportNonPublicCxxMembers is not
+        // enabled. Don't import it unless it is a non-inherited field, which
+        // we must import because it may affect implicit conformances that
+        // iterate through all of a struct's fields, e.g., Sendable (#76892).
+        continue;
+    }
+
     // Don't import constructors on foreign reference types.
     if (isa<clang::CXXConstructorDecl>(found) && isa<ClassDecl>(recordDecl))
       continue;
@@ -6275,6 +6271,10 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
       foundNameArities.insert(getArity(valueDecl));
 
     for (auto base : cxxRecord->bases()) {
+      if (!ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers) &&
+          base.getAccessSpecifier() != clang::AS_public)
+        continue;
+
       clang::QualType baseType = base.getType();
       if (auto spectType = dyn_cast<clang::TemplateSpecializationType>(baseType))
         baseType = spectType->desugar();

lib/ClangImporter/ImportDecl.cpp

@@ -9963,6 +9963,18 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
     if (!nd)
       continue;
 
+    if (!swiftDecl->getASTContext().LangOpts.hasFeature(
+            Feature::ImportNonPublicCxxMembers)) {
+      auto access = nd->getAccess();
+      if ((access == clang::AS_private || access == clang::AS_protected) &&
+          (inheritance || !isa<clang::FieldDecl>(nd)))
+        // 'nd' is a non-public member and ImportNonPublicCxxMembers is not
+        // enabled. Don't import it unless it is a non-inherited field, which
+        // we must import because it may affect implicit conformances that
+        // iterate through all of a struct's fields, e.g., Sendable (#76892).
+        continue;
+    }
+
     // Currently, we don't import unnamed bitfields.
     if (isa<clang::FieldDecl>(m) &&
         cast<clang::FieldDecl>(m)->isUnnamedBitField())
@@ -10030,6 +10042,11 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
   // If this is a C++ record, look through the base classes too.
   if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(clangRecord)) {
     for (auto base : cxxRecord->bases()) {
+      if (!swiftDecl->getASTContext().LangOpts.hasFeature(
+              Feature::ImportNonPublicCxxMembers) &&
+          base.getAccessSpecifier() != clang::AS_public)
+        continue;
+
       clang::QualType baseType = base.getType();
       if (auto spectType = dyn_cast<clang::TemplateSpecializationType>(baseType))
         baseType = spectType->desugar();

test/Interop/Cxx/class/access/access-inversion-module-interface.swift

@@ -1,4 +1,5 @@
-// RUN: %target-swift-ide-test -print-module -print-access -module-to-print=AccessInversion -I %S/Inputs -source-filename=x -cxx-interoperability-mode=default | %FileCheck %s
+// RUN: %target-swift-ide-test -print-module -print-access -module-to-print=AccessInversion -I %S/Inputs -source-filename=x -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers | %FileCheck %s
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
 
 // CHECK: public struct Leaky {
 

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

@@ -4,7 +4,8 @@
 // but does not necessarily specify it (in the deliberate sense). In other words,
 // there may be behaviors captured in these tests that deserve amending.
 
-// RUN: %target-typecheck-verify-swift -I %S/Inputs -cxx-interoperability-mode=default
+// RUN: %target-typecheck-verify-swift -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
 
 import AccessInversion
 

test/Interop/Cxx/class/access/access-specifiers-module-interface.swift

@@ -2,7 +2,8 @@
 // Public C++ members should be imported with Swift-public access, and
 // private C++ members should be imported with Swift-private access.
 
-// RUN: %target-swift-ide-test -print-module -module-to-print=AccessSpecifiers -print-access -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
+// RUN: %target-swift-ide-test -print-module -module-to-print=AccessSpecifiers -print-access -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop -enable-experimental-feature ImportNonPublicCxxMembers | %FileCheck %s
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
 
 // CHECK:      public struct PublicPrivate {
 // CHECK-NEXT:   public init()

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

@@ -1,6 +1,7 @@
 // Test that C++ access specifiers are honored.
 
-// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -enable-experimental-cxx-interop
+// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -enable-experimental-cxx-interop -enable-experimental-feature ImportNonPublicCxxMembers
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
 
 import AccessSpecifiers
 

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

@@ -2,11 +2,12 @@
 // Test that all accessible inherited methods can be called.
 //
 // RUN: split-file %s %t
-// RUN: %target-build-swift -module-name main %t/blessed.swift -I %S/Inputs -o %t/out -Xfrontend -cxx-interoperability-mode=default
+// RUN: %target-build-swift -module-name main %t/blessed.swift -I %S/Inputs -o %t/out -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
 // RUN: %target-codesign %t/out
 // RUN: %target-run %t/out
 //
 // REQUIRES: executable_test
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
 
 import StdlibUnittest
 import NonPublicInheritance

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

@@ -7,7 +7,8 @@
 // against message wording changes), but it does require the message to mention
 // something about "private".
 //
-// RUN: %target-swift-ide-test -print-module -module-to-print=NonPublicInheritance -print-access -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
+// RUN: %target-swift-ide-test -print-module -module-to-print=NonPublicInheritance -print-access -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop -enable-experimental-feature ImportNonPublicCxxMembers | %FileCheck %s
+// REQUIRES: swift_feature_ImportNonPublicCxxMembers
 
 // CHECK:      public struct Base {
 // CHECK-NEXT:   public init()