[cxx-interop] Use a synthesized C++ method when accessing a base field or subscript from a derived class synthesized method

The use of a synthesized C++ method allows us to avoid making a copy of self when accessing the base field or subscript from Swift

Author: hyp

lib/ClangImporter/ClangImporter.cpp

@@ -4768,24 +4768,63 @@ MemberRefExpr *getSelfInteropStaticCast(FuncDecl *funcDecl,
 // to the base class during the invocation of the method.
 static clang::CXXMethodDecl *synthesizeCxxBaseMethod(
     ClangImporter &impl, const clang::CXXRecordDecl *derivedClass,
-    const clang::CXXRecordDecl *baseClass, const clang::CXXMethodDecl *method) {
+    const clang::CXXRecordDecl *baseClass, const clang::CXXMethodDecl *method,
+    bool promoteReferenceReturnToValue = false,
+    bool forceConstQualifier = false) {
   auto &clangCtx = impl.getClangASTContext();
   auto &clangSema = impl.getClangSema();
 
   // Create a new method in the derived class that calls the base method.
-  auto name = method->getNameInfo().getName();
+  clang::DeclarationName name = method->getNameInfo().getName();
   if (name.isIdentifier()) {
     std::string newName;
     llvm::raw_string_ostream os(newName);
     os << "__synthesizedBaseCall_" << name.getAsIdentifierInfo()->getName();
     name = clang::DeclarationName(
         &impl.getClangPreprocessor().getIdentifierTable().get(os.str()));
+  } else if (name.getCXXOverloadedOperator() == clang::OO_Subscript) {
+    name = clang::DeclarationName(
+        &impl.getClangPreprocessor().getIdentifierTable().get(
+            "__synthesizedBaseCall_operatorSubscript"));
+  } else if (name.getCXXOverloadedOperator() == clang::OO_Star) {
+    name = clang::DeclarationName(
+        &impl.getClangPreprocessor().getIdentifierTable().get(
+            "__synthesizedBaseCall_operatorStar"));
+  }
+  auto methodType = method->getType();
+  // Check if we need to drop the reference from the return type
+  // of the new method. This is needed when a synthesized `operator []`
+  // derived-to-base call is invoked from Swift's subscript getter.
+  if (promoteReferenceReturnToValue) {
+    if (const auto *fpt = methodType->getAs<clang::FunctionProtoType>()) {
+      auto retType = fpt->getReturnType();
+      if (retType->isReferenceType()) {
+        methodType = clangCtx.getFunctionType(retType->getPointeeType(),
+                                              fpt->getParamTypes(),
+                                              fpt->getExtProtoInfo());
+      }
+    }
+  }
+  // Check if this method requires an additional `const` qualifier.
+  // This might needed when a non-const synthesized `operator []`
+  // derived-to-base call is invoked from Swift's subscript getter.
+  bool castThisToNonConstThis = false;
+  if (forceConstQualifier) {
+    if (const auto *fpt = methodType->getAs<clang::FunctionProtoType>()) {
+      auto info = fpt->getExtProtoInfo();
+      if (!info.TypeQuals.hasConst()) {
+        info.TypeQuals.addConst();
+        castThisToNonConstThis = true;
+        methodType = clangCtx.getFunctionType(fpt->getReturnType(),
+                                              fpt->getParamTypes(), info);
+      }
+    }
   }
   auto newMethod = clang::CXXMethodDecl::Create(
       clangCtx, const_cast<clang::CXXRecordDecl *>(derivedClass),
       method->getSourceRange().getBegin(),
-      clang::DeclarationNameInfo(name, clang::SourceLocation()),
-      method->getType(), method->getTypeSourceInfo(), method->getStorageClass(),
+      clang::DeclarationNameInfo(name, clang::SourceLocation()), methodType,
+      method->getTypeSourceInfo(), method->getStorageClass(),
       method->UsesFPIntrin(), /*isInline=*/true, method->getConstexprKind(),
       method->getSourceRange().getEnd());
   newMethod->setImplicit();
@@ -4810,8 +4849,23 @@ static clang::CXXMethodDecl *synthesizeCxxBaseMethod(
   auto diagState = clangSema.DelayedDiagnostics.push(diagPool);
 
   // Construct the method's body.
-  auto *thisExpr = new (clangCtx) clang::CXXThisExpr(
+  clang::Expr *thisExpr = new (clangCtx) clang::CXXThisExpr(
       clang::SourceLocation(), newMethod->getThisType(), /*IsImplicit=*/false);
+  if (castThisToNonConstThis) {
+    auto baseClassPtr =
+        clangCtx.getPointerType(clangCtx.getRecordType(derivedClass));
+    clang::CastKind Kind;
+    clang::CXXCastPath Path;
+    clangSema.CheckPointerConversion(thisExpr, baseClassPtr, Kind, Path,
+                                     /*IgnoreBaseAccess=*/false,
+                                     /*Diagnose=*/true);
+    auto conv = clangSema.ImpCastExprToType(thisExpr, baseClassPtr, Kind,
+                                            clang::VK_PRValue, &Path);
+    if (!conv.isUsable())
+      return nullptr;
+    thisExpr = conv.get();
+  }
+
   auto memberExpr = clangSema.BuildMemberExpr(
       thisExpr, /*isArrow=*/true, clang::SourceLocation(),
       clang::NestedNameSpecifierLoc(), clang::SourceLocation(),
@@ -4865,7 +4919,13 @@ const clang::CXXMethodDecl *getCalledBaseCxxMethod(FuncDecl *baseMember) {
       body->getElements().front().dyn_cast<Stmt *>());
   if (!returnStmt)
     return nullptr;
-  auto *callExpr = dyn_cast<CallExpr>(returnStmt->getResult());
+  Expr *returnExpr = returnStmt->getResult();
+  // A member ref expr for `.pointee` access can be wrapping a call
+  // when looking through the synthesized Swift body for `.pointee`
+  // accessor.
+  if (MemberRefExpr *mre = dyn_cast<MemberRefExpr>(returnExpr))
+    returnExpr = mre->getBase();
+  auto *callExpr = dyn_cast<CallExpr>(returnExpr);
   if (!callExpr)
     return nullptr;
   auto *cv = callExpr->getCalledValue();
@@ -4966,9 +5026,94 @@ synthesizeBaseClassMethodBody(AbstractFunctionDecl *afd, void *context) {
   return {body, /*isTypeChecked=*/true};
 }
 
-// Getters are relatively easy. Just cast and return the member:
-//   %0 = __swift_interopStaticCast<Base>(self)
-//   return %0.member
+// Synthesize a C++ method that returns the field of interest from the base
+// class. This lets Clang take care of the cast from the derived class
+// to the base class while the field is accessed.
+static clang::CXXMethodDecl *synthesizeCxxBaseGetterAccessorMethod(
+    ClangImporter &impl, const clang::CXXRecordDecl *derivedClass,
+    const clang::CXXRecordDecl *baseClass, const clang::FieldDecl *field) {
+  auto &clangCtx = impl.getClangASTContext();
+  auto &clangSema = impl.getClangSema();
+
+  // Create a new method in the derived class that calls the base method.
+  auto name = field->getDeclName();
+  if (name.isIdentifier()) {
+    std::string newName;
+    llvm::raw_string_ostream os(newName);
+    os << "__synthesizedBaseGetterAccessor_"
+       << name.getAsIdentifierInfo()->getName();
+    name = clang::DeclarationName(
+        &impl.getClangPreprocessor().getIdentifierTable().get(os.str()));
+  }
+  auto returnType = field->getType();
+  if (returnType->isReferenceType())
+    returnType = returnType->getPointeeType();
+  clang::FunctionProtoType::ExtProtoInfo info;
+  info.TypeQuals.addConst();
+  info.ExceptionSpec.Type = clang::EST_NoThrow;
+  auto ftype = clangCtx.getFunctionType(returnType, {}, info);
+  auto newMethod = clang::CXXMethodDecl::Create(
+      clangCtx, const_cast<clang::CXXRecordDecl *>(derivedClass),
+      field->getSourceRange().getBegin(),
+      clang::DeclarationNameInfo(name, clang::SourceLocation()), ftype,
+      clangCtx.getTrivialTypeSourceInfo(ftype), clang::SC_None,
+      /*UsesFPIntrin=*/false, /*isInline=*/true,
+      clang::ConstexprSpecKind::Unspecified, field->getSourceRange().getEnd());
+  newMethod->setImplicit();
+  newMethod->setImplicitlyInline();
+  newMethod->setAccess(clang::AccessSpecifier::AS_public);
+
+  // Create a new Clang diagnostic pool to capture any diagnostics
+  // emitted during the construction of the method.
+  clang::sema::DelayedDiagnosticPool diagPool{
+      clangSema.DelayedDiagnostics.getCurrentPool()};
+  auto diagState = clangSema.DelayedDiagnostics.push(diagPool);
+
+  // Construct the method's body.
+  auto *thisExpr = new (clangCtx) clang::CXXThisExpr(
+      clang::SourceLocation(), newMethod->getThisType(), /*IsImplicit=*/false);
+  clang::QualType baseClassPtr = clangCtx.getRecordType(baseClass);
+  baseClassPtr.addConst();
+  baseClassPtr = clangCtx.getPointerType(baseClassPtr);
+
+  clang::CastKind Kind;
+  clang::CXXCastPath Path;
+  clangSema.CheckPointerConversion(thisExpr, baseClassPtr, Kind, Path,
+                                   /*IgnoreBaseAccess=*/false,
+                                   /*Diagnose=*/true);
+  auto conv = clangSema.ImpCastExprToType(thisExpr, baseClassPtr, Kind,
+                                          clang::VK_PRValue, &Path);
+  if (!conv.isUsable())
+    return nullptr;
+  auto memberExpr = clangSema.BuildMemberExpr(
+      conv.get(), /*isArrow=*/true, clang::SourceLocation(),
+      clang::NestedNameSpecifierLoc(), clang::SourceLocation(),
+      const_cast<clang::FieldDecl *>(field),
+      clang::DeclAccessPair::make(const_cast<clang::FieldDecl *>(field),
+                                  clang::AS_public),
+      /*HadMultipleCandidates=*/false,
+      clang::DeclarationNameInfo(field->getDeclName(), clang::SourceLocation()),
+      returnType, clang::VK_LValue, clang::OK_Ordinary);
+  auto returnCast = clangSema.ImpCastExprToType(
+      memberExpr, returnType, clang::CK_LValueToRValue, clang::VK_PRValue);
+  if (!returnCast.isUsable())
+    return nullptr;
+  auto returnStmt = clang::ReturnStmt::Create(clangCtx, clang::SourceLocation(),
+                                              returnCast.get(), nullptr);
+
+  // Check if there were any Clang errors during the construction
+  // of the method body.
+  clangSema.DelayedDiagnostics.popWithoutEmitting(diagState);
+  if (!diagPool.empty())
+    return nullptr;
+  newMethod->setBody(returnStmt);
+  return newMethod;
+}
+
+// Generates the body of a derived method, that invokes the base
+// field getter or the base subscript.
+// The method's body takes the following form:
+//   return self.__synthesizedBaseCall_fn(args...)
 static std::pair<BraceStmt *, bool>
 synthesizeBaseClassFieldGetterBody(AbstractFunctionDecl *afd, void *context) {
   ASTContext &ctx = afd->getASTContext();
@@ -4980,48 +5125,80 @@ synthesizeBaseClassFieldGetterBody(AbstractFunctionDecl *afd, void *context) {
   NominalTypeDecl *derivedStruct =
       cast<NominalTypeDecl>(getterDecl->getDeclContext()->getAsDecl());
 
+  const clang::Decl *baseClangDecl;
+  if (baseClassVar->getClangDecl())
+    baseClangDecl = baseClassVar->getClangDecl();
+  else
+    baseClangDecl =
+        getCalledBaseCxxMethod(baseClassVar->getAccessor(AccessorKind::Get));
+
+  clang::CXXMethodDecl *baseGetterCxxMethod = nullptr;
+  if (auto *md = dyn_cast_or_null<clang::CXXMethodDecl>(baseClangDecl)) {
+    // Subscript operator, or `.pointee` wrapper is represented through a
+    // generated C++ method call that calls the base operator.
+    baseGetterCxxMethod = synthesizeCxxBaseMethod(
+        *static_cast<ClangImporter *>(ctx.getClangModuleLoader()),
+        cast<clang::CXXRecordDecl>(derivedStruct->getClangDecl()),
+        cast<clang::CXXRecordDecl>(baseStruct->getClangDecl()), md,
+        /*promoteReferenceReturnToValue=*/true,
+        /*forceConstQualifier=*/true);
+  } else if (auto *fd = dyn_cast_or_null<clang::FieldDecl>(baseClangDecl)) {
+    // Field getter is represented through a generated
+    // C++ method call that returns the value of the base field.
+    baseGetterCxxMethod = synthesizeCxxBaseGetterAccessorMethod(
+        *static_cast<ClangImporter *>(ctx.getClangModuleLoader()),
+        cast<clang::CXXRecordDecl>(derivedStruct->getClangDecl()),
+        cast<clang::CXXRecordDecl>(baseStruct->getClangDecl()), fd);
+  }
+
+  if (!baseGetterCxxMethod) {
+    ctx.Diags.diagnose(SourceLoc(), diag::failed_base_method_call_synthesis,
+                       getterDecl, baseStruct);
+    auto body = BraceStmt::create(ctx, SourceLoc(), {}, SourceLoc(),
+                                  /*implicit=*/true);
+    return {body, true};
+  }
+  auto *baseGetterMethod = cast<FuncDecl>(
+      ctx.getClangModuleLoader()->importDeclDirectly(baseGetterCxxMethod));
+
   auto selfDecl = getterDecl->getImplicitSelfDecl();
   auto selfExpr = new (ctx) DeclRefExpr(selfDecl, DeclNameLoc(),
                                         /*implicit*/ true);
   selfExpr->setType(selfDecl->getTypeInContext());
 
-  auto staticCastRefExpr = getInteropStaticCastDeclRefExpr(
-      ctx, baseStruct->getClangDecl()->getOwningModule(),
-      baseStruct->getSelfInterfaceType(),
-      derivedStruct->getSelfInterfaceType());
+  Argument selfArg = Argument::unlabeled(selfExpr);
 
-  auto *argList = ArgumentList::forImplicitUnlabeled(ctx, {selfExpr});
-  auto casted = CallExpr::createImplicit(ctx, staticCastRefExpr, argList);
-  casted->setType(baseStruct->getSelfInterfaceType());
-  casted->setThrows(false);
+  auto *baseMemberExpr =
+      new (ctx) DeclRefExpr(ConcreteDeclRef(baseGetterMethod), DeclNameLoc(),
+                            /*Implicit=*/true);
+  baseMemberExpr->setType(baseGetterMethod->getInterfaceType());
+
+  auto baseMemberDotCallExpr =
+      DotSyntaxCallExpr::create(ctx, baseMemberExpr, SourceLoc(), selfArg);
+  baseMemberDotCallExpr->setType(baseGetterMethod->getMethodInterfaceType());
+  baseMemberDotCallExpr->setThrows(false);
 
-  Expr *baseMember = nullptr;
+  ArgumentList *argumentList;
   if (auto subscript = dyn_cast<SubscriptDecl>(baseClassVar)) {
     auto paramDecl = getterDecl->getParameters()->get(0);
-    auto paramRefExpr = new (ctx) DeclRefExpr(paramDecl,
-                                              DeclNameLoc(),
-                                              /*Implicit=*/ true);
+    auto paramRefExpr = new (ctx) DeclRefExpr(paramDecl, DeclNameLoc(),
+                                              /*Implicit=*/true);
     paramRefExpr->setType(paramDecl->getTypeInContext());
-
-    auto *argList = ArgumentList::forImplicitUnlabeled(ctx, {paramRefExpr});
-    baseMember = SubscriptExpr::create(ctx, casted, argList, subscript);
-    baseMember->setType(subscript->getElementInterfaceType());
+    argumentList = ArgumentList::forImplicitUnlabeled(ctx, {paramRefExpr});
   } else {
-    // If the base class var has a clang decl, that means it's an access into a
-    // stored field. Otherwise, we're looking into another base class, so it's a
-    // another synthesized accessor.
-    AccessSemantics accessKind = baseClassVar->getClangDecl()
-                                     ? AccessSemantics::DirectToStorage
-                                     : AccessSemantics::DirectToImplementation;
-    baseMember =
-        new (ctx) MemberRefExpr(casted, SourceLoc(), baseClassVar, DeclNameLoc(),
-                                /*Implicit=*/true, accessKind);
-    baseMember->setType(cast<VarDecl>(baseClassVar)->getTypeInContext());
+    argumentList = ArgumentList::forImplicitUnlabeled(ctx, {});
   }
 
-  auto ret = new (ctx) ReturnStmt(SourceLoc(), baseMember);
-  auto body = BraceStmt::create(ctx, SourceLoc(), {ret}, SourceLoc(),
-                                /*implicit*/ true);
+  auto *baseMemberCallExpr =
+      CallExpr::createImplicit(ctx, baseMemberDotCallExpr, argumentList);
+  baseMemberCallExpr->setType(baseGetterMethod->getResultInterfaceType());
+  baseMemberCallExpr->setThrows(false);
+
+  auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), baseMemberCallExpr,
+                                         /*implicit=*/true);
+
+  auto body = BraceStmt::create(ctx, SourceLoc(), {returnStmt}, SourceLoc(),
+                                /*implicit=*/true);
   return {body, /*isTypeChecked=*/true};
 }
 

test/Interop/Cxx/class/inheritance/Inputs/fields.h

@@ -66,3 +66,37 @@ struct ClassTemplate {
 };
 
 struct DerivedFromClassTemplate : ClassTemplate<int> {};
+
+int &getCopyCounter() {
+    static int copyCounter = 0;
+    return copyCounter;
+}
+
+class CopyTrackedBaseClass {
+public:
+    CopyTrackedBaseClass(int x) : x(x) {}
+    CopyTrackedBaseClass(const CopyTrackedBaseClass &other) : x(other.x) {
+        ++getCopyCounter();
+    }
+
+    int x;
+};
+
+class CopyTrackedDerivedClass: public CopyTrackedBaseClass {
+public:
+    CopyTrackedDerivedClass(int x) : CopyTrackedBaseClass(x) {}
+};
+
+class NonEmptyBase {
+public:
+    int getY() const {
+        return y;
+    }
+private:
+    int y = 11;
+};
+
+class CopyTrackedDerivedDerivedClass: public NonEmptyBase, public CopyTrackedDerivedClass {
+public:
+    CopyTrackedDerivedDerivedClass(int x) : CopyTrackedDerivedClass(x) {}
+};

test/Interop/Cxx/class/inheritance/fields.swift

@@ -85,4 +85,31 @@ FieldsTestSuite.test("Same field from derived") {
   expectEqual(derived.a, 42)
 }
 
+extension CopyTrackedBaseClass {
+    var swiftProp: CInt {
+        return x
+    }
+}
+
+FieldsTestSuite.test("Get field without copying base in the getter accessor") {
+  let base = CopyTrackedBaseClass(0)
+  var copyCounter = getCopyCounter().pointee
+  expectEqual(base.swiftProp, 0)
+  // Measure copy counter of a regular
+  // property access for a C++ type to compare
+  // it to see if any additional copies are
+  // needed to access the property from the base class.
+  let expectedCopyCountDiff = getCopyCounter().pointee - copyCounter
+
+  let derived = CopyTrackedDerivedClass(234)
+  copyCounter = getCopyCounter().pointee
+  expectEqual(derived.x, 234)
+  expectEqual(copyCounter, getCopyCounter().pointee - expectedCopyCountDiff)
+
+  let derivedDerived = CopyTrackedDerivedDerivedClass(-11)
+  copyCounter = getCopyCounter().pointee
+  expectEqual(derivedDerived.x, -11)
+  expectEqual(copyCounter, getCopyCounter().pointee - expectedCopyCountDiff)
+}
+
 runAllTests()