[cxx-interop] fix the use of '.pointee' with address accessors for derived-to-base synthesized accessors

Author: hyp

lib/ClangImporter/ClangImporter.cpp

@@ -19,6 +19,7 @@
 #include "ClangDiagnosticConsumer.h"
 #include "ClangIncludePaths.h"
 #include "ImporterImpl.h"
+#include "SwiftDeclSynthesizer.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/Builtins.h"
 #include "swift/AST/ClangModuleLoader.h"
@@ -4948,6 +4949,18 @@ const clang::CXXMethodDecl *getCalledBaseCxxMethod(FuncDecl *baseMember) {
   if (!returnStmt)
     return nullptr;
   Expr *returnExpr = returnStmt->getResult();
+  // Look through a potential 'reinterpresetCast' that can be used
+  // to cast UnsafeMutablePointer to UnsafePointer in the synthesized
+  // Swift body for `.pointee`.
+  if (auto *ce = dyn_cast<CallExpr>(returnExpr)) {
+    if (auto *v = ce->getCalledValue()) {
+      if (v->getModuleContext() ==
+              baseMember->getASTContext().TheBuiltinModule &&
+          v->getBaseName().getIdentifier().str() == "reinterpretCast") {
+        returnExpr = ce->getArgs()->get(0).getExpr();
+      }
+    }
+  }
   // A member ref expr for `.pointee` access can be wrapping a call
   // when looking through the synthesized Swift body for `.pointee`
   // accessor.
@@ -5325,10 +5338,22 @@ synthesizeBaseClassFieldGetterOrAddressGetterBody(AbstractFunctionDecl *afd,
 
   auto *baseMemberCallExpr =
       CallExpr::createImplicit(ctx, baseMemberDotCallExpr, argumentList);
-  baseMemberCallExpr->setType(baseGetterMethod->getResultInterfaceType());
+  Type resultType = baseGetterMethod->getResultInterfaceType();
+  if (kind == AccessorKind::Address && resultType->isUnsafeMutablePointer()) {
+    resultType = getterDecl->getResultInterfaceType();
+  }
+  baseMemberCallExpr->setType(resultType);
   baseMemberCallExpr->setThrows(nullptr);
 
-  auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), baseMemberCallExpr,
+  Expr *returnExpr = baseMemberCallExpr;
+  // Cast an 'address' result from a mutable pointer if needed.
+  if (kind == AccessorKind::Address &&
+      baseGetterMethod->getResultInterfaceType()->isUnsafeMutablePointer())
+    returnExpr = SwiftDeclSynthesizer::synthesizeReturnReinterpretCast(
+        ctx, baseGetterMethod->getResultInterfaceType(), resultType,
+        returnExpr);
+
+  auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), returnExpr,
                                          /*implicit=*/true);
 
   auto body = BraceStmt::create(ctx, SourceLoc(), {returnStmt}, SourceLoc(),
@@ -5613,10 +5638,11 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
                          ? StorageIsNotMutable : StorageIsMutable;
     out->setImplInfo(
         accessors[0]->getAccessorKind() == AccessorKind::Address
-            ? (accessors[1] ? StorageImplInfo(ReadImplKind::Address,
-                                              WriteImplKind::MutableAddress,
-                                              ReadWriteImplKind::MutableAddress)
-                            : StorageImplInfo(ReadImplKind::Address))
+            ? (accessors.size() > 1
+                   ? StorageImplInfo(ReadImplKind::Address,
+                                     WriteImplKind::MutableAddress,
+                                     ReadWriteImplKind::MutableAddress)
+                   : StorageImplInfo(ReadImplKind::Address))
             : StorageImplInfo::getComputed(isMutable));
     out->setIsSetterMutating(true);
     return out;

lib/ClangImporter/SwiftDeclSynthesizer.cpp

@@ -1496,6 +1496,29 @@ AccessorDecl *SwiftDeclSynthesizer::buildSubscriptSetterDecl(
 
 // MARK: C++ subscripts
 
+Expr *SwiftDeclSynthesizer::synthesizeReturnReinterpretCast(ASTContext &ctx,
+                                                            Type givenType,
+                                                            Type exprType,
+                                                            Expr *baseExpr) {
+  auto reinterpretCast = cast<FuncDecl>(
+      getBuiltinValueDecl(ctx, ctx.getIdentifier("reinterpretCast")));
+  SubstitutionMap subMap = SubstitutionMap::get(
+      reinterpretCast->getGenericSignature(), {givenType, exprType}, {});
+  ConcreteDeclRef concreteDeclRef(reinterpretCast, subMap);
+  auto reinterpretCastRef =
+      new (ctx) DeclRefExpr(concreteDeclRef, DeclNameLoc(), /*implicit*/ true);
+  FunctionType::ExtInfo info;
+  reinterpretCastRef->setType(
+      FunctionType::get({FunctionType::Param(givenType)}, exprType, info));
+
+  auto *argList = ArgumentList::forImplicitUnlabeled(ctx, {baseExpr});
+  auto reinterpreted =
+      CallExpr::createImplicit(ctx, reinterpretCastRef, argList);
+  reinterpreted->setType(exprType);
+  reinterpreted->setThrows(nullptr);
+  return reinterpreted;
+}
+
 /// Synthesizer callback for a subscript getter or a getter for a
 /// dereference property (`var pointee`). If the getter's implementation returns
 /// an UnsafePointer or UnsafeMutablePointer, it unwraps the pointer and returns
@@ -1544,27 +1567,9 @@ synthesizeUnwrappingGetterOrAddressGetterBody(AbstractFunctionDecl *afd,
   }
   // Cast an 'address' result from a mutable pointer if needed.
   if (isAddress &&
-      getterImpl->getResultInterfaceType()->isUnsafeMutablePointer()) {
-    auto reinterpretCast = cast<FuncDecl>(
-        getBuiltinValueDecl(ctx, ctx.getIdentifier("reinterpretCast")));
-    auto rawTy = getterImpl->getResultInterfaceType();
-    auto enumTy = elementTy;
-    SubstitutionMap subMap = SubstitutionMap::get(
-        reinterpretCast->getGenericSignature(), {rawTy, enumTy}, {});
-    ConcreteDeclRef concreteDeclRef(reinterpretCast, subMap);
-    auto reinterpretCastRef = new (ctx)
-        DeclRefExpr(concreteDeclRef, DeclNameLoc(), /*implicit*/ true);
-    FunctionType::ExtInfo info;
-    reinterpretCastRef->setType(
-        FunctionType::get({FunctionType::Param(rawTy)}, enumTy, info));
-
-    auto *argList = ArgumentList::forImplicitUnlabeled(ctx, {propertyExpr});
-    auto reinterpreted =
-        CallExpr::createImplicit(ctx, reinterpretCastRef, argList);
-    reinterpreted->setType(enumTy);
-    reinterpreted->setThrows(nullptr);
-    propertyExpr = reinterpreted;
-  }
+      getterImpl->getResultInterfaceType()->isUnsafeMutablePointer())
+    propertyExpr = SwiftDeclSynthesizer::synthesizeReturnReinterpretCast(
+        ctx, getterImpl->getResultInterfaceType(), elementTy, propertyExpr);
 
   auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), propertyExpr,
                                          /*implicit*/ true);

