Don't import C++ class members that are protected or private (swiftlang#30233)

martinboehme · web-flow · commit fa7155073d1e · 2020-03-19T08:20:36.000-07:00
* Don't import C++ class members that are protected or private.

We omit protected members in addition to private members because Swift
structs can't inherit from C++ classes, so there's effectively no way to
access them.

* Check access specifiers centrally in importDeclImpl().

* Fix macOS build by using &lt;stddef.h&gt; instead of &lt;cstddef&gt;.

Apparently, the macOS toolchain doesn't provide &lt;cstddef&gt;.

&lt;stddef.h&gt; is used in test/Inputs/clang-importer-sdk/usr/include/macros.h,
so I'm assuming it will be OK. (I don't unfortunately have a macOS
machine to test on.)

* Add comment explaining why we skip private and protected C++ class
members.
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -7731,6 +7731,17 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
                                               bool &HadForwardDeclaration) {
   assert(ClangDecl);
 
+  // Private and protected C++ class members should never be used, so we skip
+  // them entirely (instead of importing them with a corresponding Swift access
+  // level) to remove clutter from the module interface.
+  //
+  // We omit protected members in addition to private members because Swift
+  // structs can't inherit from C++ classes, so there's effectively no way to
+  // access them.
+  clang::AccessSpecifier access = ClangDecl->getAccess();
+  if (access == clang::AS_protected || access == clang::AS_private)
+    return nullptr;
+
   bool SkippedOverTypedef = false;
   Decl *Result = nullptr;
   if (auto *UnderlyingDecl = canSkipOverTypedef(*this, ClangDecl,
diff --git a/test/ClangImporter/Inputs/custom-modules/AccessSpecifiers.h b/test/ClangImporter/Inputs/custom-modules/AccessSpecifiers.h
@@ -0,0 +1,35 @@
+class PublicPrivate {
+public:
+  int PublicMemberVar;
+  static int PublicStaticMemberVar;
+  void publicMemberFunc();
+
+  typedef int PublicTypedef;
+  struct PublicStruct {};
+  enum PublicEnum { PublicEnumValue1 };
+  enum { PublicAnonymousEnumValue };
+  enum PublicClosedEnum {
+    PublicClosedEnumValue1
+  } __attribute__((enum_extensibility(closed)));
+  enum PublicOpenEnum {
+    PublicOpenEnumValue1
+  } __attribute__((enum_extensibility(open)));
+  enum PublicFlagEnum {} __attribute__((flag_enum));
+
+private:
+  int PrivateMemberVar;
+  static int PrivateStaticMemberVar;
+  void privateMemberFunc() {}
+
+  typedef int PrivateTypedef;
+  struct PrivateStruct {};
+  enum PrivateEnum { PrivateEnumValue1 };
+  enum { PrivateAnonymousEnumValue1 };
+  enum PrivateClosedEnum {
+    PrivateClosedEnumValue1
+  } __attribute__((enum_extensibility(closed)));
+  enum PrivateOpenEnum {
+    PrivateOpenEnumValue1
+  } __attribute__((enum_extensibility(open)));
+  enum PrivateFlagEnum {} __attribute__((flag_enum));
+};
diff --git a/test/ClangImporter/Inputs/custom-modules/MemoryLayout.h b/test/ClangImporter/Inputs/custom-modules/MemoryLayout.h
@@ -0,0 +1,17 @@
+#include <stddef.h>
+#include <stdint.h>
+
+class PrivateMemberLayout {
+  uint32_t a;
+
+public:
+  uint32_t b;
+};
+
+inline size_t sizeOfPrivateMemberLayout() {
+  return sizeof(PrivateMemberLayout);
+}
+
+inline size_t offsetOfPrivateMemberLayout_b() {
+  return offsetof(PrivateMemberLayout, b);
+}
diff --git a/test/ClangImporter/Inputs/custom-modules/module.map b/test/ClangImporter/Inputs/custom-modules/module.map
@@ -2,6 +2,11 @@ module script {
   header "script.h"
 }
 
+module AccessSpecifiers {
+  header "AccessSpecifiers.h"
+  export *
+}
+
 module AvailabilityExtras {
   header "AvailabilityExtras.h"
   export *
@@ -84,6 +89,11 @@ module LinkMusket {
   link framework "Barrel"
 }
 
+module MemoryLayout {
+  header "MemoryLayout.h"
+  export *
+}
+
 module MissingHeader {
   header "this-header-does-not-exist.h"
 }
diff --git a/test/ClangImporter/access-specifiers-module-interface.swift b/test/ClangImporter/access-specifiers-module-interface.swift
@@ -0,0 +1,43 @@
+// Test module interface produced for C++ access specifiers test.
+// In particular, we don't want any of the private members showing up here.
+
+// RUN: %target-swift-ide-test -print-module -module-to-print=AccessSpecifiers -I %S/Inputs/ -source-filename=x -enable-cxx-interop | %FileCheck %s
+
+// CHECK:      struct PublicPrivate {
+// CHECK-NEXT:   typealias PublicTypedef = Int32
+// CHECK-NEXT:   struct PublicStruct {
+// CHECK-NEXT:     init()
+// CHECK-NEXT:   }
+// CHECK-NEXT:   struct PublicEnum : Equatable, RawRepresentable {
+// CHECK-NEXT:     init(_ rawValue: UInt32)
+// CHECK-NEXT:     init(rawValue: UInt32)
+// CHECK-NEXT:     var rawValue: UInt32
+// CHECK-NEXT:     typealias RawValue = UInt32
+// CHECK-NEXT:   }
+// CHECK-NEXT:   @frozen enum PublicClosedEnum : UInt32 {
+// CHECK-NEXT:     init?(rawValue: UInt32)
+// CHECK-NEXT:     var rawValue: UInt32 { get }
+// CHECK-NEXT:     typealias RawValue = UInt32
+// CHECK-NEXT:     case value1
+// CHECK-NEXT:     @available(swift, obsoleted: 3, renamed: "value1")
+// CHECK-NEXT:     static var Value1: PublicPrivate.PublicClosedEnum { get }
+// CHECK-NEXT:   }
+// CHECK-NEXT:   enum PublicOpenEnum : UInt32 {
+// CHECK-NEXT:     init?(rawValue: UInt32)
+// CHECK-NEXT:     var rawValue: UInt32 { get }
+// CHECK-NEXT:     typealias RawValue = UInt32
+// CHECK-NEXT:     case value1
+// CHECK-NEXT:     @available(swift, obsoleted: 3, renamed: "value1")
+// CHECK-NEXT:     static var Value1: PublicPrivate.PublicOpenEnum { get }
+// CHECK-NEXT:   }
+// CHECK-NEXT:   struct PublicFlagEnum : OptionSet {
+// CHECK-NEXT:     init(rawValue: UInt32)
+// CHECK-NEXT:     let rawValue: UInt32
+// CHECK-NEXT:     typealias RawValue = UInt32
+// CHECK-NEXT:     typealias Element = PublicPrivate.PublicFlagEnum
+// CHECK-NEXT:     typealias ArrayLiteralElement = PublicPrivate.PublicFlagEnum
+// CHECK-NEXT:   }
+// CHECK-NEXT:   var PublicMemberVar: Int32
+// CHECK-NEXT:   init()
+// CHECK-NEXT:   mutating func publicMemberFunc()
+// CHECK-NEXT: }
diff --git a/test/ClangImporter/access-specifiers.swift b/test/ClangImporter/access-specifiers.swift
@@ -0,0 +1,47 @@
+// Test that C++ access specifiers are honored, i.e. private members aren't
+// imported.
+
+// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs/custom-modules/ -enable-cxx-interop
+
+import AccessSpecifiers
+
+var v = PublicPrivate()
+
+// Can access all public members and types.
+
+v.PublicMemberVar = 1
+// TODO: Static member variables don't appear to be imported correctly yet. Once
+// they are, verify that PublicStaticMemberVar is accessible.
+// PublicPrivate.PublicStaticMemberVar = 1
+v.publicMemberFunc()
+
+var publicTypedefVar: PublicPrivate.PublicTypedef
+var publicStructVar: PublicPrivate.PublicStruct
+var publicEnumVar: PublicPrivate.PublicEnum
+// TODO: These enum values don't yet appear to be imported correctly into the
+// scope of PublicPrivate yet. Once they are, verify that they are accessible.
+// print(PublicPrivate.PublicEnumValue1)
+// print(PublicPrivate.PublicAnonymousEnumValue1)
+var publicClosedEnumVar: PublicPrivate.PublicClosedEnum
+var publicOpenEnumVar: PublicPrivate.PublicOpenEnum
+var publicFlagEnumVar: PublicPrivate.PublicFlagEnum
+
+// Cannot access any private members and types.
+
+v.PrivateMemberVar = 1 // expected-error {{value of type 'PublicPrivate' has no member 'PrivateMemberVar'}}
+// TODO: This gives the expected error, but only because static member variables
+// (private or  public) aren't imported at all. Once that is fixed, remove this
+PublicPrivate.PrivateStaticMemberVar = 1 // expected-error {{'PublicPrivate' has no member 'PrivateStaticMemberVar'}}
+v.privateMemberFunc() // expected-error {{value of type 'PublicPrivate' has no member 'privateMemberFunc'}}
+
+var privateTypedefVar: PublicPrivate.PrivateTypedef // expected-error {{'PrivateTypedef' is not a member type of 'PublicPrivate'}}
+var privateStructVar: PublicPrivate.PrivateStruct // expected-error {{'PrivateStruct' is not a member type of 'PublicPrivate'}}
+var privateEnumVar: PublicPrivate.PrivateEnum // expected-error {{'PrivateEnum' is not a member type of 'PublicPrivate'}}
+// TODO: PrivateEnumValue1 and PrivateAnonymousEnumValue1 give the expected
+// error, but only because these types of enums (private or public) aren't
+// currently imported at all. Once that is fixed, remove this TODO.
+print(PublicPrivate.PrivateEnumValue1) // expected-error {{'PublicPrivate' has no member 'PrivateEnumValue1'}}
+print(PublicPrivate.PrivateAnonymousEnumValue1) // expected-error {{'PublicPrivate' has no member 'PrivateAnonymousEnumValue1'}}
+var privateClosedEnumVar: PublicPrivate.PrivateClosedEnum // expected-error {{'PrivateClosedEnum' is not a member type of 'PublicPrivate'}}
+var privateOpenEnumVar: PublicPrivate.PrivateOpenEnum // expected-error {{'PrivateOpenEnum' is not a member type of 'PublicPrivate'}}
+var privateFlagEnumVar: PublicPrivate.PrivateFlagEnum // expected-error {{'PrivateFlagEnum' is not a member type of 'PublicPrivate'}}
diff --git a/test/ClangImporter/memory-layout-executable.swift b/test/ClangImporter/memory-layout-executable.swift
@@ -0,0 +1,24 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift %s -I %S/Inputs/custom-modules/ -o %t/class_layout -Xfrontend -enable-cxx-interop
+// RUN: %target-codesign %t/class_layout
+// RUN: %target-run %t/class_layout 2&>1
+//
+// REQUIRES: executable_test
+
+import MemoryLayout
+import StdlibUnittest
+
+var MemoryLayoutTestSuite = TestSuite("MemoryLayout")
+
+MemoryLayoutTestSuite.test("PrivateMemberLayout") {
+  // Check that, even though the private member variable 'a' is not imported, we
+  // still use the same memory layout as Clang.
+  expectEqual(sizeOfPrivateMemberLayout(),
+    MemoryLayout<PrivateMemberLayout>.size)
+  expectEqual(sizeOfPrivateMemberLayout(),
+    MemoryLayout<PrivateMemberLayout>.stride)
+  expectEqual(offsetOfPrivateMemberLayout_b(),
+    MemoryLayout<PrivateMemberLayout>.offset(of: \PrivateMemberLayout.b));
+}
+
+runAllTests()
diff --git a/test/ClangImporter/memory-layout-ir.swift b/test/ClangImporter/memory-layout-ir.swift
@@ -0,0 +1,12 @@
+// RUN: %target-swift-frontend -I %S/Inputs/custom-modules/ -enable-cxx-interop -emit-ir -o - %s | %FileCheck %s
+
+import MemoryLayout
+
+var v = PrivateMemberLayout()
+// Check that even though the private member variable `a` is not imported, it is
+// still taken into account for computing the layout of the class. In other
+// words, we use Clang's, not Swift's view of the class to compute the memory
+// layout.
+// The important point here is that the second index is 1, not 0.
+// CHECK: store i32 42, i32* getelementptr inbounds (%TSo19PrivateMemberLayoutV, %TSo19PrivateMemberLayoutV* @"$s4main1vSo19PrivateMemberLayoutVvp", i32 0, i32 1, i32 0), align 4
+v.b = 42
diff --git a/test/CxxInterop/class/Inputs/module.modulemap b/test/CxxInterop/class/Inputs/module.modulemap
@@ -0,0 +1,7 @@
+module AccessSpecifiers {
+  header "access_specifiers.h"
+}
+
+module MemoryLayout {
+  header "memory_layout.h"
+}
diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp
@@ -697,6 +697,11 @@ static llvm::cl::opt<bool>
                        llvm::cl::desc("Disable ObjC interop."),
                        llvm::cl::cat(Category), llvm::cl::init(false));
 
+static llvm::cl::opt<bool>
+    EnableCxxInterop("enable-cxx-interop",
+                     llvm::cl::desc("Enable C++ interop."),
+                     llvm::cl::cat(Category), llvm::cl::init(false));
+
 static llvm::cl::opt<std::string>
 GraphVisPath("output-request-graphviz",
              llvm::cl::desc("Emit GraphViz output visualizing the request graph."),
@@ -3373,6 +3378,9 @@ int main(int argc, char *argv[]) {
     InitInvok.getLangOptions().EnableObjCInterop =
         llvm::Triple(options::Triple).isOSDarwin();
   }
+  if (options::EnableCxxInterop) {
+    InitInvok.getLangOptions().EnableCXXInterop = true;
+  }
 
   // We disable source location resolutions from .swiftsourceinfo files by
   // default to match sourcekitd-test's and ide clients' expected behavior
