Skip to content

[cxx-interop] Adding swift_name attributes to virtual methods overrides #83067

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
Jul 18, 2025
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 include/swift/AST/DiagnosticsClangImporter.def
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ GROUPED_WARNING(inconsistent_swift_name, ClangDeclarationImport, none,
(bool, StringRef, StringRef, DeclName, StringRef, DeclName,
StringRef))

WARNING(swift_name_attr_ignored, none,
"ignoring swift_name attribute %0; swift_name attributes have no "
"effect on method overrides",
(DeclName))

GROUPED_WARNING(swift_name_circular_context_import, ClangDeclarationImport, none,
"cycle detected while resolving '%0' in swift_name attribute for '%1'",
(StringRef, StringRef))
Expand Down
11 changes: 5 additions & 6 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6336,11 +6336,9 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
// Capture the arity of already found members in the
// current record, to avoid adding ambiguous members
// from base classes.
const auto getArity =
ClangImporter::Implementation::getImportedBaseMemberDeclArity;
llvm::SmallSet<size_t, 4> foundNameArities;
llvm::SmallSet<DeclName, 4> foundMethodNames;
for (const auto *valueDecl : result)
foundNameArities.insert(getArity(valueDecl));
foundMethodNames.insert(valueDecl->getName());

for (auto base : cxxRecord->bases()) {
if (skipIfNonPublic && base.getAccessSpecifier() != clang::AS_public)
Expand Down Expand Up @@ -6374,9 +6372,9 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
{});

for (auto foundInBase : baseResults) {
// Do not add duplicate entry with the same arity,
// Do not add duplicate entry with the same DeclName,
// as that would cause an ambiguous lookup.
if (foundNameArities.count(getArity(foundInBase)))
if (foundMethodNames.count(foundInBase->getName()))
continue;

collector.add(foundInBase);
Expand Down Expand Up @@ -7671,6 +7669,7 @@ ValueDecl *ClangImporter::Implementation::importBaseMemberDecl(
auto known = clonedBaseMembers.find(key);
if (known == clonedBaseMembers.end()) {
ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext, inheritance);
handleAmbiguousSwiftName(cloned);
known = clonedBaseMembers.insert({key, cloned}).first;
clonedMembers.insert(std::make_pair(cloned, decl));
}
Expand Down
55 changes: 54 additions & 1 deletion lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "swift/ClangImporter/ClangImporterRequests.h"
#include "swift/ClangImporter/ClangModule.h"
#include "swift/Parse/Lexer.h"
#include "swift/Parse/ParseDeclName.h"
#include "swift/Strings.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
Expand Down Expand Up @@ -4424,7 +4425,43 @@ namespace {
// Create a thunk that will perform dynamic dispatch.
// TODO: we don't have to import the actual `method` in this case,
// we can just synthesize a thunk and import that instead.
auto result = synthesizer.makeVirtualMethod(decl);

FuncDecl *result;
if (decl->size_overridden_methods() > 0) {
if (auto swiftNameAttr = decl->getAttr<clang::SwiftNameAttr>()) {
auto parsedDeclName = parseDeclName(swiftNameAttr->getName());
auto swiftDeclName =
parsedDeclName.formDeclName(method->getASTContext());
ImportedName importedName;
std::tie(importedName, std::ignore) = importFullName(decl);

result = synthesizer.makeVirtualMethod(decl);

if (swiftDeclName != importedName.getDeclName()) {
Impl.diagnose(HeaderLoc(swiftNameAttr->getLoc()),
diag::swift_name_attr_ignored, swiftDeclName);

Impl.markUnavailable(
result, (llvm::Twine("ignoring swift_name '") +
swiftNameAttr->getName() + "' in '" +
decl->getParent()->getName() +
"'; swift_name attributes have no effect "
"on method overrides")
.str());
}
} else {
// If there's no swift_name attribute, we don't import this method.
// This is because if the overridden method was renamed and
// this one is not, we want to use the overridden method's name.
// This is reasonable because `makeVirtualMethod` returns
// a thunk that will perform dynamic dispatch, and consequently
// the correct instance of the method will get executed.
return nullptr;
}
} else {
result = synthesizer.makeVirtualMethod(decl);
}

if (result) {
return result;
} else {
Expand Down Expand Up @@ -10367,6 +10404,22 @@ ValueDecl *ClangImporter::Implementation::createUnavailableDecl(
return var;
}

void ClangImporter::Implementation::handleAmbiguousSwiftName(ValueDecl *decl) {
if (!decl || decl->isUnavailable())
return;

auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(
decl->getDeclContext()->getAsDecl()->getClangDecl());

if (!cxxRecordDecl)
return;

if (findUnavailableMethod(cxxRecordDecl, decl->getName())) {
markUnavailable(decl,
"overrides multiple C++ methods with different Swift names");
}
}

// Force the members of the entire inheritance hierarchy to be loaded and
// deserialized before loading the members of this class. This allows the
// decl members table to be warmed up and enables the correct identification of
Expand Down
42 changes: 42 additions & 0 deletions lib/ClangImporter/ImportName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1706,6 +1706,12 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
}
}

// `swift_name` attribute is not supported in virtual methods overrides
if (auto method = dyn_cast<clang::CXXMethodDecl>(D)) {
if (method->isVirtual() && method->size_overridden_methods() > 0)
skipCustomName = true;
}

if (!skipCustomName) {
result.info.hasCustomName = true;
result.declName = parsedName.formDeclName(
Expand Down Expand Up @@ -2315,6 +2321,42 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
newName = baseName.substr(StringRef("__synthesizedVirtualCall_").size());
baseName = newName;
}
if (method->isVirtual()) {
// The name should be imported from the base method
if (method->size_overridden_methods() > 0) {
DeclName overriddenName;
bool foundDivergentMethod = false;
for (auto overriddenMethod : method->overridden_methods()) {
ImportedName importedName =
importName(overriddenMethod, version, givenName);
if (!overriddenName) {
overriddenName = importedName.getDeclName();
} else if (overriddenName.compare(importedName.getDeclName())) {
importerImpl->insertUnavailableMethod(method->getParent(),
importedName.getDeclName());
foundDivergentMethod = true;
}
}

if (foundDivergentMethod) {
// The method we want to mark as unavailable will be generated
// lazily, when we clone the methods from base classes to the derived
// class method->getParent().
// Since we don't have the actual method here, we store this
// information to be accessed when we generate the actual method.
importerImpl->insertUnavailableMethod(method->getParent(),
overriddenName);
return ImportedName();
}

baseName = overriddenName.getBaseIdentifier().str();
// Also inherit argument names from base method
argumentNames.clear();
llvm::for_each(overriddenName.getArgumentNames(), [&](Identifier arg) {
argumentNames.push_back(arg.str());
});
}
}
}

// swift_newtype-ed declarations may have common words with the type name
Expand Down
20 changes: 20 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,14 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
// Map all cloned methods back to the original member
llvm::DenseMap<ValueDecl *, ValueDecl *> clonedMembers;

// Keep track of methods that are unavailale in each class.
// We need this set because these methods will be imported lazily. We don't
// have the corresponding Swift method when the availability check is
// performed, so instead we store the information in this set and then, when
// the method is finally generated, we check if it's present here
llvm::DenseSet<std::pair<const clang::CXXRecordDecl *, DeclName>>
unavailableMethods;

public:
llvm::DenseMap<const clang::ParmVarDecl*, FuncDecl*> defaultArgGenerators;

Expand Down Expand Up @@ -722,6 +730,18 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
return getNameImporter().getEnumKind(decl);
}

