Skip to content

Commit 6e4f3ba

Browse files
committed
[cxx-interop] Import using decls that expose methods from private base classes
If a C++ type `Derived` inherits from `Base` privately, the public methods from `Base` should not be callable on an instance of `Derived`. However, C++ supports exposing such methods via a using declaration: `using MyPrivateBase::myPublicMethod;`. MSVC started using this feature for `std::optional` which means Swift doesn't correctly import `var pointee: Pointee` for instantiations of `std::optional` on Windows. This prevents the automatic conformance to `CxxOptional` from being synthesized. rdar://114282353 / resolves #68068 Cherrypick commit efc008a Cherrypick PR #69623
1 parent 0906aa1 commit 6e4f3ba

17 files changed

+390
-94
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7077,6 +7077,11 @@ static bool isSufficientlyTrivial(const clang::CXXRecordDecl *decl) {
70777077
/// Checks if a record provides the required value type lifetime operations
70787078
/// (copy and destroy).
70797079
static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
7080+
// Hack for a base type of std::optional from the Microsoft standard library.
7081+
if (decl->isInStdNamespace() && decl->getIdentifier() &&
7082+
decl->getName() == "_Optional_construct_base")
7083+
return true;
7084+
70807085
// If we have no way of copying the type we can't import the class
70817086
// at all because we cannot express the correct semantics as a swift
70827087
// struct.

lib/ClangImporter/ImportDecl.cpp

Lines changed: 166 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -2695,6 +2695,31 @@ namespace {
26952695
// SemaLookup.cpp).
26962696
if (!decl->isBeingDefined() && !decl->isDependentContext() &&
26972697
areRecordFieldsComplete(decl)) {
2698+
if (decl->hasInheritedConstructor() &&
2699+
Impl.isCxxInteropCompatVersionAtLeast(
2700+
version::getUpcomingCxxInteropCompatVersion())) {
2701+
for (auto member : decl->decls()) {
2702+
if (auto usingDecl = dyn_cast<clang::UsingDecl>(member)) {
2703+
for (auto usingShadowDecl : usingDecl->shadows()) {
2704+
if (auto ctorUsingShadowDecl =
2705+
dyn_cast<clang::ConstructorUsingShadowDecl>(
2706+
usingShadowDecl)) {
2707+
auto baseCtorDecl = dyn_cast<clang::CXXConstructorDecl>(
2708+
ctorUsingShadowDecl->getTargetDecl());
2709+
if (!baseCtorDecl || baseCtorDecl->isDeleted())
2710+
continue;
2711+
auto derivedCtorDecl = clangSema.findInheritingConstructor(
2712+
clang::SourceLocation(), baseCtorDecl,
2713+
ctorUsingShadowDecl);
2714+
if (!derivedCtorDecl->isDefined() &&
2715+
!derivedCtorDecl->isDeleted())
2716+
clangSema.DefineInheritingConstructor(
2717+
clang::SourceLocation(), derivedCtorDecl);
2718+
}
2719+
}
2720+
}
2721+
}
2722+
}
26982723
if (decl->needsImplicitDefaultConstructor()) {
26992724
clang::CXXConstructorDecl *ctor =
27002725
clangSema.DeclareImplicitDefaultConstructor(
@@ -3234,6 +3259,65 @@ namespace {
32343259
llvm::None);
32353260
}
32363261

3262+
/// Handles special functions such as subscripts and dereference operators.
3263+
bool processSpecialImportedFunc(FuncDecl *func, ImportedName importedName) {
3264+
auto dc = func->getDeclContext();
3265+
3266+
if (importedName.isSubscriptAccessor()) {
3267+
assert(func->getParameters()->size() == 1);
3268+
auto typeDecl = dc->getSelfNominalTypeDecl();
3269+
auto parameter = func->getParameters()->get(0);
3270+
auto parameterType = parameter->getTypeInContext();
3271+
if (!typeDecl || !parameterType)
3272+
return false;
3273+
if (parameter->isInOut())
3274+
// Subscripts with inout parameters are not allowed in Swift.
3275+
return false;
3276+
// Subscript setter is marked as mutating in Swift even if the
3277+
// C++ `operator []` is `const`.
3278+
if (importedName.getAccessorKind() ==
3279+
ImportedAccessorKind::SubscriptSetter &&
3280+
!dc->isModuleScopeContext() &&
3281+
!typeDecl->getDeclaredType()->isForeignReferenceType())
3282+
func->setSelfAccessKind(SelfAccessKind::Mutating);
3283+
3284+
auto &getterAndSetter = Impl.cxxSubscripts[{typeDecl, parameterType}];
3285+
3286+
switch (importedName.getAccessorKind()) {
3287+
case ImportedAccessorKind::SubscriptGetter:
3288+
getterAndSetter.first = func;
3289+
break;
3290+
case ImportedAccessorKind::SubscriptSetter:
3291+
getterAndSetter.second = func;
3292+
break;
3293+
default:
3294+
llvm_unreachable("invalid subscript kind");
3295+
}
3296+
3297+
Impl.markUnavailable(func, "use subscript");
3298+
}
3299+
3300+
if (importedName.isDereferenceAccessor()) {
3301+
auto typeDecl = dc->getSelfNominalTypeDecl();
3302+
auto &getterAndSetter = Impl.cxxDereferenceOperators[typeDecl];
3303+
3304+
switch (importedName.getAccessorKind()) {
3305+
case ImportedAccessorKind::DereferenceGetter:
3306+
getterAndSetter.first = func;
3307+
break;
3308+
case ImportedAccessorKind::DereferenceSetter:
3309+
getterAndSetter.second = func;
3310+
break;
3311+
default:
3312+
llvm_unreachable("invalid dereference operator kind");
3313+
}
3314+
3315+
Impl.markUnavailable(func, "use .pointee property");
3316+
}
3317+
3318+
return true;
3319+
}
3320+
32373321
Decl *importFunctionDecl(
32383322
const clang::FunctionDecl *decl, ImportedName importedName,
32393323
llvm::Optional<ImportedName> correctSwiftName,
@@ -3554,69 +3638,14 @@ namespace {
35543638
func->setImportAsStaticMember();
35553639
}
35563640
}
3641+
// Someday, maybe this will need to be 'open' for C++ virtual methods.
3642+
func->setAccess(AccessLevel::Public);
35573643

3558-
bool makePrivate = false;
3559-
3560-
if (importedName.isSubscriptAccessor() && !importFuncWithoutSignature) {
3561-
assert(func->getParameters()->size() == 1);
3562-
auto typeDecl = dc->getSelfNominalTypeDecl();
3563-
auto parameter = func->getParameters()->get(0);
3564-
auto parameterType = parameter->getTypeInContext();
3565-
if (!typeDecl || !parameterType)
3644+
if (!importFuncWithoutSignature) {
3645+
bool success = processSpecialImportedFunc(func, importedName);
3646+
if (!success)
35663647
return nullptr;
3567-
if (parameter->isInOut())
3568-
// Subscripts with inout parameters are not allowed in Swift.
3569-
return nullptr;
3570-
// Subscript setter is marked as mutating in Swift even if the
3571-
// C++ `operator []` is `const`.
3572-
if (importedName.getAccessorKind() ==
3573-
ImportedAccessorKind::SubscriptSetter &&
3574-
!dc->isModuleScopeContext() &&
3575-
!typeDecl->getDeclaredType()->isForeignReferenceType())
3576-
func->setSelfAccessKind(SelfAccessKind::Mutating);
3577-
3578-
auto &getterAndSetter = Impl.cxxSubscripts[{ typeDecl,
3579-
parameterType }];
3580-
3581-
switch (importedName.getAccessorKind()) {
3582-
case ImportedAccessorKind::SubscriptGetter:
3583-
getterAndSetter.first = func;
3584-
break;
3585-
case ImportedAccessorKind::SubscriptSetter:
3586-
getterAndSetter.second = func;
3587-
break;
3588-
default:
3589-
llvm_unreachable("invalid subscript kind");
3590-
}
3591-
3592-
Impl.markUnavailable(func, "use subscript");
35933648
}
3594-
3595-
if (importedName.isDereferenceAccessor() &&
3596-
!importFuncWithoutSignature) {
3597-
auto typeDecl = dc->getSelfNominalTypeDecl();
3598-
auto &getterAndSetter = Impl.cxxDereferenceOperators[typeDecl];
3599-
3600-
switch (importedName.getAccessorKind()) {
3601-
case ImportedAccessorKind::DereferenceGetter:
3602-
getterAndSetter.first = func;
3603-
break;
3604-
case ImportedAccessorKind::DereferenceSetter:
3605-
getterAndSetter.second = func;
3606-
break;
3607-
default:
3608-
llvm_unreachable("invalid dereference operator kind");
3609-
}
3610-
3611-
Impl.markUnavailable(func, "use .pointee property");
3612-
makePrivate = true;
3613-
}
3614-
3615-
if (makePrivate)
3616-
func->setAccess(AccessLevel::Private);
3617-
else
3618-
// Someday, maybe this will need to be 'open' for C++ virtual methods.
3619-
func->setAccess(AccessLevel::Public);
36203649
}
36213650

36223651
result->setIsObjC(false);
@@ -3929,18 +3958,31 @@ namespace {
39293958
}
39303959

39313960
Decl *VisitUsingDecl(const clang::UsingDecl *decl) {
3932-
// Using declarations are not imported.
3961+
// See VisitUsingShadowDecl below.
39333962
return nullptr;
39343963
}
39353964

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

39413980
ImportedName importedName;
39423981
llvm::Optional<ImportedName> correctSwiftName;
39433982
std::tie(importedName, correctSwiftName) = importFullName(decl);
3983+
// Don't import something that doesn't have a name.
3984+
if (importedName.getDeclName().isSpecial())
3985+
return nullptr;
39443986
auto Name = importedName.getDeclName().getBaseIdentifier();
39453987
if (Name.empty())
39463988
return nullptr;
@@ -3951,30 +3993,66 @@ namespace {
39513993
return importCompatibilityTypeAlias(decl, importedName,
39523994
*correctSwiftName);
39533995

3954-
auto DC =
3996+
auto importedDC =
39553997
Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
3956-
if (!DC)
3998+
if (!importedDC)
39573999
return nullptr;
39584000

3959-
Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
3960-
if (!SwiftDecl)
3961-
return nullptr;
4001+
if (isa<clang::TypeDecl>(decl->getTargetDecl())) {
4002+
Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
4003+
if (!SwiftDecl)
4004+
return nullptr;
39624005

3963-
const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
3964-
if (!SwiftTypeDecl)
3965-
return nullptr;
4006+
const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
4007+
if (!SwiftTypeDecl)
4008+
return nullptr;
39664009

3967-
auto Loc = Impl.importSourceLoc(decl->getLocation());
3968-
auto Result = Impl.createDeclWithClangNode<TypeAliasDecl>(
3969-
decl,
3970-
AccessLevel::Public,
3971-
Impl.importSourceLoc(decl->getBeginLoc()),
3972-
SourceLoc(), Name,
3973-
Loc,
3974-
/*genericparams*/nullptr, DC);
3975-
Result->setUnderlyingType(SwiftTypeDecl->getDeclaredInterfaceType());
4010+
auto Loc = Impl.importSourceLoc(decl->getLocation());
4011+
auto Result = Impl.createDeclWithClangNode<TypeAliasDecl>(
4012+
decl,
4013+
AccessLevel::Public,
4014+
Impl.importSourceLoc(decl->getBeginLoc()),
4015+
SourceLoc(), Name,
4016+
Loc,
4017+
/*genericparams*/nullptr, importedDC);
4018+
Result->setUnderlyingType(SwiftTypeDecl->getDeclaredInterfaceType());
4019+
4020+
return Result;
4021+
}
4022+
if (auto targetMethod =
4023+
dyn_cast<clang::CXXMethodDecl>(decl->getTargetDecl())) {
4024+
auto dc = dyn_cast<clang::CXXRecordDecl>(decl->getDeclContext());
4025+
4026+
auto targetDC = targetMethod->getDeclContext();
4027+
auto targetRecord = dyn_cast<clang::CXXRecordDecl>(targetDC);
4028+
if (!targetRecord)
4029+
return nullptr;
39764030

3977-
return Result;
4031+
// If this struct is not inherited from the struct where the method is
4032+
// defined, bail.
4033+
if (!dc->isDerivedFrom(targetRecord))
4034+
return nullptr;
4035+
4036+
auto importedBaseMethod = dyn_cast_or_null<FuncDecl>(
4037+
Impl.importDecl(targetMethod, getActiveSwiftVersion()));
4038+
// This will be nullptr for a protected method of base class that is
4039+
// made public with a using declaration in a derived class. This is
4040+
// valid in C++ but we do not import such using declarations now.
4041+
// TODO: make this work for protected base methods.
4042+
if (!importedBaseMethod)
4043+
return nullptr;
4044+
auto clonedMethod = dyn_cast_or_null<FuncDecl>(
4045+
Impl.importBaseMemberDecl(importedBaseMethod, importedDC));
4046+
if (!clonedMethod)
4047+
return nullptr;
4048+
4049+
bool success = processSpecialImportedFunc(clonedMethod, importedName);
4050+
if (!success)
4051+
return nullptr;
4052+
4053+
return clonedMethod;
4054+
}
4055+
return nullptr;
39784056
}
39794057

39804058
/// Add an @objc(name) attribute with the given, optional name expressed as
@@ -8359,11 +8437,17 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
83598437
if (Result &&
83608438
(!Result->getDeclContext()->isModuleScopeContext() ||
83618439
isa<ClangModuleUnit>(Result->getDeclContext()))) {
8440+
// For using declarations that expose a method of a base class, the Clang
8441+
// decl is synthesized lazily when the method is actually used from Swift.
8442+
bool hasSynthesizedClangNode =
8443+
isa<clang::UsingShadowDecl>(ClangDecl) && isa<FuncDecl>(Result);
8444+
83628445
// Either the Swift declaration was from stdlib,
83638446
// or we imported the underlying decl of the typedef,
83648447
// or we imported the decl itself.
83658448
bool ImportedCorrectly =
83668449
!Result->getClangDecl() || SkippedOverTypedef ||
8450+
hasSynthesizedClangNode ||
83678451
Result->getClangDecl()->getCanonicalDecl() == Canon;
83688452

83698453
// Or the other type is a typedef,
@@ -8386,7 +8470,7 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
83868470
}
83878471
assert(ImportedCorrectly);
83888472
}
8389-
assert(Result->hasClangNode());
8473+
assert(Result->hasClangNode() || hasSynthesizedClangNode);
83908474
}
83918475
#else
83928476
(void)SkippedOverTypedef;

lib/ClangImporter/ImportName.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,6 +1523,17 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
15231523
return ImportedName();
15241524
result.effectiveContext = effectiveCtx;
15251525

1526+
// If this is a using declaration, import the name of the shadowed decl and
1527+
// adjust the context.
1528+
if (auto usingShadowDecl = dyn_cast<clang::UsingShadowDecl>(D)) {
1529+
auto targetDecl = usingShadowDecl->getTargetDecl();
1530+
if (isa<clang::CXXMethodDecl>(targetDecl)) {
1531+
ImportedName baseName = importName(targetDecl, version, givenName);
1532+
baseName.effectiveContext = effectiveCtx;
1533+
return baseName;
1534+
}
1535+
}
1536+
15261537
// Gather information from the swift_async attribute, if there is one.
15271538
llvm::Optional<unsigned> completionHandlerParamIndex;
15281539
bool completionHandlerFlagIsZeroOnError = false;

lib/ClangImporter/ImporterImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,9 +663,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
663663
llvm::DenseMap<std::pair<ValueDecl *, DeclContext *>, ValueDecl *>
664664
clonedBaseMembers;
665665

666+
public:
666667
ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext);
667668

668-
public:
669669
static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl);
670670

