Skip to content

Commit 36aaa67

Browse files
author
Gabor Horvath
committed
[cxx-interop] Fix a rare compilation error in reverse interop header
To trigger this error one needs to import a nested type from C++, use it in a generic context in Swift, and export it back to C++. We were inconsisent in what namespace did we declare the functions to get the type metadata for types. It was in the swift namespace for foreign types and in the module namespace for Swift types. This PR standardizes on how the metadata function is declared and called to fix the issue. Fixes #80538. rdar://148597079
1 parent 1ea5458 commit 36aaa67

File tree

9 files changed

+77
-29
lines changed

9 files changed

+77
-29
lines changed

include/swift/ClangImporter/ClangImporter.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,9 @@ getPrivateFileIDAttrs(const clang::CXXRecordDecl *decl);
773773
///
774774
/// Returns false if \a decl was not imported by ClangImporter.
775775
bool declIsCxxOnly(const Decl *decl);
776+
777+
/// Is this DeclContext an `enum` that represents a C++ namespace?
778+
bool isClangNamespace(const DeclContext *dc);
776779
} // namespace importer
777780

778781
struct ClangInvocationFileMapping {

lib/ClangImporter/ClangImporter.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8793,3 +8793,10 @@ bool importer::declIsCxxOnly(const Decl *decl) {
87938793
}
87948794
return false;
87958795
}
8796+
8797+
bool importer::isClangNamespace(const DeclContext *dc) {
8798+
if (const auto *ed = dc->getSelfEnumDecl())
8799+
return llvm::isa_and_nonnull<clang::NamespaceDecl>(ed->getClangDecl());
8800+
8801+
return false;
8802+
}

lib/ClangImporter/ImportDecl.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
#include "llvm/ADT/StringMap.h"
7676
#include "llvm/ADT/StringSwitch.h"
7777
#include "llvm/ADT/TinyPtrVector.h"
78+
#include "llvm/Support/Casting.h"
7879
#include "llvm/Support/Path.h"
7980

8081
#include <algorithm>
@@ -3730,13 +3731,6 @@ namespace {
37303731
return nullptr;
37313732
}
37323733

