Skip to content

[cxx-interop] Import default public ctor of C++ foreign ref types as Swift Initializer #79986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ EXPERIMENTAL_FEATURE(AssumeResilientCxxTypes, true)
/// Import inherited non-public members when importing C++ classes.
EXPERIMENTAL_FEATURE(ImportNonPublicCxxMembers, true)

/// Synthesize static factory methods for C++ foreign reference types and import
/// them as Swift initializers.
EXPERIMENTAL_FEATURE(CXXForeignReferenceTypeInitializers, true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that all the major concerns are addressed in this PR, I would even be OK to not have the feature flag and have this feature on by default for default constructors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per some discussions offline, we might want to invert this flag so users can opt out of this feature if it causes any trouble.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to invert this flag so users can opt out of this feature if it causes any trouble.

I seem like we can use the same feature flag to either enable or disable a feature.
Disable: -disable-experimental-feature CXXForeignReferenceTypeInitializers
Enable: -enable-experimental-feature CXXForeignReferenceTypeInitializers

I tested this at least for the features which are marked as EXPERIMENTAL_FEATURE(<feature-name>, true)

I am not sure what would be the semantics of EXPERIMENTAL_FEATURE(<feature-name>, false)

I am not sure what is the best thing to do to call this feature as ready. My inclination is towards completely removing the feature flag if it feels stable enough and we are happy with the test coverage.

Copy link
Contributor

@Xazax-hun Xazax-hun Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My inclination is towards completely removing the feature flag if it feels stable enough and we are happy with the test coverage.

I think the compilation error with operator new and immortal references is a blocker to turn this on by default. Until those are resolved there will be user code that is broken by this patch. I.e., it was compiling fine before but stopped compiling after.


// Isolated deinit
SUPPRESSIBLE_LANGUAGE_FEATURE(IsolatedDeinit, 371, "isolated deinit")

Expand Down
1 change: 1 addition & 0 deletions lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ UNINTERESTING_FEATURE(StrictMemorySafety)
UNINTERESTING_FEATURE(SafeInteropWrappers)
UNINTERESTING_FEATURE(AssumeResilientCxxTypes)
UNINTERESTING_FEATURE(ImportNonPublicCxxMembers)
UNINTERESTING_FEATURE(CXXForeignReferenceTypeInitializers)
UNINTERESTING_FEATURE(CoroutineAccessorsUnwindOnCallerError)

static bool usesFeatureSwiftSettings(const Decl *decl) {
Expand Down
13 changes: 1 addition & 12 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7686,17 +7686,6 @@ static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
});
}

// TODO: Move all these utility functions in a new file ClangImporterUtils.h
// rdar://138803759
static bool hasImmortalAtts(const clang::RecordDecl *decl) {
return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
return swiftAttr->getAttribute() == "retain:immortal" ||
swiftAttr->getAttribute() == "release:immortal";
return false;
});
}

// Is this a pointer to a foreign reference type.
bool importer::isForeignReferenceTypeWithoutImmortalAttrs(const clang::QualType type) {
if (!type->isPointerType())
Expand All @@ -7708,7 +7697,7 @@ bool importer::isForeignReferenceTypeWithoutImmortalAttrs(const clang::QualType
return false;

return hasImportAsRefAttr(pointeeType->getDecl()) &&
!hasImmortalAtts(pointeeType->getDecl());
!hasImmortalAttrs(pointeeType->getDecl());
}

static bool hasDiamondInheritanceRefType(const clang::CXXRecordDecl *decl) {
Expand Down
42 changes: 40 additions & 2 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ bool importer::recordHasReferenceSemantics(
return semanticsKind == CxxRecordSemanticsKind::Reference;
}

bool importer::hasImmortalAttrs(const clang::RecordDecl *decl) {
return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
return swiftAttr->getAttribute() == "retain:immortal" ||
swiftAttr->getAttribute() == "release:immortal";
return false;
});
}