671671
// Cache for already-specialized function templates and any thunks they may

lib/ClangImporter/SwiftLookupTable.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1991,6 +1991,12 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
19911991
}
19921992
}
19931993
}
1994+
if (auto usingDecl = dyn_cast<clang::UsingDecl>(named)) {
1995+
for (auto usingShadowDecl : usingDecl->shadows()) {
1996+
if (isa<clang::CXXMethodDecl>(usingShadowDecl->getTargetDecl()))
1997+
addEntryToLookupTable(table, usingShadowDecl, nameImporter);
1998+
}
1999+
}
19942000
}
19952001

19962002
/// Returns the nearest parent of \p module that is marked \c explicit in its

test/Interop/Cxx/class/Inputs/constructors.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,4 @@ struct DeletedCopyConstructor {
8080
DeletedCopyConstructor(const DeletedCopyConstructor &) = delete;
8181
};
8282

83-
// TODO: we should be able to import this constructor correctly. Until we can,
84-
// make sure not to crash.
85-
struct UsingBaseConstructor : ConstructorWithParam {
86-
using ConstructorWithParam::ConstructorWithParam;
87-
};
88-
8983
#endif // TEST_INTEROP_CXX_CLASS_INPUTS_CONSTRUCTORS_H

test/Interop/Cxx/class/inheritance/Inputs/module.modulemap

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ module TypeAliases {
2828
header "type-aliases.h"
2929
}
3030

31+
module UsingBaseMembers {
32+
header "using-base-members.h"
33+
requires cplusplus
34+
}
35+
3136
module VirtualMethods {
3237
header "virtual-methods.h"
3338
requires cplusplus

0 commit comments

Comments
 (0)