Skip to content

[cxx-interop] Import using decls that expose methods from private base classes #69623

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
Nov 14, 2023
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
5 changes: 5 additions & 0 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7074,6 +7074,11 @@ static bool isSufficientlyTrivial(const clang::CXXRecordDecl *decl) {
/// Checks if a record provides the required value type lifetime operations
/// (copy and destroy).
static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
// Hack for a base type of std::optional from the Microsoft standard library.
if (decl->isInStdNamespace() && decl->getIdentifier() &&
decl->getName() == "_Optional_construct_base")
return true;

// If we have no way of copying the type we can't import the class
// at all because we cannot express the correct semantics as a swift
// struct.
Expand Down
248 changes: 166 additions & 82 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2697,6 +2697,31 @@ namespace {
// SemaLookup.cpp).
if (!decl->isBeingDefined() && !decl->isDependentContext() &&
areRecordFieldsComplete(decl)) {
if (decl->hasInheritedConstructor() &&
Impl.isCxxInteropCompatVersionAtLeast(
version::getUpcomingCxxInteropCompatVersion())) {
for (auto member : decl->decls()) {
if (auto usingDecl = dyn_cast<clang::UsingDecl>(member)) {
for (auto usingShadowDecl : usingDecl->shadows()) {
if (auto ctorUsingShadowDecl =
dyn_cast<clang::ConstructorUsingShadowDecl>(
usingShadowDecl)) {
auto baseCtorDecl = dyn_cast<clang::CXXConstructorDecl>(
ctorUsingShadowDecl->getTargetDecl());
if (!baseCtorDecl || baseCtorDecl->isDeleted())
continue;
auto derivedCtorDecl = clangSema.findInheritingConstructor(
clang::SourceLocation(), baseCtorDecl,
ctorUsingShadowDecl);
if (!derivedCtorDecl->isDefined() &&
!derivedCtorDecl->isDeleted())
clangSema.DefineInheritingConstructor(
clang::SourceLocation(), derivedCtorDecl);
}
}
}
}
}
if (decl->needsImplicitDefaultConstructor()) {
clang::CXXConstructorDecl *ctor =
clangSema.DeclareImplicitDefaultConstructor(
Expand Down Expand Up @@ -3237,6 +3262,65 @@ namespace {
llvm::None);
}

/// Handles special functions such as subscripts and dereference operators.
bool processSpecialImportedFunc(FuncDecl *func, ImportedName importedName) {
auto dc = func->getDeclContext();

if (importedName.isSubscriptAccessor()) {
assert(func->getParameters()->size() == 1);
auto typeDecl = dc->getSelfNominalTypeDecl();
auto parameter = func->getParameters()->get(0);
auto parameterType = parameter->getTypeInContext();
if (!typeDecl || !parameterType)
return false;
if (parameter->isInOut())
// Subscripts with inout parameters are not allowed in Swift.
return false;
// Subscript setter is marked as mutating in Swift even if the
// C++ `operator []` is `const`.
if (importedName.getAccessorKind() ==
ImportedAccessorKind::SubscriptSetter &&
!dc->isModuleScopeContext() &&
!typeDecl->getDeclaredType()->isForeignReferenceType())
func->setSelfAccessKind(SelfAccessKind::Mutating);

auto &getterAndSetter = Impl.cxxSubscripts[{typeDecl, parameterType}];

switch (importedName.getAccessorKind()) {
case ImportedAccessorKind::SubscriptGetter:
getterAndSetter.first = func;
break;
case ImportedAccessorKind::SubscriptSetter:
getterAndSetter.second = func;
break;
default:
llvm_unreachable("invalid subscript kind");
}

Impl.markUnavailable(func, "use subscript");
}

if (importedName.isDereferenceAccessor()) {
auto typeDecl = dc->getSelfNominalTypeDecl();
auto &getterAndSetter = Impl.cxxDereferenceOperators[typeDecl];

switch (importedName.getAccessorKind()) {
case ImportedAccessorKind::DereferenceGetter:
getterAndSetter.first = func;
break;
case ImportedAccessorKind::DereferenceSetter:
getterAndSetter.second = func;
break;
default:
llvm_unreachable("invalid dereference operator kind");
}

Impl.markUnavailable(func, "use .pointee property");
}

return true;
}

Decl *importFunctionDecl(
const clang::FunctionDecl *decl, ImportedName importedName,
llvm::Optional<ImportedName> correctSwiftName,
Expand Down Expand Up @@ -3556,69 +3640,14 @@ namespace {
func->setImportAsStaticMember();
}
}
// Someday, maybe this will need to be 'open' for C++ virtual methods.
func->setAccess(AccessLevel::Public);

bool makePrivate = false;

if (importedName.isSubscriptAccessor() && !importFuncWithoutSignature) {
assert(func->getParameters()->size() == 1);
auto typeDecl = dc->getSelfNominalTypeDecl();
auto parameter = func->getParameters()->get(0);
auto parameterType = parameter->getTypeInContext();
if (!typeDecl || !parameterType)
if (!importFuncWithoutSignature) {
bool success = processSpecialImportedFunc(func, importedName);
if (!success)
return nullptr;
if (parameter->isInOut())
// Subscripts with inout parameters are not allowed in Swift.
return nullptr;
// Subscript setter is marked as mutating in Swift even if the
// C++ `operator []` is `const`.
if (importedName.getAccessorKind() ==
ImportedAccessorKind::SubscriptSetter &&
!dc->isModuleScopeContext() &&
!typeDecl->getDeclaredType()->isForeignReferenceType())
func->setSelfAccessKind(SelfAccessKind::Mutating);

auto &getterAndSetter = Impl.cxxSubscripts[{ typeDecl,
parameterType }];

switch (importedName.getAccessorKind()) {
case ImportedAccessorKind::SubscriptGetter:
getterAndSetter.first = func;
break;
case ImportedAccessorKind::SubscriptSetter:
getterAndSetter.second = func;
break;
default:
llvm_unreachable("invalid subscript kind");
}

Impl.markUnavailable(func, "use subscript");
}

if (importedName.isDereferenceAccessor() &&
!importFuncWithoutSignature) {
auto typeDecl = dc->getSelfNominalTypeDecl();
auto &getterAndSetter = Impl.cxxDereferenceOperators[typeDecl];

switch (importedName.getAccessorKind()) {
case ImportedAccessorKind::DereferenceGetter:
getterAndSetter.first = func;
break;
case ImportedAccessorKind::DereferenceSetter:
getterAndSetter.second = func;
break;
default:
llvm_unreachable("invalid dereference operator kind");
}

Impl.markUnavailable(func, "use .pointee property");
makePrivate = true;
}

if (makePrivate)
func->setAccess(AccessLevel::Private);
else
// Someday, maybe this will need to be 'open' for C++ virtual methods.
func->setAccess(AccessLevel::Public);
}

result->setIsObjC(false);
Expand Down Expand Up @@ -3931,18 +3960,31 @@ namespace {
}

Decl *VisitUsingDecl(const clang::UsingDecl *decl) {
// Using declarations are not imported.
// See VisitUsingShadowDecl below.
return nullptr;
}

Decl *VisitUsingShadowDecl(const clang::UsingShadowDecl *decl) {
// Only import types for now.
if (!isa<clang::TypeDecl>(decl->getUnderlyingDecl()))
// Only import:
// 1. Types
// 2. C++ methods from privately inherited base classes
if (!isa<clang::TypeDecl>(decl->getTargetDecl()) &&
!(isa<clang::CXXMethodDecl>(decl->getTargetDecl()) &&
Impl.isCxxInteropCompatVersionAtLeast(
version::getUpcomingCxxInteropCompatVersion())))
return nullptr;
// Constructors (e.g. `using BaseClass::BaseClass`) are handled in
// VisitCXXRecordDecl, since we need them to determine whether a struct
// can be imported into Swift.
if (isa<clang::CXXConstructorDecl>(decl->getTargetDecl()))
return nullptr;

ImportedName importedName;
llvm::Optional<ImportedName> correctSwiftName;
std::tie(importedName, correctSwiftName) = importFullName(decl);
// Don't import something that doesn't have a name.
if (importedName.getDeclName().isSpecial())
return nullptr;
auto Name = importedName.getDeclName().getBaseIdentifier();
if (Name.empty())
return nullptr;
Expand All @@ -3953,30 +3995,66 @@ namespace {
return importCompatibilityTypeAlias(decl, importedName,
*correctSwiftName);

auto DC =
auto importedDC =
Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
if (!DC)
if (!importedDC)
return nullptr;

Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
if (!SwiftDecl)
return nullptr;
if (isa<clang::TypeDecl>(decl->getTargetDecl())) {
Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
if (!SwiftDecl)
return nullptr;

const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
if (!SwiftTypeDecl)
return nullptr;
const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
if (!SwiftTypeDecl)
return nullptr;

auto Loc = Impl.importSourceLoc(decl->getLocation());
auto Result = Impl.createDeclWithClangNode<TypeAliasDecl>(
decl,
AccessLevel::Public,
Impl.importSourceLoc(decl->getBeginLoc()),
SourceLoc(), Name,
Loc,
/*genericparams*/nullptr, DC);
Result->setUnderlyingType(SwiftTypeDecl->getDeclaredInterfaceType());
auto Loc = Impl.importSourceLoc(decl->getLocation());
auto Result = Impl.createDeclWithClangNode<TypeAliasDecl>(
decl,
AccessLevel::Public,
Impl.importSourceLoc(decl->getBeginLoc()),
SourceLoc(), Name,
Loc,
/*genericparams*/nullptr, importedDC);
Result->setUnderlyingType(SwiftTypeDecl->getDeclaredInterfaceType());

return Result;
}
if (auto targetMethod =
dyn_cast<clang::CXXMethodDecl>(decl->getTargetDecl())) {
auto dc = dyn_cast<clang::CXXRecordDecl>(decl->getDeclContext());

auto targetDC = targetMethod->getDeclContext();
auto targetRecord = dyn_cast<clang::CXXRecordDecl>(targetDC);
if (!targetRecord)
return nullptr;

return Result;
// If this struct is not inherited from the struct where the method is
// defined, bail.
if (!dc->isDerivedFrom(targetRecord))
return nullptr;

auto importedBaseMethod = dyn_cast_or_null<FuncDecl>(
Impl.importDecl(targetMethod, getActiveSwiftVersion()));
// This will be nullptr for a protected method of base class that is
// made public with a using declaration in a derived class. This is
// valid in C++ but we do not import such using declarations now.
// TODO: make this work for protected base methods.
if (!importedBaseMethod)
return nullptr;
auto clonedMethod = dyn_cast_or_null<FuncDecl>(
Impl.importBaseMemberDecl(importedBaseMethod, importedDC));
if (!clonedMethod)
return nullptr;

bool success = processSpecialImportedFunc(clonedMethod, importedName);
if (!success)
return nullptr;

return clonedMethod;
}
return nullptr;
}

/// Add an @objc(name) attribute with the given, optional name expressed as
Expand Down Expand Up @@ -8360,11 +8438,17 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
if (Result &&
(!Result->getDeclContext()->isModuleScopeContext() ||
isa<ClangModuleUnit>(Result->getDeclContext()))) {
// For using declarations that expose a method of a base class, the Clang
// decl is synthesized lazily when the method is actually used from Swift.
bool hasSynthesizedClangNode =
isa<clang::UsingShadowDecl>(ClangDecl) && isa<FuncDecl>(Result);

// Either the Swift declaration was from stdlib,
// or we imported the underlying decl of the typedef,
// or we imported the decl itself.
bool ImportedCorrectly =
!Result->getClangDecl() || SkippedOverTypedef ||
hasSynthesizedClangNode ||
Result->getClangDecl()->getCanonicalDecl() == Canon;

// Or the other type is a typedef,
Expand All @@ -8387,7 +8471,7 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
}
assert(ImportedCorrectly);
}
assert(Result->hasClangNode());
assert(Result->hasClangNode() || hasSynthesizedClangNode);
}
#else
(void)SkippedOverTypedef;
Expand Down
11 changes: 11 additions & 0 deletions lib/ClangImporter/ImportName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,17 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
return ImportedName();
result.effectiveContext = effectiveCtx;

// If this is a using declaration, import the name of the shadowed decl and
// adjust the context.
if (auto usingShadowDecl = dyn_cast<clang::UsingShadowDecl>(D)) {
auto targetDecl = usingShadowDecl->getTargetDecl();
if (isa<clang::CXXMethodDecl>(targetDecl)) {
ImportedName baseName = importName(targetDecl, version, givenName);
baseName.effectiveContext = effectiveCtx;
return baseName;
}
}

// Gather information from the swift_async attribute, if there is one.
llvm::Optional<unsigned> completionHandlerParamIndex;
bool completionHandlerFlagIsZeroOnError = false;
Expand Down
2 changes: 1 addition & 1 deletion lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -663,9 +663,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
llvm::DenseMap<std::pair<ValueDecl *, DeclContext *>, ValueDecl *>
clonedBaseMembers;

public:
ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext);

public:
static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl);

// Cache for already-specialized function templates and any thunks they may
Expand Down
6 changes: 6 additions & 0 deletions lib/ClangImporter/SwiftLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,12 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
}
}
}
if (auto usingDecl = dyn_cast<clang::UsingDecl>(named)) {
for (auto usingShadowDecl : usingDecl->shadows()) {
if (isa<clang::CXXMethodDecl>(usingShadowDecl->getTargetDecl()))
addEntryToLookupTable(table, usingShadowDecl, nameImporter);
}
}
}

/// Returns the nearest parent of \p module that is marked \c explicit in its
Expand Down
6 changes: 0 additions & 6 deletions test/Interop/Cxx/class/Inputs/constructors.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,4 @@ struct DeletedCopyConstructor {
DeletedCopyConstructor(const DeletedCopyConstructor &) = delete;
};

// TODO: we should be able to import this constructor correctly. Until we can,
// make sure not to crash.
struct UsingBaseConstructor : ConstructorWithParam {
using ConstructorWithParam::ConstructorWithParam;
};

#endif // TEST_INTEROP_CXX_CLASS_INPUTS_CONSTRUCTORS_H
5 changes: 5 additions & 0 deletions test/Interop/Cxx/class/inheritance/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ module TypeAliases {
header "type-aliases.h"
}

module UsingBaseMembers {
header "using-base-members.h"
requires cplusplus
}

module VirtualMethods {
header "virtual-methods.h"
requires cplusplus
Expand Down
Loading