3733-
static bool isClangNamespace(const DeclContext *dc) {
3734-
if (const auto *ed = dc->getSelfEnumDecl())
3735-
return isa<clang::NamespaceDecl>(ed->getClangDecl());
3736-
3737-
return false;
3738-
}
3739-
37403734
Decl *importFunctionDecl(
37413735
const clang::FunctionDecl *decl, ImportedName importedName,
37423736
std::optional<ImportedName> correctSwiftName,

lib/PrintAsClang/ClangSyntaxPrinter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ void ClangSyntaxPrinter::printNamespace(
207207
void ClangSyntaxPrinter::printParentNamespaceForNestedTypes(
208208
const ValueDecl *D, llvm::function_ref<void(raw_ostream &OS)> bodyPrinter,
209209
NamespaceTrivia trivia) const {
210-
if (!isa_and_nonnull<NominalTypeDecl>(D->getDeclContext()->getAsDecl())) {
210+
if (!isa_and_nonnull<NominalTypeDecl>(D->getDeclContext()->getAsDecl()) ||
211+
importer::isClangNamespace(D->getDeclContext())) {
211212
bodyPrinter(os);
212213
return;
213214
}

lib/PrintAsClang/PrintClangValueType.cpp

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -630,18 +630,21 @@ void ClangValueTypePrinter::printTypeGenericTraits(
630630
bool isOpaqueLayout) {
631631
auto *NTD = dyn_cast<NominalTypeDecl>(typeDecl);
632632
ClangSyntaxPrinter printer(typeDecl->getASTContext(), os);
633-
// FIXME: avoid popping out of the module's namespace here.
634-
os << "} // end namespace \n\n";
635-
os << "namespace swift SWIFT_PRIVATE_ATTR {\n";
636-
637633
if (typeDecl->hasClangNode()) {
638634
/// Print a reference to the type metadata function for a C++ type.
639-
ClangSyntaxPrinter(typeDecl->getASTContext(), os).printNamespace(
640-
cxx_synthesis::getCxxImplNamespaceName(), [&](raw_ostream &os) {
641-
ClangSyntaxPrinter(typeDecl->getASTContext(), os).printCTypeMetadataTypeFunction(
642-
typeDecl, typeMetadataFuncName, typeMetadataFuncRequirements);
643-
});
635+
printer.printParentNamespaceForNestedTypes(typeDecl, [&](raw_ostream &os) {
636+
printer.printNamespace(
637+
cxx_synthesis::getCxxImplNamespaceName(), [&](raw_ostream &os) {
638+
ClangSyntaxPrinter(typeDecl->getASTContext(), os)
639+
.printCTypeMetadataTypeFunction(typeDecl, typeMetadataFuncName,
640+
typeMetadataFuncRequirements);
641+
});
642+
});
644643
}
644+
645+
// FIXME: avoid popping out of the module's namespace here.
646+
os << "} // end namespace \n\n";
647+
os << "namespace swift SWIFT_PRIVATE_ATTR {\n";
645648
auto classDecl = dyn_cast<ClassDecl>(typeDecl);
646649
bool addPointer =
647650
typeDecl->isObjC() || (classDecl && classDecl->isForeignReferenceType());
@@ -676,10 +679,11 @@ void ClangValueTypePrinter::printTypeGenericTraits(
676679
ClangSyntaxPrinter(typeDecl->getASTContext(), os).printInlineForHelperFunction();
677680
os << "void * _Nonnull getTypeMetadata() {\n";
678681
os << " return ";
679-
if (!typeDecl->hasClangNode()) {
682+
if (typeDecl->hasClangNode())
683+
printer.printBaseName(moduleContext);
684+
else
680685
printer.printBaseName(typeDecl->getModuleContext());
681-
os << "::";
682-
}
686+
os << "::";
683687
if (!printer.printNestedTypeNamespaceQualifiers(typeDecl))
684688
os << "::";
685689
os << cxx_synthesis::getCxxImplNamespaceName() << "::";

test/Interop/CxxToSwiftToCxx/bridge-cxx-struct-back-to-cxx.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,7 @@ public struct Strct {
200200
// CHECK: ns::ImmortalTemplate<int> *_Nonnull retImmortalTemplate() noexcept SWIFT_SYMBOL({{.*}}) SWIFT_WARN_UNUSED_RESULT {
201201
// CHECK-NEXT: return UseCxxTy::_impl::$s8UseCxxTy19retImmortalTemplateSo2nsO0028ImmortalTemplateCInt_jBAGgnbVyF();
202202
// CHECK-NEXT: }
203-
204-
// CHECK: } // end namespace
205203
// CHECK-EMPTY:
206-
// CHECK-NEXT: namespace swift SWIFT_PRIVATE_ATTR {
207204
// CHECK-NEXT: namespace _impl {
208205
// CHECK-EMPTY:
209206
// CHECK-NEXT: // Type metadata accessor for NonTrivialTemplateInt
@@ -212,14 +209,17 @@ public struct Strct {
212209
// CHECK-EMPTY:
213210
// CHECK-NEXT: } // namespace _impl
214211
// CHECK-EMPTY:
212+
// CHECK-NEXT: } // end namespace
213+
// CHECK-EMPTY:
214+
// CHECK-NEXT: namespace swift SWIFT_PRIVATE_ATTR {
215215
// CHECK-NEXT: #pragma clang diagnostic push
216216
// CHECK-NEXT: #pragma clang diagnostic ignored "-Wc++17-extensions"
217217
// CHECK-NEXT: template<>
218218
// CHECK-NEXT: inline const constexpr bool isUsableInGenericContext<ns::NonTrivialTemplateInt> = true;
219219
// CHECK-NEXT: template<>
220220
// CHECK-NEXT: struct TypeMetadataTrait<ns::NonTrivialTemplateInt> {
221221
// CHECK-NEXT: static SWIFT_INLINE_PRIVATE_HELPER void * _Nonnull getTypeMetadata() {
222-
// CHECK-NEXT: return _impl::$sSo2nsO0030NonTrivialTemplateCInt_hHAFhrbVMa(0)._0;
222+
// CHECK-NEXT: return UseCxxTy::_impl::$sSo2nsO0030NonTrivialTemplateCInt_hHAFhrbVMa(0)._0;
223223
// CHECK-NEXT: }
224224
// CHECK-NEXT: };
225225
// CHECK-NEXT: namespace _impl{
@@ -240,9 +240,6 @@ public struct Strct {
240240
// CHECK-NEXT: return result;
241241
// CHECK-NEXT: }
242242
// CHECK-EMPTY:
243-
// CHECK-NEXT: } // end namespace
244-
// CHECK-EMPTY:
245-
// CHECK-NEXT: namespace swift SWIFT_PRIVATE_ATTR {
246243
// CHECK-NEXT: namespace _impl {
247244
// CHECK-EMPTY:
248245
// CHECK-NEXT: // Type metadata accessor for NonTrivialTemplateTrivial
@@ -251,14 +248,17 @@ public struct Strct {
251248
// CHECK-EMPTY:
252249
// CHECK-NEXT: } // namespace _impl
253250
// CHECK-EMPTY:
251+
// CHECK-NEXT: } // end namespace
252+
// CHECK-EMPTY:
253+
// CHECK-NEXT: namespace swift SWIFT_PRIVATE_ATTR {
254254
// CHECK-NEXT: #pragma clang diagnostic push
255255
// CHECK-NEXT: #pragma clang diagnostic ignored "-Wc++17-extensions"
256256
// CHECK-NEXT: template<>
257257
// CHECK-NEXT: inline const constexpr bool isUsableInGenericContext<ns::NonTrivialTemplateTrivial> = true;
258258
// CHECK-NEXT: template<>
259259
// CHECK-NEXT: struct TypeMetadataTrait<ns::NonTrivialTemplateTrivial> {
260260
// CHECK-NEXT: static SWIFT_INLINE_PRIVATE_HELPER void * _Nonnull getTypeMetadata() {
261-
// CHECK-NEXT: return _impl::$sSo2nsO0042NonTrivialTemplatensTrivialinNS_HlGFlenawcVMa(0)._0;
261+
// CHECK-NEXT: return UseCxxTy::_impl::$sSo2nsO0042NonTrivialTemplatensTrivialinNS_HlGFlenawcVMa(0)._0;
262262
// CHECK-NEXT: }
263263
// CHECK-NEXT: };
264264
// CHECK-NEXT: namespace _impl{
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// RUN: %target-swift-frontend %t/use-cxx-types.swift -module-name UseCxxTy -typecheck -verify -emit-clang-header-path %t/UseCxxTy.h -I %t -enable-experimental-cxx-interop -clang-header-expose-decls=all-public -disable-availability-checking
5+
// RUN: cat %t/header.h >> %t/full-header.h
6+
// RUN: cat %t/UseCxxTy.h >> %t/full-header.h
7+
// RUN: %target-interop-build-clangxx -std=c++20 -c -xc++-header %t/full-header.h -o %t/o.o
8+
9+
// UN: %FileCheck %s < %t/UseCxxTy.h
10+
11+
//--- header.h
12+
13+
struct Cell { class Visitor {}; };
14+
15+
//--- module.modulemap
16+
module CxxTest {
17+
header "header.h"
18+
requires cplusplus
19+
}
20+
21+
//--- use-cxx-types.swift
22+
import CxxTest
23+
24+
public extension Cell.Visitor {
25+
func visit() {}
26+
}
27+
28+
public func f() -> [Cell.Visitor] {
29+
}
30+
31+
// CHECK: fooBarBaz

test/Interop/ObjCToSwiftToObjCxx/bridge-objc-types-back-to-objcxx.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public func retObjCClassArray() -> [ObjCKlass] {
9494
// CHECK-NEXT: template<>
9595
// CHECK-NEXT: struct TypeMetadataTrait<ObjCKlass*> {
9696
// CHECK-NEXT: static SWIFT_INLINE_PRIVATE_HELPER void * _Nonnull getTypeMetadata() {
97-
// CHECK-NEXT: return _impl::$sSo9ObjCKlassCMa(0)._0;
97+
// CHECK-NEXT: return UseObjCTy::_impl::$sSo9ObjCKlassCMa(0)._0;
9898
// CHECK-NEXT: }
9999
// CHECK-NEXT: };
100100

test/Interop/SwiftToCxx/structs/nested-structs-in-cxx.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,11 @@ public class TestObject {
6868
case invalid
6969
}
7070
}
71+
72+
extension RecordConfig.File {
73+
public func getFileExtension() -> String { ".wav" }
74+
}
75+
76+
public func getFiles() -> [RecordConfig.File] {
77+
[]
78+
}

0 commit comments

Comments
 (0)