Skip to content

[cxx-interop] Import private members #78942

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 21 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
469f34e
Clean up ClangRecordMemberLookup::evaluate slightly
j-hui Jan 22, 2025
cffc639
Add importer::convertClangAccess helper
j-hui Jan 22, 2025
bd82933
Import non-public C++ members
j-hui Jan 23, 2025
5ad0a8a
Remove assumptions of AccessLevel::Public from SwiftDeclSynth
j-hui Jan 27, 2025
7d0ba28
Make default arg functions private static functions instead of public…
j-hui Jan 27, 2025
f1adfcf
Do not try to retrieve discriminator for Clang module members
j-hui Jan 27, 2025
4b263b2
Revert "Make default arg functions private static functions instead o…
j-hui Jan 27, 2025
c71d340
Fix some test cases (but it looks like there are still actual bugs too
j-hui Jan 27, 2025
9005ea6
Merge remote-tracking branch 'origin/main' into import-private-members
j-hui Jan 27, 2025
6843ba4
Fix missing cases that were assuming C++ public
j-hui Jan 27, 2025
e8cf93e
Format ImportDecl.cpp
j-hui Jan 27, 2025
d3a55a4
Add test case checking a leaky C++ record type
j-hui Jan 29, 2025
2327245
Add tests that check default arguments
j-hui Jan 30, 2025
1d97e03
Add executable test for access inversion
j-hui Jan 29, 2025
846f017
Add some tests showing what happens when you declare global vars with…
j-hui Jan 30, 2025
c2e03f2
Add module interface test
j-hui Jan 30, 2025
7b52095
Remove comments
j-hui Jan 30, 2025
f8decef
Formatting
j-hui Jan 30, 2025
5e9a0c8
Added rdar to unimported enums
j-hui Jan 30, 2025
adfdb76
Update comments and links in tests
j-hui Jan 30, 2025
247609b
Make underlying type for enums generic
j-hui Jan 30, 2025
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
8 changes: 7 additions & 1 deletion include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
#ifndef SWIFT_CLANG_IMPORTER_H
#define SWIFT_CLANG_IMPORTER_H

#include "swift/AST/AttrKind.h"
#include "swift/AST/ClangModuleLoader.h"
#include "clang/Basic/Specifiers.h"
#include "llvm/Support/VirtualFileSystem.h"

/// The maximum number of SIMD vector elements we currently try to import.
Expand Down Expand Up @@ -728,7 +730,11 @@ llvm::StringRef getCFTypeName(const clang::TypedefNameDecl *decl);
ValueDecl *getImportedMemberOperator(const DeclBaseName &name,
NominalTypeDecl *selfType,
std::optional<Type> parameterType);

/// Map the access specifier of a Clang record member to a Swift access level.
///
/// This mapping is conservative: the resulting Swift access should be at _most_
/// as permissive as the input C++ access.
AccessLevel convertClangAccess(clang::AccessSpecifier access);
} // namespace importer

struct ClangInvocationFileMapping {
Expand Down
2 changes: 1 addition & 1 deletion include/swift/ClangImporter/ClangModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class ClangModuleUnit final : public LoadedFile {

Identifier
getDiscriminatorForPrivateDecl(const Decl *D) const override {
llvm_unreachable("no private decls in Clang modules");
llvm_unreachable("Clang modules do not need discriminators");
}

virtual version::Version getLanguageVersionBuiltWith() const override {
Expand Down
8 changes: 7 additions & 1 deletion lib/AST/ASTMangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "swift/Basic/Defer.h"
#include "swift/Basic/SourceManager.h"
#include "swift/ClangImporter/ClangImporter.h"
#include "swift/ClangImporter/ClangModule.h"
#include "swift/Demangling/Demangler.h"
#include "swift/Demangling/ManglingMacros.h"
#include "swift/Demangling/ManglingUtils.h"
Expand All @@ -53,8 +54,8 @@
#include "clang/AST/Mangle.h"
#include "clang/Basic/CharInfo.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/ErrorHandling.h"
Expand Down Expand Up @@ -1092,6 +1093,11 @@ static StringRef getPrivateDiscriminatorIfNecessary(const Decl *decl) {
auto topLevelSubcontext = decl->getDeclContext()->getModuleScopeContext();
auto fileUnit = cast<FileUnit>(topLevelSubcontext);

// Clang modules do not provide a namespace, so no discriminator is needed
// here, even for non-public declarations.
if (isa<ClangModuleUnit>(fileUnit))
return StringRef();

Identifier discriminator =
fileUnit->getDiscriminatorForPrivateDecl(decl);
assert(!discriminator.empty());
Expand Down
39 changes: 36 additions & 3 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6152,6 +6152,8 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
NominalTypeDecl *inheritingDecl = desc.inheritingDecl;
DeclName name = desc.name;

bool inheritedLookup = recordDecl != inheritingDecl;

auto &ctx = recordDecl->getASTContext();
auto directResults = evaluateOrDefault(
ctx.evaluator,
Expand All @@ -6177,7 +6179,7 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(

// If this member is found due to inheritance, clone it from the base class
// by synthesizing getters and setters.
if (inheritingDecl != recordDecl) {
if (inheritedLookup) {
imported = clangModuleLoader->importBaseMemberDecl(
cast<ValueDecl>(imported), inheritingDecl);
if (!imported)
Expand All @@ -6186,8 +6188,8 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
result.push_back(cast<ValueDecl>(imported));
}

if (inheritingDecl != recordDecl) {
// For inheritied members, add members that are synthesized eagerly, such as
if (inheritedLookup) {
// For inherited members, add members that are synthesized eagerly, such as
// subscripts. This is not necessary for non-inherited members because those
// should already be in the lookup table.
for (auto member :
Expand Down Expand Up @@ -8275,3 +8277,34 @@ bool importer::isCxxConstReferenceType(const clang::Type *type) {
auto pointeeType = getCxxReferencePointeeTypeOrNone(type);
return pointeeType && pointeeType->isConstQualified();
}

AccessLevel importer::convertClangAccess(clang::AccessSpecifier access) {
switch (access) {
case clang::AS_public:
// C++ 'public' is actually closer to Swift 'open' than Swift 'public',
// since C++ 'public' does not prevent users from subclassing a type or
// overriding a method. However, subclassing and overriding are currently
// unsupported across the interop boundary, so we conservatively map C++
// 'public' to Swift 'public' in case there are other C++ subtleties that
// are being missed at this time (e.g., C++ 'final' vs Swift 'final').
return AccessLevel::Public;

case clang::AS_protected:
// Swift does not have a notion of protected fields, so map C++ 'protected'
// to Swift 'private'.
return AccessLevel::Private;

case clang::AS_private:
// N.B. Swift 'private' is more restrictive than C++ 'private' because it
// also cares about what source file the member is accessed.
return AccessLevel::Private;

case clang::AS_none:
// The fictional 'none' specifier is given to top-level C++ declarations,
// for which C++ lacks the syntax to give an access specifier. (It may also
// be used in other cases I'm not aware of.) Those declarations are globally
// visible and thus correspond to Swift 'public' (with the same caveats
// about Swift 'public' vs 'open'; see above).
return AccessLevel::Public;
}
}
Loading