Skip to content

Commit be73254

Browse files
authored
[cxx-interop] Import private members (#78942)
This commit removes the guardrails in ImportDecl.cpp:SwiftDeclConverter that prevent it from importing non-public C++ members. It also accordingly adjusts all code that assumes generated Swift decls should be public. This commit does not import non-public inherited members; that needs its own follow-up patch. Note that Swift enforces stricter invariants about access levels than C++. For instance, public typealiases cannot be assigned private underlying types, and public functions cannot take or return private types. Meanwhile, both of these patterns are supported in C++, where exposing private types from a class's public interface is considered feature. As far as I am aware, Swift was already importing such private-containing public decls from C++ already, but I added a test suite, access inversion, that checks and documents this scenario, to ensure that it doesn't trip any assertions.
1 parent 8c19cd7 commit be73254

17 files changed

+859
-205
lines changed

include/swift/ClangImporter/ClangImporter.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
#ifndef SWIFT_CLANG_IMPORTER_H
1717
#define SWIFT_CLANG_IMPORTER_H
1818

19+
#include "swift/AST/AttrKind.h"
1920
#include "swift/AST/ClangModuleLoader.h"
21+
#include "clang/Basic/Specifiers.h"
2022
#include "llvm/Support/VirtualFileSystem.h"
2123

2224
/// The maximum number of SIMD vector elements we currently try to import.
@@ -728,7 +730,11 @@ llvm::StringRef getCFTypeName(const clang::TypedefNameDecl *decl);
728730
ValueDecl *getImportedMemberOperator(const DeclBaseName &name,
729731
NominalTypeDecl *selfType,
730732
std::optional<Type> parameterType);
731-
733+
/// Map the access specifier of a Clang record member to a Swift access level.
734+
///
735+
/// This mapping is conservative: the resulting Swift access should be at _most_
736+
/// as permissive as the input C++ access.
737+
AccessLevel convertClangAccess(clang::AccessSpecifier access);
732738
} // namespace importer
733739

734740
struct ClangInvocationFileMapping {

include/swift/ClangImporter/ClangModule.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class ClangModuleUnit final : public LoadedFile {
106106

107107
Identifier
108108
getDiscriminatorForPrivateDecl(const Decl *D) const override {
109-
llvm_unreachable("no private decls in Clang modules");
109+
llvm_unreachable("Clang modules do not need discriminators");
110110
}
111111

112112
virtual version::Version getLanguageVersionBuiltWith() const override {

lib/AST/ASTMangler.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "swift/Basic/Defer.h"
4242
#include "swift/Basic/SourceManager.h"
4343
#include "swift/ClangImporter/ClangImporter.h"
44+
#include "swift/ClangImporter/ClangModule.h"
4445
#include "swift/Demangling/Demangler.h"
4546
#include "swift/Demangling/ManglingMacros.h"
4647
#include "swift/Demangling/ManglingUtils.h"
@@ -53,8 +54,8 @@
5354
#include "clang/AST/Mangle.h"
5455
#include "clang/Basic/CharInfo.h"
5556
#include "llvm/ADT/DenseMap.h"
56-
#include "llvm/ADT/SmallString.h"
5757
#include "llvm/ADT/STLExtras.h"
58+
#include "llvm/ADT/SmallString.h"
5859
#include "llvm/ADT/StringRef.h"
5960
#include "llvm/Support/CommandLine.h"
6061
#include "llvm/Support/ErrorHandling.h"
@@ -1092,6 +1093,11 @@ static StringRef getPrivateDiscriminatorIfNecessary(const Decl *decl) {
10921093
auto topLevelSubcontext = decl->getDeclContext()->getModuleScopeContext();
10931094
auto fileUnit = cast<FileUnit>(topLevelSubcontext);
10941095

1096+
// Clang modules do not provide a namespace, so no discriminator is needed
1097+
// here, even for non-public declarations.
1098+
if (isa<ClangModuleUnit>(fileUnit))
1099+
return StringRef();
1100+
10951101
Identifier discriminator =
10961102
fileUnit->getDiscriminatorForPrivateDecl(decl);
10971103
assert(!discriminator.empty());

lib/ClangImporter/ClangImporter.cpp

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6160,6 +6160,8 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
61606160
NominalTypeDecl *inheritingDecl = desc.inheritingDecl;
61616161
DeclName name = desc.name;
61626162

6163+
bool inheritedLookup = recordDecl != inheritingDecl;
6164+
61636165
auto &ctx = recordDecl->getASTContext();
61646166
auto directResults = evaluateOrDefault(
61656167
ctx.evaluator,
@@ -6185,7 +6187,7 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
61856187

61866188
// If this member is found due to inheritance, clone it from the base class
61876189
// by synthesizing getters and setters.
6188-
if (inheritingDecl != recordDecl) {
6190+
if (inheritedLookup) {
61896191
imported = clangModuleLoader->importBaseMemberDecl(
61906192
cast<ValueDecl>(imported), inheritingDecl);
61916193
if (!imported)
@@ -6194,8 +6196,8 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
61946196
result.push_back(cast<ValueDecl>(imported));
61956197
}
61966198

6197-
if (inheritingDecl != recordDecl) {
6198-
// For inheritied members, add members that are synthesized eagerly, such as
6199+
if (inheritedLookup) {
6200+
// For inherited members, add members that are synthesized eagerly, such as
61996201
// subscripts. This is not necessary for non-inherited members because those
62006202
// should already be in the lookup table.
62016203
for (auto member :
@@ -8283,3 +8285,34 @@ bool importer::isCxxConstReferenceType(const clang::Type *type) {
82838285
auto pointeeType = getCxxReferencePointeeTypeOrNone(type);
82848286
return pointeeType && pointeeType->isConstQualified();
82858287
}
8288+
8289+
AccessLevel importer::convertClangAccess(clang::AccessSpecifier access) {
8290+
switch (access) {
8291+
case clang::AS_public:
8292+
// C++ 'public' is actually closer to Swift 'open' than Swift 'public',
8293+
// since C++ 'public' does not prevent users from subclassing a type or
8294+
// overriding a method. However, subclassing and overriding are currently
8295+
// unsupported across the interop boundary, so we conservatively map C++
8296+
// 'public' to Swift 'public' in case there are other C++ subtleties that
8297+
// are being missed at this time (e.g., C++ 'final' vs Swift 'final').
8298+
return AccessLevel::Public;
8299+
8300+
case clang::AS_protected:
8301+
// Swift does not have a notion of protected fields, so map C++ 'protected'
8302+
// to Swift 'private'.
8303+
return AccessLevel::Private;
8304+
8305+
case clang::AS_private:
8306+
// N.B. Swift 'private' is more restrictive than C++ 'private' because it
8307+
// also cares about what source file the member is accessed.
8308+
return AccessLevel::Private;
8309+
8310+
case clang::AS_none:
8311+
// The fictional 'none' specifier is given to top-level C++ declarations,
8312+
// for which C++ lacks the syntax to give an access specifier. (It may also
8313+
// be used in other cases I'm not aware of.) Those declarations are globally
8314+
// visible and thus correspond to Swift 'public' (with the same caveats
8315+
// about Swift 'public' vs 'open'; see above).
8316+
return AccessLevel::Public;
8317+
}
8318+
}

0 commit comments

Comments
 (0)