Skip to content

Commit b3b2031

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 b3b2031

File tree

9 files changed

+73
-29
lines changed

9 files changed

+73
-29
lines changed

include/swift/ClangImporter/ClangImporter.h

+3
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

+7
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 isa_and_nonnull<clang::NamespaceDecl>(ed->getClangDecl());
8800+
8801+
return false;
8802+
}

lib/ClangImporter/ImportDecl.cpp

+1-7
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

+2-1
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

+16-12
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

+8-8
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{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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+
//--- header.h
10+
11+
struct Cell { class Visitor {}; };
12+
13+
//--- module.modulemap
14+
module CxxTest {
15+
header "header.h"
16+
requires cplusplus
17+
}
18+
19+
//--- use-cxx-types.swift
20+
import CxxTest
21+
22+
public extension Cell.Visitor {
23+
func visit() {}
24+
}
25+
26+
public func f() -> [Cell.Visitor] {
27+
}

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

+1-1
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

+8
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)