Skip to content

IRGen: Only skip round-trip demangler check for types that involve C++ types #85288

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.

Sign up for GitHub

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
Nov 4, 2025
Merged

IRGen: Only skip round-trip demangler check for types that involve C++ types #85288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 12 additions & 1 deletion lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,18 @@ void ModuleDecl::lookupMember(SmallVectorImpl<ValueDecl*> &results,
} else if (privateDiscriminator.empty()) {
auto newEnd = std::remove_if(results.begin()+oldSize, results.end(),
[](const ValueDecl *VD) -> bool {
return VD->getFormalAccess() <= AccessLevel::FilePrivate;
// FIXME: The ClangImporter sometimes generates private declarations but
// doesn't give their file unit a private discriminator so they mangle
// incorrectly.
//
// The hasClangNode() carveout makes the ASTDemangler work properly in
// this case.
//
// We should fix things up so that such declarations also mangle with a
// a private discriminator, in which case this entry point will be called
// with the right parameters so that this isn't needed.
return (VD->getFormalAccess() <= AccessLevel::FilePrivate &&
!VD->hasClangNode());
Copy link
Contributor

@j-hui j-hui Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that this won't work for inherited members like methods and fields.

We import those as a synthesized Swift shim that calls a synthesized C++ forwarding function that accesses the underlying inherited member (doing so isolates Swift from some of the nuances of C++ inheritance, whose logic we'd like to not duplicate from Clang).

However, the synthesized Swift shim is not associated with any Clang node, and will thus be missed by the check here. And I don't think we have any mechanism for saying "this node was imported from Clang but I don't have a Clang node pointer to show for it."

I'm wondering if the right thing to do here is to just introduce a private discriminator for anything imported from C++? And what would be the requirements for such a discriminator? All non-public decls in C++ are members of record types, i.e., not top-level, so I wonder if it suffices to just have Clang module units to just all share __ObjC or something like that as their private discriminator.

Copy link
Contributor Author

@slavapestov slavapestov Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the round-trip check is only enabled in asserts builds by default, can we leave that for a follow-up PR? I agree that fixing the private discriminator will allow this part of the hack to be removed.

Copy link
Contributor

@j-hui j-hui Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that sounds good to me!

});
results.erase(newEnd, results.end());

Expand Down
45 changes: 40 additions & 5 deletions lib/IRGen/IRGenDebugInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1075,12 +1075,11 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
CanonicalName.clear();
}

bool IsTypeOriginallyDefinedIn =
containsOriginallyDefinedIn(DbgTy.getType());
// TODO(https://github.com/apple/swift/issues/57699): We currently cannot round trip some C++ types.
bool IsTypeOriginallyDefinedIn = containsOriginallyDefinedIn(DbgTy.getType());
bool IsCxxType = containsCxxType(DbgTy.getType());
// There's no way to round trip when respecting @_originallyDefinedIn for a type.
if (!Opts.DisableRoundTripDebugTypes &&
!Ty->getASTContext().LangOpts.EnableCXXInterop && !IsTypeOriginallyDefinedIn) {
// TODO(https://github.com/apple/swift/issues/57699): We currently cannot round trip some C++ types.
if (!Opts.DisableRoundTripDebugTypes && !IsTypeOriginallyDefinedIn && !IsCxxType) {
// Make sure we can reconstruct mangled types for the debugger.
auto &Ctx = Ty->getASTContext();
Type Reconstructed = Demangle::getTypeForMangling(Ctx, SugaredName, Sig);
Expand Down Expand Up @@ -2504,6 +2503,42 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
return Walker.visitedOriginallyDefinedIn;
}

/// Returns true if the type contains an imported C++ type. Due to
/// various unimplemented features these cannot round-trip through
/// the ASTDemangler.
///
/// FIXME: Get these cases working with the ASTDemangler instead.
bool containsCxxType(Type T) {
return T.findIf([&](Type t) -> bool {
if (auto *decl = t->getAnyNominal()) {
if (auto *clangDecl = decl->getClangDecl()) {
// Lookup of template instantiations is not implemented.
if (isa<clang::ClassTemplateSpecializationDecl>(clangDecl))
return true;

// Lookup of types in weird contexts is not implemented.
if (isa<clang::EnumDecl>(clangDecl) ||
isa<clang::CXXRecordDecl>(clangDecl)) {
auto *dc = clangDecl->getDeclContext();
while (!isa<clang::TranslationUnitDecl>(dc)) {
// ... in namespaces,
if (isa<clang::NamespaceDecl>(dc))
return true;

// ... or inside other types.
if (isa<clang::CXXRecordDecl>(dc))
return true;

dc = dc->getParent();
}
}
}
}

return false;
});
}

/// Returns the decl of the type's parent chain annotated by
/// @_originallyDefinedIn. Returns null if no type is annotated.
NominalTypeDecl *getDeclAnnotatedByOriginallyDefinedIn(DebugTypeInfo DbgTy) {
Expand Down