[cxx-interop] add support for 'address' pointee for non-copyable C++ types

Author: hyp

lib/ClangImporter/ImportDecl.cpp

@@ -125,12 +125,22 @@ void ClangImporter::Implementation::makeComputed(AbstractStorageDecl *storage,
                                                  AccessorDecl *getter,
                                                  AccessorDecl *setter) {
   assert(getter);
+  // The synthesized computed property can either use a `get` or an
+  // `unsafeAddress` accessor.
+  auto isAddress = getter->getAccessorKind() == AccessorKind::Address;
   storage->getASTContext().evaluator.cacheOutput(HasStorageRequest{storage}, false);
   if (setter) {
-    storage->setImplInfo(StorageImplInfo::getMutableComputed());
+    if (isAddress)
+      assert(setter->getAccessorKind() == AccessorKind::MutableAddress);
+    storage->setImplInfo(
+        isAddress ? StorageImplInfo(ReadImplKind::Address,
+                                    WriteImplKind::MutableAddress,
+                                    ReadWriteImplKind::MutableAddress)
+                  : StorageImplInfo::getMutableComputed());
     storage->setAccessors(SourceLoc(), {getter, setter}, SourceLoc());
   } else {
-    storage->setImplInfo(StorageImplInfo::getImmutableComputed());
+    storage->setImplInfo(isAddress ? StorageImplInfo(ReadImplKind::Address)
+                                   : StorageImplInfo::getImmutableComputed());
     storage->setAccessors(SourceLoc(), {getter}, SourceLoc());
   }
 }

lib/ClangImporter/SwiftDeclSynthesizer.cpp

