Skip to content

Commit 71aa696

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
1 parent cf73629 commit 71aa696

17 files changed

+389
-93
lines changed

lib/ClangImporter/ClangImporter.cpp

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

lib/ClangImporter/ImportDecl.cpp

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

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

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

36243653
result->setIsObjC(false);
@@ -3931,18 +3960,31 @@ namespace {
39313960
}
39323961

39333962
Decl *VisitUsingDecl(const clang::UsingDecl *decl) {
3934-
// Using declarations are not imported.
3963+
// See VisitUsingShadowDecl below.
39353964
return nullptr;
39363965
}
39373966

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

39433982
ImportedName importedName;
39443983
llvm::Optional<ImportedName> correctSwiftName;
39453984
std::tie(importedName, correctSwiftName) = importFullName(decl);
3985+
// Don't import something that doesn't have a name.
3986+
if (importedName.getDeclName().isSpecial())
3987+
return nullptr;
39463988
auto Name = importedName.getDeclName().getBaseIdentifier();
39473989
if (Name.empty())
39483990
return nullptr;
@@ -3953,30 +3995,66 @@ namespace {
39533995
return importCompatibilityTypeAlias(decl, importedName,
39543996
*correctSwiftName);
39553997

3956-
auto DC =
3998+
auto importedDC =
39573999
Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
3958-
if (!DC)
4000+
if (!importedDC)
39594001
return nullptr;
39604002

3961-
Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
3962-
if (!SwiftDecl)
3963-
return nullptr;
4003+
if (isa<clang::TypeDecl>(decl->getTargetDecl())) {
4004+
Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
4005+
if (!SwiftDecl)
4006+
return nullptr;
39644007

3965-
const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
3966-
if (!SwiftTypeDecl)
3967-
return nullptr;
4008+
const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
4009+
if (!SwiftTypeDecl)
4010+
return nullptr;
39684011

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

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

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

83708454
// Or the other type is a typedef,
@@ -8387,7 +8471,7 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
83878471
}
83888472
assert(ImportedCorrectly);
83898473
}
8390-
assert(Result->hasClangNode());
8474+
assert(Result->hasClangNode() || hasSynthesizedClangNode);
83918475
}
83928476
#else
83938477
(void)SkippedOverTypedef;

lib/ClangImporter/ImportName.cpp

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

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