lib/ClangImporter/SwiftDeclSynthesizer.h

@@ -60,6 +60,10 @@ class SwiftDeclSynthesizer {
                        VarDecl::Introducer introducer, bool isImplicit,
                        AccessLevel access, AccessLevel setterAccess);
 
+  /// Create a reinterpretCast from the `exprType`, to the `givenType`.
+  static Expr *synthesizeReturnReinterpretCast(ASTContext &ctx, Type givenType,
+                                               Type exprType, Expr *baseExpr);
+
   /// Create a new named constant with the given value.
   ///
   /// \param name The name of the constant.

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

@@ -60,4 +60,18 @@ class NonCopyableHolderValueMutDeref {
     inline NonCopyable operator *() { return NonCopyable(x.x); }
 };
 
+template<class T>
+class OneDerived: public T {
+public:
+    OneDerived(int x) : T(x) {}
+};
+
+using NonCopyableHolderConstDerefDerivedDerived = OneDerived<OneDerived<NonCopyableHolderConstDeref>>;
+
+using NonCopyableHolderPairedDerefDerivedDerived = OneDerived<OneDerived<NonCopyableHolderPairedDeref>>;
+
+using NonCopyableHolderMutDerefDerivedDerived = OneDerived<OneDerived<NonCopyableHolderMutDeref>>;
+
+using NonCopyableHolderValueConstDerefDerivedDerived = OneDerived<OneDerived<NonCopyableHolderValueConstDeref>>;
+
 #endif // TEST_INTEROP_CXX_OPERATORS_MOVE_ONLY_OPS_H

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

@@ -81,4 +81,52 @@ MoveOnlyCxxOperators.test("testNonCopyableHolderValueMutDeref pointee value") {
   expectEqual(k.x, k2.x)
 }
 
+MoveOnlyCxxOperators.test("NonCopyableHolderConstDerefDerivedDerived pointee borrow") {
+  let holder = NonCopyableHolderConstDerefDerivedDerived(11)
+  var k = borrowNC(holder.pointee)
+  expectEqual(k, 33)
+  k = holder.pointee.method(2)
+  expectEqual(k, 22)
+  k = holder.pointee.x
+  expectEqual(k, 11)
+}
+
+MoveOnlyCxxOperators.test("testNonCopyableHolderPairedDerefDerivedDerived pointee borrow") {
+  var holder = NonCopyableHolderPairedDerefDerivedDerived(11)
+  var k = borrowNC(holder.pointee)
+  expectEqual(k, 33)
+  k = holder.pointee.method(2)
+  expectEqual(k, 22)
+  k = holder.pointee.x
+  expectEqual(k, 11)
+  k = inoutNC(&holder.pointee, -1)
+  expectEqual(k, -1)
+  expectEqual(holder.pointee.x, -1)
+  holder.pointee.mutMethod(3)
+  expectEqual(holder.pointee.x, 3)
+  holder.pointee.x = 34
+  expectEqual(holder.pointee.x, 34)
+  consumingNC(holder.pointee)
+  expectEqual(holder.pointee.x, 0)
+}
+
+MoveOnlyCxxOperators.test("testNonCopyableHolderMutDerefDerivedDerived pointee borrow") {
+  var holder = NonCopyableHolderMutDerefDerivedDerived(11)
+  var k = borrowNC(holder.pointee)
+  expectEqual(k, 33)
+  k = holder.pointee.method(2)
+  expectEqual(k, 22)
+  k = holder.pointee.x
+  expectEqual(k, 11)
+  k = inoutNC(&holder.pointee, -1)
+  expectEqual(k, -1)
+  expectEqual(holder.pointee.x, -1)
+  holder.pointee.mutMethod(3)
+  expectEqual(holder.pointee.x, 3)
+  holder.pointee.x = 34
+  expectEqual(holder.pointee.x, 34)
+  consumingNC(holder.pointee)
+  expectEqual(holder.pointee.x, 0)
+}
+
 runAllTests()