#ifndef NDEBUG
static bool verifyNameMapping(MappedTypeNameKind NameMapping,
StringRef left, StringRef right) {
Expand Down Expand Up @@ -2472,14 +2481,43 @@ namespace {
ctors.push_back(valueCtor);
}

// Do not allow Swift to construct foreign reference types (at least, not
// yet).
if (isa<StructDecl>(result)) {
for (auto ctor : ctors) {
// Add ctors directly as they cannot always be looked up from the
// clang decl (some are synthesized by Swift).
result->addMember(ctor);
}
} else {
if (Impl.SwiftContext.LangOpts.hasFeature(
Feature::CXXForeignReferenceTypeInitializers)) {
assert(
isa<ClassDecl>(result) &&
"Expected result to be a ClassDecl as it cannot be a StructDecl");
// When we add full support for C foreign reference types then we
// should synthesize static factories for them as well
if (auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(decl)) {
bool hasUserProvidedStaticFactory = llvm::any_of(
cxxRecordDecl->methods(),
[](const clang::CXXMethodDecl *method) {
return method->isStatic() &&
llvm::any_of(
method->specific_attrs<clang::SwiftNameAttr>(),
[](const auto *attr) {
return attr->getName().starts_with("init(");
});
});
if (!hasUserProvidedStaticFactory) {
if (auto generatedCxxMethodDecl =
synthesizer.synthesizeStaticFactoryForCXXForeignRef(
cxxRecordDecl)) {
if (Decl *importedInitDecl =
Impl.SwiftContext.getClangModuleLoader()
->importDeclDirectly(generatedCxxMethodDecl))
result->addMember(importedInitDecl);
}
}
}
}
}

if (auto structResult = dyn_cast<StructDecl>(result)) {
Expand Down
4 changes: 4 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1890,6 +1890,10 @@ namespace importer {
bool recordHasReferenceSemantics(const clang::RecordDecl *decl,
ClangImporter::Implementation *importerImpl);

/// Returns true if the given C/C++ reference type uses "immortal"
/// retain/release functions.
bool hasImmortalAttrs(const clang::RecordDecl *decl);

/// Whether this is a forward declaration of a type. We ignore forward
/// declarations in certain cases, and instead process the real declarations.
bool isForwardDeclOfType(const clang::Decl *decl);
Expand Down
128 changes: 128 additions & 0 deletions lib/ClangImporter/SwiftDeclSynthesizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2529,3 +2529,131 @@ SwiftDeclSynthesizer::makeDefaultArgument(const clang::ParmVarDecl *param,

return callExpr;
}

// MARK: C++ foreign reference type constructors

clang::CXXMethodDecl *
SwiftDeclSynthesizer::synthesizeStaticFactoryForCXXForeignRef(
const clang::CXXRecordDecl *cxxRecordDecl) {

clang::ASTContext &clangCtx = cxxRecordDecl->getASTContext();
clang::Sema &clangSema = ImporterImpl.getClangSema();

clang::QualType cxxRecordTy = clangCtx.getRecordType(cxxRecordDecl);

clang::CXXConstructorDecl *defaultCtorDecl = nullptr;
for (clang::CXXConstructorDecl *ctor : cxxRecordDecl->ctors()) {
if (ctor->parameters().empty() && !ctor->isDeleted() &&
ctor->getAccess() != clang::AS_private &&
ctor->getAccess() != clang::AS_protected) {
defaultCtorDecl = ctor;
break;
}
}
if (!defaultCtorDecl)
return nullptr;

clang::FunctionDecl *operatorNew = nullptr;
clang::FunctionDecl *operatorDelete = nullptr;
bool passAlignment = false;
bool findingAllocFuncFailed = clangSema.FindAllocationFunctions(
cxxRecordDecl->getLocation(), clang::SourceRange(), clang::Sema::AFS_Both,
clang::Sema::AFS_Both, cxxRecordTy,
/*IsArray*/ false, passAlignment, clang::MultiExprArg(), operatorNew,
operatorDelete, /*Diagnose*/ false);
if (findingAllocFuncFailed || !operatorNew || operatorNew->isDeleted() ||
operatorNew->getAccess() == clang::AS_private ||
operatorNew->getAccess() == clang::AS_protected)
return nullptr;

clang::QualType cxxRecordPtrTy = clangCtx.getPointerType(cxxRecordTy);
// Adding `_Nonnull` to the return type of synthesized static factory
bool nullabilityCannotBeAdded =
clangSema.CheckImplicitNullabilityTypeSpecifier(
cxxRecordPtrTy, clang::NullabilityKind::NonNull,
cxxRecordDecl->getLocation(),
/*isParam=*/false,
/*OverrideExisting=*/true);
assert(!nullabilityCannotBeAdded &&
"Failed to add _Nonnull specifier to synthesized "
"static factory's return type");

clang::IdentifierTable &clangIdents = clangCtx.Idents;
clang::IdentifierInfo *funcNameToSynthesize = &clangIdents.get(
("__returns_" + cxxRecordDecl->getNameAsString()).c_str());
clang::FunctionProtoType::ExtProtoInfo EPI;
clang::QualType funcTypeToSynthesize =
clangCtx.getFunctionType(cxxRecordPtrTy, {}, EPI);

clang::CXXMethodDecl *synthesizedCxxMethodDecl = clang::CXXMethodDecl::Create(
clangCtx, const_cast<clang::CXXRecordDecl *>(cxxRecordDecl),
cxxRecordDecl->getLocation(),
clang::DeclarationNameInfo(funcNameToSynthesize,
cxxRecordDecl->getLocation()),
funcTypeToSynthesize,
clangCtx.getTrivialTypeSourceInfo(funcTypeToSynthesize), clang::SC_Static,
/*UsesFPIntrin=*/false, /*isInline=*/true,
clang::ConstexprSpecKind::Unspecified, cxxRecordDecl->getLocation());
assert(synthesizedCxxMethodDecl &&
"Unable to synthesize static factory for c++ foreign reference type");
synthesizedCxxMethodDecl->setAccess(clang::AccessSpecifier::AS_public);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment about not assuming clang::AS_public; perhaps just take the access specifier from the factory function you're synthesizing from?

(also, this is usually spelled clang::AS_public since it's defined as a C-style enum)


if (!hasImmortalAttrs(cxxRecordDecl)) {
clang::SwiftAttrAttr *returnsRetainedAttrForSynthesizedCxxMethodDecl =
clang::SwiftAttrAttr::Create(clangCtx, "returns_retained");
synthesizedCxxMethodDecl->addAttr(
returnsRetainedAttrForSynthesizedCxxMethodDecl);
}

clang::SwiftNameAttr *swiftNameInitAttrForSynthesizedCxxMethodDecl =
clang::SwiftNameAttr::Create(clangCtx, "init()");
synthesizedCxxMethodDecl->addAttr(
swiftNameInitAttrForSynthesizedCxxMethodDecl);

clang::ExprResult synthesizedConstructExprResult =
clangSema.BuildCXXConstructExpr(
clang::SourceLocation(), cxxRecordTy, defaultCtorDecl,
/*Elidable=*/false, clang::MultiExprArg(),
/*HadMultipleCandidates=*/false,
/*IsListInitialization=*/false,
/*IsStdInitListInitialization=*/false,
/*RequiresZeroInit=*/false, clang::CXXConstructionKind::Complete,
clang::SourceRange());
assert(!synthesizedConstructExprResult.isInvalid() &&
"Unable to synthesize constructor expression for c++ foreign "
"reference type");
clang::CXXConstructExpr *synthesizedConstructExpr =
cast<clang::CXXConstructExpr>(synthesizedConstructExprResult.get());

clang::ExprResult synthesizedNewExprResult = clangSema.BuildCXXNew(
clang::SourceRange(), /*UseGlobal=*/false, clang::SourceLocation(), {},
clang::SourceLocation(), clang::SourceRange(), cxxRecordTy,
clangCtx.getTrivialTypeSourceInfo(cxxRecordTy), std::nullopt,
clang::SourceRange(), synthesizedConstructExpr);
assert(
!synthesizedNewExprResult.isInvalid() &&
"Unable to synthesize `new` expression for c++ foreign reference type");
clang::CXXNewExpr *synthesizedNewExpr =
cast<clang::CXXNewExpr>(synthesizedNewExprResult.get());

clang::ReturnStmt *synthesizedRetStmt =
clang::ReturnStmt::Create(clangCtx, clang::SourceLocation(),
synthesizedNewExpr, /*VarDecl=*/nullptr);
assert(synthesizedRetStmt && "Unable to synthesize return statement for "
"static factory of c++ foreign reference type");

clang::CompoundStmt *synthesizedFuncBody = clang::CompoundStmt::Create(
clangCtx, {synthesizedRetStmt}, clang::FPOptionsOverride(),
clang::SourceLocation(), clang::SourceLocation());
assert(synthesizedRetStmt && "Unable to synthesize function body for static "
"factory of c++ foreign reference type");

synthesizedCxxMethodDecl->setBody(synthesizedFuncBody);
synthesizedCxxMethodDecl->addAttr(
clang::NoDebugAttr::CreateImplicit(clangCtx));

synthesizedCxxMethodDecl->setImplicit();
synthesizedCxxMethodDecl->setImplicitlyInline();

return synthesizedCxxMethodDecl;
}
6 changes: 6 additions & 0 deletions lib/ClangImporter/SwiftDeclSynthesizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,12 @@ class SwiftDeclSynthesizer {
const swift::Type &swiftParamTy,
SourceLoc paramLoc);

/// Synthesize a static factory method for a C++ foreign reference type,
/// returning a `CXXMethodDecl*` or `nullptr` if the required constructor or
/// allocation function is not found.
clang::CXXMethodDecl *synthesizeStaticFactoryForCXXForeignRef(
const clang::CXXRecordDecl *cxxRecordDecl);

private:
Type getConstantLiteralType(Type type, ConstantConvertKind convertKind);
};
Expand Down
Loading