bool findUnavailableMethod(const clang::CXXRecordDecl *classDecl,
DeclName name) {
return unavailableMethods.contains({classDecl, name});
}

void insertUnavailableMethod(const clang::CXXRecordDecl *classDecl,
DeclName name) {
unavailableMethods.insert({classDecl, name});
}

void handleAmbiguousSwiftName(ValueDecl *decl);

private:
/// A mapping from imported declarations to their "alternate" declarations,
/// for cases where a single Clang declaration is imported to two
Expand Down
10 changes: 10 additions & 0 deletions lib/ClangImporter/SwiftLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "swift/Basic/STLExtras.h"
#include "swift/Basic/Version.h"
#include "swift/ClangImporter/ClangImporter.h"
#include "swift/Parse/ParseDeclName.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/Lex/MacroInfo.h"
Expand Down Expand Up @@ -1945,6 +1946,15 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
named, importedName.getEffectiveContext());
}

if (auto swiftNameAttr = named->getAttr<clang::SwiftNameAttr>()) {
auto parsedDeclName = parseDeclName(swiftNameAttr->getName());
auto swiftDeclName =
parsedDeclName.formDeclName(nameImporter.getContext());
if (importedName.getDeclName() != swiftDeclName)
table.addEntry(swiftDeclName, named,
importedName.getEffectiveContext());
}

