Merge pull request #71994 from apple/egorzhdan/virtual-methods

[cxx-interop] Overhaul virtual method support

Author: egorzhdan

include/swift/ClangImporter/ClangImporter.h

@@ -131,6 +131,7 @@ typedef llvm::PointerUnion<const clang::Decl *, const clang::MacroInfo *,
 /// from Clang ASTs over to Swift ASTs.
 class ClangImporter final : public ClangModuleLoader {
   friend class ClangModuleUnit;
+  friend class SwiftDeclSynthesizer;
 
   // Make requests in the ClangImporter zone friends so they can access `Impl`.
 #define SWIFT_REQUEST(Zone, Name, Sig, Caching, LocOptions)                    \

lib/ClangImporter/ClangImporter.cpp

@@ -4891,187 +4891,6 @@ MemberRefExpr *getSelfInteropStaticCast(FuncDecl *funcDecl,
   return pointeePropertyRefExpr;
 }
 
-enum class ReferenceReturnTypeBehaviorForBaseMethodSynthesis {
-  KeepReference,
-  RemoveReference,
-  RemoveReferenceIfPointer,
-};
-
-// Synthesize a C++ method that invokes the method from the base
-// class. This lets Clang take care of the cast from the derived class
-// 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,
-    ReferenceReturnTypeBehaviorForBaseMethodSynthesis
-        referenceReturnTypeBehavior =
-            ReferenceReturnTypeBehaviorForBaseMethodSynthesis::KeepReference,
-    bool forceConstQualifier = false,
-    bool isVirtualCall = false) {
-  auto &clangCtx = impl.getClangASTContext();
-  auto &clangSema = impl.getClangSema();
-  // When emitting symbolic decls, the method might not have a concrete
-  // record type as this type.
-  if (impl.isSymbolicImportEnabled()
-      && !method->getThisType()->getPointeeCXXRecordDecl()) {
-    return nullptr;
-  }
-
-  // Create a new method in the derived class that calls the base method.
-  clang::DeclarationName name = method->getNameInfo().getName();
-  if (name.isIdentifier()) {
-    std::string newName;
-    llvm::raw_string_ostream os(newName);
-    os << (isVirtualCall ? "__synthesizedVirtualCall_" :
-                           "__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(
-            (isVirtualCall ? "__synthesizedVirtualCall_operatorSubscript" :
-                             "__synthesizedBaseCall_operatorSubscript")));
-  } else if (name.getCXXOverloadedOperator() == clang::OO_Star) {
-    name = clang::DeclarationName(
-        &impl.getClangPreprocessor().getIdentifierTable().get(
-            (isVirtualCall ? "__synthesizedVirtualCall_operatorStar" :
-                             "__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 (referenceReturnTypeBehavior !=
-      ReferenceReturnTypeBehaviorForBaseMethodSynthesis::KeepReference) {
-    if (const auto *fpt = methodType->getAs<clang::FunctionProtoType>()) {
-      auto retType = fpt->getReturnType();
-      if (retType->isReferenceType() &&
-          (referenceReturnTypeBehavior ==
-               ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
-                   RemoveReference ||
-           (referenceReturnTypeBehavior ==
-                ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
-                    RemoveReferenceIfPointer &&
-            retType->getPointeeType()->isPointerType()))) {
-        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()), methodType,
-      method->getTypeSourceInfo(), method->getStorageClass(),
-      method->UsesFPIntrin(), /*isInline=*/true, method->getConstexprKind(),
-      method->getSourceRange().getEnd());
-  newMethod->setImplicit();
-  newMethod->setImplicitlyInline();
-  newMethod->setAccess(clang::AccessSpecifier::AS_public);
-  if (method->hasAttr<clang::CFReturnsRetainedAttr>()) {
-    // Return an FRT field at +1 if the base method also follows this
-    // convention.
-    newMethod->addAttr(clang::CFReturnsRetainedAttr::CreateImplicit(clangCtx));
-  }
-
-  llvm::SmallVector<clang::ParmVarDecl *, 4> params;
-  for (size_t i = 0; i < method->getNumParams(); ++i) {
-    const auto &param = *method->getParamDecl(i);
-    params.push_back(clang::ParmVarDecl::Create(
-        clangCtx, newMethod, param.getSourceRange().getBegin(),
-        param.getLocation(), param.getIdentifier(), param.getType(),
-        param.getTypeSourceInfo(), param.getStorageClass(),
-        /*DefExpr=*/nullptr));
-  }
-  newMethod->setParams(params);
-
-  // 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.
-  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(),
-      const_cast<clang::CXXMethodDecl *>(method),
-      clang::DeclAccessPair::make(const_cast<clang::CXXMethodDecl *>(method),
-                                  clang::AS_public),
-      /*HadMultipleCandidates=*/false, method->getNameInfo(),
-      clangCtx.BoundMemberTy, clang::VK_PRValue, clang::OK_Ordinary);
-  llvm::SmallVector<clang::Expr *, 4> args;
-  for (size_t i = 0; i < newMethod->getNumParams(); ++i) {
-    auto *param = newMethod->getParamDecl(i);
-    auto type = param->getType();
-    if (type->isReferenceType())
-      type = type->getPointeeType();
-    args.push_back(new (clangCtx) clang::DeclRefExpr(
-        clangCtx, param, false, type, clang::ExprValueKind::VK_LValue,
-        clang::SourceLocation()));
-  }
-  auto memberCall = clangSema.BuildCallToMemberFunction(
-      nullptr, memberExpr, clang::SourceLocation(), args,
-      clang::SourceLocation());
-  if (!memberCall.isUsable())
-    return nullptr;
-  auto returnStmt = clang::ReturnStmt::Create(clangCtx, clang::SourceLocation(),
-                                              memberCall.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;
-}
-
-// Synthesize a C++ virtual method
-clang::CXXMethodDecl *synthesizeCxxVirtualMethod(
-    swift::ClangImporter &Impl, const clang::CXXRecordDecl *derivedClass,
-    const clang::CXXRecordDecl *baseClass, const clang::CXXMethodDecl *method) {
-  return synthesizeCxxBaseMethod(
-      Impl, derivedClass, baseClass, method,
-      ReferenceReturnTypeBehaviorForBaseMethodSynthesis::KeepReference,
-      false /* forceConstQualifier */, true /* isVirtualCall */);
-}
-
 // Find the base C++ method called by the base function we want to synthesize
 // the derived thunk for.
 // The base C++ method is either the original C++ method that corresponds
@@ -5127,9 +4946,11 @@ FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl, ASTContext &ctx,
   auto *cxxMethod = getCalledBaseCxxMethod(baseMember);
   if (!cxxMethod)
     return nullptr;
-  auto *newClangMethod = synthesizeCxxBaseMethod(
-      impl, cast<clang::CXXRecordDecl>(derivedStruct->getClangDecl()),
-      cast<clang::CXXRecordDecl>(baseStruct->getClangDecl()), cxxMethod);
+  auto *newClangMethod =
+      SwiftDeclSynthesizer(&impl).synthesizeCXXForwardingMethod(
+          cast<clang::CXXRecordDecl>(derivedStruct->getClangDecl()),
+          cast<clang::CXXRecordDecl>(baseStruct->getClangDecl()), cxxMethod,
+          ForwardingMethodKind::Base);
   if (!newClangMethod)
     return nullptr;
   return cast_or_null<FuncDecl>(
@@ -5388,19 +5209,22 @@ synthesizeBaseClassFieldGetterOrAddressGetterBody(AbstractFunctionDecl *afd,
   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,
-        getterDecl->getResultInterfaceType()->isForeignReferenceType()
-            ? ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
-                  RemoveReferenceIfPointer
-            : (kind != AccessorKind::Get
-                   ? ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
-                         KeepReference
-                   : ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
-                         RemoveReference),
-        /*forceConstQualifier=*/kind != AccessorKind::MutableAddress);
+    baseGetterCxxMethod =
+        SwiftDeclSynthesizer(
+            static_cast<ClangImporter *>(ctx.getClangModuleLoader()))
+            .synthesizeCXXForwardingMethod(
+                cast<clang::CXXRecordDecl>(derivedStruct->getClangDecl()),
+                cast<clang::CXXRecordDecl>(baseStruct->getClangDecl()), md,
+                ForwardingMethodKind::Base,
+                getterDecl->getResultInterfaceType()->isForeignReferenceType()
+                    ? ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
+                          RemoveReferenceIfPointer
+                    : (kind != AccessorKind::Get
+                           ? ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
+                                 KeepReference
+                           : ReferenceReturnTypeBehaviorForBaseMethodSynthesis::
+                                 RemoveReference),
+                /*forceConstQualifier=*/kind != AccessorKind::MutableAddress);
   } else if (auto *fd = dyn_cast_or_null<clang::FieldDecl>(baseClangDecl)) {
     ValueDecl *retainOperationFn = nullptr;
     // Check if this field getter is returning a retainable FRT.
@@ -6881,7 +6705,7 @@ static ValueDecl *addThunkForDependentTypes(FuncDecl *oldDecl,
 // are not used in the function signature. We supply the type params as explicit
 // metatype arguments to aid in typechecking, but they shouldn't be forwarded to
 // the corresponding C++ function.
-std::pair<BraceStmt *, bool>
+static std::pair<BraceStmt *, bool>
 synthesizeForwardingThunkBody(AbstractFunctionDecl *afd, void *context) {
   ASTContext &ctx = afd->getASTContext();
 

lib/ClangImporter/ImportDecl.cpp

@@ -3753,38 +3753,42 @@ namespace {
         }
       }
 
-      if (decl->isVirtual() && isa_and_nonnull<ValueDecl>(method)) {
-        if (Impl.isCxxInteropCompatVersionAtLeast(6)) {
-          if (auto dc = method->getDeclContext();
-              !decl->isPure() &&
-              isa_and_nonnull<NominalTypeDecl>(dc->getAsDecl())) {
-
-            // generates the __synthesizedVirtualCall_ C++ thunk
-            clang::CXXMethodDecl *cxxThunk = synthesizeCxxVirtualMethod(
-                *static_cast<ClangImporter *>(
-                    dc->getASTContext().getClangModuleLoader()),
-                decl->getParent(), decl->getParent(), decl);
-
-            // call the __synthesizedVirtualCall_ C++ thunk from a Swift thunk
-            if (Decl *swiftThunk =
-                    cxxThunk ? VisitCXXMethodDecl(cxxThunk) : nullptr;
-                isa_and_nonnull<FuncDecl>(swiftThunk)) {
-              // synthesize the body of the Swift method to call the swiftThunk
-              synthesizeForwardingThunkBody(cast<FuncDecl>(method),
-                                            cast<FuncDecl>(swiftThunk));
-              return method;
+      if (decl->isVirtual()) {
+        if (auto funcDecl = dyn_cast_or_null<FuncDecl>(method)) {
+          if (Impl.isCxxInteropCompatVersionAtLeast(6)) {
+            if (auto structDecl =
+                    dyn_cast_or_null<StructDecl>(method->getDeclContext())) {
+              // If this is a method of a Swift struct, any possible override of
+              // this method would get sliced away, and an invocation would get
+              // dispatched statically. This is fine because it matches the C++
+              // behavior.
+              if (decl->isPure()) {
+                // If this is a pure virtual method, we won't have any
+                // implementation of it to invoke.
+                Impl.markUnavailable(
+                    funcDecl, "virtual function is not available in Swift "
+                              "because it is pure");
+              }
+            } else if (auto classDecl = dyn_cast_or_null<ClassDecl>(
+                           funcDecl->getDeclContext())) {
+              // This is a foreign reference type. Since `class T` on the Swift
+              // side is mapped from `T*` on the C++ side, an invocation of a
+              // virtual method `t->method()` should get dispatched dynamically.
+              // Create a thunk that will perform dynamic dispatch.
+              // TODO: we don't have to import the actual `method` in this case,
+              // we can just synthesize a thunk and import that instead.
+              auto result = synthesizer.makeVirtualMethod(decl);
+              if (result) {
+                return result;
+              } else {
+                Impl.markUnavailable(
+                    funcDecl, "virtual function is not available in Swift");
+              }
             }
+          } else {
+            Impl.markUnavailable(
+                funcDecl, "virtual functions are not yet available in Swift");
           }
-
-          Impl.markUnavailable(
-              cast<ValueDecl>(method),
-              decl->isPure() ? "virtual function is not available in Swift "
-                               "because it is pure"
-                             : "virtual function is not available in Swift");
-        } else {
-          Impl.markUnavailable(
-              cast<ValueDecl>(method),
-              "virtual functions are not yet available in Swift");
         }
       }
 

lib/ClangImporter/ImportName.cpp

@@ -2258,6 +2258,14 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
         baseName = newName;
       }
     }
+    if (method->isImplicit() &&
+        baseName.starts_with("__synthesizedVirtualCall_")) {
+      // If this is a thunk for a virtual method of a C++ reference type, we
+      // strip away the underscored prefix. This method should be visible and
+      // callable from Swift.
+      newName = baseName.substr(StringRef("__synthesizedVirtualCall_").size());
+      baseName = newName;
+    }
   }
 
   // swift_newtype-ed declarations may have common words with the type name

lib/ClangImporter/ImporterImpl.h

@@ -2024,14 +2024,4 @@ bool isViewType(const clang::CXXRecordDecl *decl);
 }
 }
 
-// Forwards to synthesizeCxxBasicMethod(), producing a thunk that calls a
-// virtual function.
-clang::CXXMethodDecl *synthesizeCxxVirtualMethod(
-    swift::ClangImporter &Impl, const clang::CXXRecordDecl *derivedClass,
-    const clang::CXXRecordDecl *baseClass, const clang::CXXMethodDecl *method);
-
-// Exposed to produce a Swift method body for calling a Swift thunk.
-std::pair<swift::BraceStmt *, bool>
-synthesizeForwardingThunkBody(swift::AbstractFunctionDecl *afd, void *context);
-
 #endif