@@ -1501,7 +1501,8 @@ AccessorDecl *SwiftDeclSynthesizer::buildSubscriptSetterDecl(
 /// an UnsafePointer or UnsafeMutablePointer, it unwraps the pointer and returns
 /// the underlying value.
 static std::pair<BraceStmt *, bool>
-synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
+synthesizeUnwrappingGetterOrAddressGetterBody(AbstractFunctionDecl *afd,
+                                              void *context, bool isAddress) {
   auto getterDecl = cast<AccessorDecl>(afd);
   auto getterImpl = static_cast<FuncDecl *>(context);
 
@@ -1525,7 +1526,8 @@ synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
   // reference type. This check actually checks to see if the type is a pointer
   // type, but this does not apply to C pointers because they are Optional types
   // when imported. TODO: Use a more obvious check here.
-  if (getterImpl->getResultInterfaceType()->getAnyPointerElementType(ptrKind)) {
+  if (!isAddress &&
+      getterImpl->getResultInterfaceType()->getAnyPointerElementType(ptrKind)) {
     // `getterImpl` can return either UnsafePointer or UnsafeMutablePointer.
     // Retrieve the corresponding `.pointee` declaration.
     VarDecl *pointeePropertyDecl = ctx.getPointerPointeePropertyDecl(ptrKind);
@@ -1549,6 +1551,19 @@ synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
   return {body, /*isTypeChecked*/ true};
 }
 
+static std::pair<BraceStmt *, bool>
+synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
+  return synthesizeUnwrappingGetterOrAddressGetterBody(afd, context,
+                                                       /*isAddress=*/false);
+}
+
+static std::pair<BraceStmt *, bool>
+synthesizeUnwrappingAddressGetterBody(AbstractFunctionDecl *afd,
+                                      void *context) {
+  return synthesizeUnwrappingGetterOrAddressGetterBody(afd, context,
+                                                       /*isAddress=*/true);
+}
+
 /// Synthesizer callback for a subscript setter or a setter for a dereference
 /// property (`var pointee`).
 static std::pair<BraceStmt *, bool>
@@ -1685,7 +1700,6 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
                                                       FuncDecl *setter) {
   assert((getter || setter) &&
          "getter or setter required to generate a pointee property");
-
   auto &ctx = ImporterImpl.SwiftContext;
   FuncDecl *getterImpl = getter ? getter : setter;
   FuncDecl *setterImpl = setter;
@@ -1697,6 +1711,9 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
   const auto elementTy = rawElementTy->getAnyPointerElementType()
                              ? rawElementTy->getAnyPointerElementType()
                              : rawElementTy;
+  // Use 'address' or 'mutableAddress' accessors for non-copyable
+  // types.
+  bool useAddress = elementTy->isNoncopyable(dc);
 
   auto result = new (ctx)
       VarDecl(/*isStatic*/ false, VarDecl::Introducer::Var,
@@ -1705,16 +1722,21 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
   result->setAccess(AccessLevel::Public);
 
   AccessorDecl *getterDecl = AccessorDecl::create(
-      ctx, getterImpl->getLoc(), getterImpl->getLoc(), AccessorKind::Get,
-      result, SourceLoc(), StaticSpellingKind::None,
+      ctx, getterImpl->getLoc(), getterImpl->getLoc(),
+      useAddress ? AccessorKind::Address : AccessorKind::Get, result,
+      SourceLoc(), StaticSpellingKind::None,
       /*async*/ false, SourceLoc(),
       /*throws*/ false, SourceLoc(), /*ThrownType=*/TypeLoc(),
-      ParameterList::createEmpty(ctx), elementTy, dc);
+      ParameterList::createEmpty(ctx),
+      useAddress ? elementTy->wrapInPointer(PTK_UnsafePointer) : elementTy, dc);
   getterDecl->setAccess(AccessLevel::Public);
-  getterDecl->setImplicit();
+  if (!useAddress)
+    getterDecl->setImplicit();
   getterDecl->setIsDynamic(false);
   getterDecl->setIsTransparent(true);
-  getterDecl->setBodySynthesizer(synthesizeUnwrappingGetterBody,
+  getterDecl->setBodySynthesizer(useAddress
+                                     ? synthesizeUnwrappingAddressGetterBody
+                                     : synthesizeUnwrappingGetterBody,
                                  getterImpl);
 
   if (getterImpl->isMutating()) {
@@ -1737,13 +1759,18 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
         ParameterList::create(ctx, {paramVarDecl});
 
     setterDecl = AccessorDecl::create(
-        ctx, setterImpl->getLoc(), setterImpl->getLoc(), AccessorKind::Set,
-        result, SourceLoc(), StaticSpellingKind::None,
+        ctx, setterImpl->getLoc(), setterImpl->getLoc(),
+        useAddress ? AccessorKind::MutableAddress : AccessorKind::Set, result,
+        SourceLoc(), StaticSpellingKind::None,
         /*async*/ false, SourceLoc(),
         /*throws*/ false, SourceLoc(), /*ThrownType=*/TypeLoc(),
-        setterParamList, TupleType::getEmpty(ctx), dc);
+        setterParamList,
+        useAddress ? elementTy->wrapInPointer(PTK_UnsafeMutablePointer)
+                   : TupleType::getEmpty(ctx),
+        dc);
     setterDecl->setAccess(AccessLevel::Public);
-    setterDecl->setImplicit();
+    if (!useAddress)
+      setterDecl->setImplicit();
     setterDecl->setIsDynamic(false);
     setterDecl->setIsTransparent(true);
     setterDecl->setBodySynthesizer(synthesizeUnwrappingSetterBody, setterImpl);

test/Interop/Cxx/operators/move-only/Inputs/module.modulemap

@@ -0,0 +1,4 @@
+module MoveOnlyCxxOperators {
+  header "move-only-cxx-value-operators.h"
+  requires cplusplus
+}

test/Interop/Cxx/operators/move-only/Inputs/move-only-cxx-value-operators.h

@@ -0,0 +1,38 @@
+#ifndef TEST_INTEROP_CXX_OPERATORS_MOVE_ONLY_OPS_H
+#define TEST_INTEROP_CXX_OPERATORS_MOVE_ONLY_OPS_H
+
+#include <memory>
+
+struct Copyable {
+    int x;
+};
+
+struct NonCopyable {
+    inline NonCopyable(int x) : x(x) {}
+    inline NonCopyable(const NonCopyable &) = delete;
+    inline NonCopyable(NonCopyable &&other) : x(other.x) { other.x = 0; }
+
+    inline int method(int y) const { return x * y; }
+    inline int mutMethod(int y) {
+      x = y;
+      return y;
+    }
+
+    int x;
+};
+
+#define NONCOPYABLE_HOLDER_WRAPPER(Name) \
+private: \
+NonCopyable x; \
+public: \
+inline Name(int x) : x(x) {} \
+inline Name(const Name &) = delete; \
+inline Name(Name &&other) : x(std::move(other.x)) {}
+
+class NonCopyableHolderConstDeref {
+    NONCOPYABLE_HOLDER_WRAPPER(NonCopyableHolderConstDeref)
+
+    inline const NonCopyable & operator *() const { return x; }
+};
+
+#endif // TEST_INTEROP_CXX_OPERATORS_MOVE_ONLY_OPS_H

test/Interop/Cxx/operators/move-only/move-only-synthesized-properties.swift

@@ -0,0 +1,32 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop)
+//
+// REQUIRES: executable_test
+
+import MoveOnlyCxxOperators
+import StdlibUnittest
+
+var MoveOnlyCxxOperators = TestSuite("Move Only Operators")
+
+func borrowNC(_ x: borrowing NonCopyable) -> CInt {
+  return x.method(3)
+}
+
+func inoutNC(_ x: inout NonCopyable, _ y: CInt) -> CInt {
+  return x.mutMethod(y)
+}
+
+func consumingNC(_ x: consuming NonCopyable) {
+  // do nothing.
+}
+
+MoveOnlyCxxOperators.test("NonCopyableHolderConstDeref pointee borrow") {
+  let holder = NonCopyableHolderConstDeref(11)
+  var k = borrowNC(holder.pointee)
+  expectEqual(k, 33)
+  k = holder.pointee.method(2)
+  expectEqual(k, 22)
+  k = holder.pointee.x
+  expectEqual(k, 11)
+}
+
+runAllTests()

test/Interop/Cxx/operators/move-only/move-only-synthesized-property-typecheck.swift

@@ -0,0 +1,36 @@
+// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-experimental-cxx-interop -DNO_CONSUME
+
+// Note: some errors need full SIL emission right now.
+// FIXME: they should be type checker errors.
+// RUN: not %target-swift-emit-sil %s -I %S/Inputs -enable-experimental-cxx-interop 2>&1 | %FileCheck %s
+
+import MoveOnlyCxxOperators
+
+func borrowNC(_ x: borrowing NonCopyable) -> CInt {
+  return x.method(3)
+}
+
+func inoutNC(_ x: inout NonCopyable) -> CInt {
+  return x.mutMethod(5)
+}
+
+func consumingNC(_ x: consuming NonCopyable) {
+  // do nothing.
+}
+
+func testNonCopyableHolderConstDerefPointee() {
+    var holder = NonCopyableHolderConstDeref(11)
+    _ = borrowNC(holder.pointee) // ok
+    _ = holder.pointee.method(2)
+    _ = holder.pointee.x
+#if NO_CONSUME
+    _ = inoutNC(holder.pointee)  // expected-error {{cannot pass immutable value as inout argument: 'pointee' is a get-only property}}
+    holder.pointee.mutMethod(1) // expected-error {{cannot use mutating member on immutable value: 'pointee' is a get-only property}}
+    holder.pointee.x = 2 // expected-error {{cannot assign to property: 'pointee' is a get-only property}}
+#else
+    consumingNC(holder.pointee) // CHECK: [[@LINE]]:{{.*}}: error:
+    let consumeVal = holder.pointee // CHECK: [[@LINE]]:{{.*}}: error:
+#endif
+}
+
+testNonCopyableHolderConstDerefPointee()