return true;
});
if (failed) {
Expand Down
10 changes: 5 additions & 5 deletions test/IDE/dump_swift_lookup_tables.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,19 @@ import ImportAsMember
// CHECK-NEXT: TU: __swift
// CHECK-NEXT: adding:
// CHECK-NEXT: SNSomeStruct: SNAdding
// CHECK-NEXT: blue:
// CHECK: blue:
// CHECK-NEXT: SNColorChoice: SNColorBlue
// CHECK-NEXT: defaultValue:
// CHECK-NEXT: SNSomeStruct: SNSomeStructGetDefault, SNSomeStructSetDefault
// CHECK-NEXT: defaultX:
// CHECK: defaultX:
// CHECK-NEXT: SNSomeStruct: DefaultXValue
// CHECK-NEXT: foo:
// CHECK: foo:
// CHECK-NEXT: SNSomeStruct: SNSomeStructGetFoo, SNSomeStructSetFoo
// CHECK-NEXT: green:
// CHECK: green:
// CHECK-NEXT: SNColorChoice: SNColorGreen
// CHECK-NEXT: init:
// CHECK-NEXT: SNSomeStruct: SNCreate
// CHECK-NEXT: makeSomeStruct:
// CHECK: makeSomeStruct:
// CHECK-NEXT: TU: SNMakeSomeStruct, SNMakeSomeStructForX
// CHECK-NEXT: x:
// CHECK-NEXT: SNSomeStruct: X
Expand Down
135 changes: 135 additions & 0 deletions test/Interop/Cxx/foreign-reference/Inputs/member-inheritance.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,138 @@ inline const Immortal *_Nonnull castToImmortal(
const DerivedFromImmortal *_Nonnull immortal) {
return static_cast<const Immortal *>(immortal);
}

// A1
// / \
// B1 B2
// / \
// C1 C2

struct IMMORTAL_FRT A1 {
virtual int virtualMethod() const { return 111; }

__attribute__((swift_name("swiftFooRename()")))
virtual int fooRename() const { return 112; }

__attribute__((swift_name("swiftBarRename()")))
virtual int barRename() const { return 113; }

__attribute__((swift_name("swiftParamsRename(a1:)")))
virtual int paramsRename(int i) const { return i; }

static A1 *_Nonnull create() { return new A1(); }
};

struct B1 : A1 {
__attribute__((swift_name("swiftVirtualMethod()")))
virtual int virtualMethod() const override { return 211; }

virtual int fooRename() const override { return 212; }

__attribute__((swift_name("B1BarRename()")))
virtual int barRename() const override { return 213; }

__attribute__((swift_name("swiftParamsRename(b1:)")))
virtual int paramsRename(int i) const override { return i; }

static B1 *_Nonnull create() { return new B1(); }
};

struct B2 : A1 {
int virtualMethod() const { return 221; }

int fooRename() const { return 222; }

__attribute__((swift_name("B2BarRename()"))) int barRename() const {
return 223;
}

static B2 *_Nonnull create() { return new B2(); }
};

struct C1 : B1 {
__attribute__((swift_name("swiftFooRename()")))
virtual int fooRename() const override { return 312; }

__attribute__((swift_name("swiftBarRename()")))
virtual int barRename() const override { return 313; }

virtual int paramsRename(int i) const override { return i; }

static C1 *_Nonnull create() { return new C1(); }
};

struct C2 : B1 {
__attribute__((swift_name("swiftVirtualMethod()")))
virtual int virtualMethod() const override { return 321; }

__attribute__((swift_name("C2FooRename()")))
virtual int fooRename() const override { return 322; }

__attribute__((swift_name("B1BarRename()")))
virtual int barRename() const override { return 323; }

__attribute__((swift_name("swiftParamsRename(b1:)")))
virtual int paramsRename(int i) const override { return i; }

static C2 *_Nonnull create() { return new C2(); }
};

struct IMMORTAL_FRT A2 {
__attribute__((swift_name("swiftVirtualMethod()")))
virtual int virtualMethod() const { return 121; }

__attribute__((swift_name("swiftFooRename()")))
virtual int fooRename() const { return 122; }

__attribute__((swift_name("A2BarRename()")))
virtual int barRename() const { return 123; }

__attribute__((swift_name("swiftParamsRename(a2:)"))) virtual int
paramsRename(int i) const {
return i + 1;
}

static A2 *_Nonnull create() { return new A2(); }
};

// A1 A2
// \ /
// D1

struct D1 : A1, A2 {
static D1 *_Nonnull create() { return new D1(); }
};

// A1 A2
// \ /
// B1 /
// \ /
// D2

struct D2 : B1, A2 {
__attribute__((swift_name("swiftVirtualMethod()")))
virtual int virtualMethod() const override { return 411; }

virtual int fooRename() const override { return 412; }

virtual int barRename() const override { return 413; }

virtual int paramsRename(int i) const override { return i; }
};

// A1
// / \
// / \
// B1 B2
// |\ /|
// | \ / |
// | D3 |
// C1 |
// \ |
// \ /
// D4

struct D3 : B1, B2 {};

struct D4 : C1, B2 {};
Loading