Skip to content

🍒[cxx-interop] Pull changes from swift-6 compat mode into swift-5.9 #73223

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 3 commits into from
Apr 25, 2024
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
4 changes: 0 additions & 4 deletions include/swift/AST/DiagnosticsClangImporter.def
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,6 @@ ERROR(conforms_to_ambiguous,none,
ERROR(conforms_to_not_protocol,none,
"%0 %1 referenced in protocol conformance '%2' is not a protocol", (DescriptiveDeclKind, ValueDecl *, StringRef))

ERROR(move_only_requires_move_only,none,
"use of noncopyable C++ type '%0' requires -enable-experimental-move-only",
(StringRef))

ERROR(failed_base_method_call_synthesis,none,
"failed to synthesize call to the base method %0 of type %0",
(ValueDecl *, ValueDecl *))
Expand Down
17 changes: 16 additions & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5709,6 +5709,8 @@ DeclAttributes cloneImportedAttributes(ValueDecl *decl, ASTContext &context) {

static ValueDecl *
cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
ASTContext &context = decl->getASTContext();

if (auto fn = dyn_cast<FuncDecl>(decl)) {
// TODO: function templates are specialized during type checking so to
// support these we need to tell Swift to type check the synthesized bodies.
Expand All @@ -5726,7 +5728,6 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
return nullptr;
}

ASTContext &context = decl->getASTContext();
auto out = FuncDecl::createImplicit(
context, fn->getStaticSpelling(), fn->getName(),
fn->getNameLoc(), fn->hasAsync(), fn->hasThrows(),
Expand Down Expand Up @@ -5762,6 +5763,20 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
}

if (auto var = dyn_cast<VarDecl>(decl)) {
auto oldContext = var->getDeclContext();
auto oldTypeDecl = oldContext->getSelfNominalTypeDecl();
// If the base type is non-copyable, and non-copyable generics are disabled,
// we cannot synthesize the accessor, because its implementation would use
// `UnsafePointer<BaseTy>`.
// We cannot use `ty->isNoncopyable()` here because that would create a
// cyclic dependency between ModuleQualifiedLookupRequest and
// LookupConformanceInModuleRequest, so we check for the presence of
// move-only attribute that is implicitly added to non-copyable C++ types by
// ClangImporter.
if (oldTypeDecl->getAttrs().hasAttribute<MoveOnlyAttr>() &&
!context.LangOpts.hasFeature(Feature::NoncopyableGenerics))
return nullptr;

auto rawMemory = allocateMemoryForDecl<VarDecl>(var->getASTContext(),
sizeof(VarDecl), false);
auto out =
Expand Down
97 changes: 39 additions & 58 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2192,22 +2192,12 @@ namespace {
Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;

if (recordHasMoveOnlySemantics(decl)) {
if (Impl.isCxxInteropCompatVersionAtLeast(6)) {
if (decl->isInStdNamespace() && decl->getName() == "promise") {
// Do not import std::promise.
return nullptr;
}
result->getAttrs().add(new (Impl.SwiftContext)
MoveOnlyAttr(/*Implicit=*/true));
} else {
Impl.addImportDiagnostic(
decl,
Diagnostic(
diag::move_only_requires_move_only,
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())),
decl->getLocation());
if (decl->isInStdNamespace() && decl->getName() == "promise") {
// Do not import std::promise.
return nullptr;
}
result->getAttrs().add(new (Impl.SwiftContext)
MoveOnlyAttr(/*Implicit=*/true));
}

// FIXME: Figure out what to do with superclasses in C++. One possible
Expand Down Expand Up @@ -2658,8 +2648,7 @@ namespace {
// SemaLookup.cpp).
if (!decl->isBeingDefined() && !decl->isDependentContext() &&
areRecordFieldsComplete(decl)) {
if (decl->hasInheritedConstructor() &&
Impl.isCxxInteropCompatVersionAtLeast(6)) {
if (decl->hasInheritedConstructor()) {
for (auto member : decl->decls()) {
if (auto usingDecl = dyn_cast<clang::UsingDecl>(member)) {
for (auto usingShadowDecl : usingDecl->shadows()) {
Expand Down Expand Up @@ -2830,14 +2819,12 @@ namespace {
void
addExplicitProtocolConformances(NominalTypeDecl *decl,
const clang::CXXRecordDecl *clangDecl) {
if (Impl.isCxxInteropCompatVersionAtLeast(6)) {
// Propagate conforms_to attribute from public base classes.
for (auto base : clangDecl->bases()) {
if (base.getAccessSpecifier() != clang::AccessSpecifier::AS_public)
continue;
if (auto *baseClangDecl = base.getType()->getAsCXXRecordDecl())
addExplicitProtocolConformances(decl, baseClangDecl);
}
// Propagate conforms_to attribute from public base classes.
for (auto base : clangDecl->bases()) {
if (base.getAccessSpecifier() != clang::AccessSpecifier::AS_public)
continue;
if (auto *baseClangDecl = base.getType()->getAsCXXRecordDecl())
addExplicitProtocolConformances(decl, baseClangDecl);
}

if (!clangDecl->hasAttrs())
Expand Down Expand Up @@ -3755,39 +3742,34 @@ namespace {

if (decl->isVirtual()) {
if (auto funcDecl = dyn_cast_or_null<FuncDecl>(method)) {
if (Impl.isCxxInteropCompatVersionAtLeast(6)) {
if (auto structDecl =
dyn_cast_or_null<StructDecl>(method->getDeclContext())) {
// If this is a method of a Swift struct, any possible override of
// this method would get sliced away, and an invocation would get
// dispatched statically. This is fine because it matches the C++
// behavior.
if (decl->isPure()) {
// If this is a pure virtual method, we won't have any
// implementation of it to invoke.
Impl.markUnavailable(
funcDecl, "virtual function is not available in Swift "
"because it is pure");
}
} else if (auto classDecl = dyn_cast_or_null<ClassDecl>(
funcDecl->getDeclContext())) {
// This is a foreign reference type. Since `class T` on the Swift
// side is mapped from `T*` on the C++ side, an invocation of a
// virtual method `t->method()` should get dispatched dynamically.
// 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);
if (result) {
return result;
} else {
Impl.markUnavailable(
funcDecl, "virtual function is not available in Swift");
}
if (auto structDecl =
dyn_cast_or_null<StructDecl>(method->getDeclContext())) {
// If this is a method of a Swift struct, any possible override of
// this method would get sliced away, and an invocation would get
// dispatched statically. This is fine because it matches the C++
// behavior.
if (decl->isPure()) {
// If this is a pure virtual method, we won't have any
// implementation of it to invoke.
Impl.markUnavailable(funcDecl,
"virtual function is not available in Swift "
"because it is pure");
}
} else if (auto classDecl = dyn_cast_or_null<ClassDecl>(
funcDecl->getDeclContext())) {
// This is a foreign reference type. Since `class T` on the Swift
// side is mapped from `T*` on the C++ side, an invocation of a
// virtual method `t->method()` should get dispatched dynamically.
// 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);
if (result) {
return result;
} else {
Impl.markUnavailable(
funcDecl, "virtual function is not available in Swift");
}
} else {
Impl.markUnavailable(
funcDecl, "virtual functions are not yet available in Swift");
}
}
}
Expand Down Expand Up @@ -4045,8 +4027,7 @@ namespace {
// 1. Types
// 2. C++ methods from privately inherited base classes
if (!isa<clang::TypeDecl>(decl->getTargetDecl()) &&
!(isa<clang::CXXMethodDecl>(decl->getTargetDecl()) &&
Impl.isCxxInteropCompatVersionAtLeast(6)))
!isa<clang::CXXMethodDecl>(decl->getTargetDecl()))
return nullptr;
// Constructors (e.g. `using BaseClass::BaseClass`) are handled in
// VisitCXXRecordDecl, since we need them to determine whether a struct
Expand Down
21 changes: 20 additions & 1 deletion lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,26 @@ namespace {
return importFunctionPointerLikeType(*type, pointeeType);
}

// If non-copyable generics are disabled, we cannot specify
// UnsafePointer<T> with a non-copyable type T.
// We cannot use `ty->isNoncopyable()` here because that would create a
// cyclic dependency between ModuleQualifiedLookupRequest and
// LookupConformanceInModuleRequest, so we check for the presence of
// move-only attribute that is implicitly added to non-copyable C++ types
// by ClangImporter.
if (pointeeType && pointeeType->getAnyNominal() &&
pointeeType->getAnyNominal()
->getAttrs()
.hasAttribute<MoveOnlyAttr>() &&
!Impl.SwiftContext.LangOpts.hasFeature(
Feature::NoncopyableGenerics)) {
auto opaquePointerDecl = Impl.SwiftContext.getOpaquePointerDecl();
if (!opaquePointerDecl)
return Type();
return {opaquePointerDecl->getDeclaredInterfaceType(),
ImportHint::OtherPointer};
}

PointerTypeKind pointerKind;
if (quals.hasConst()) {
pointerKind = PTK_UnsafePointer;
Expand Down Expand Up @@ -2603,7 +2623,6 @@ static ParamDecl *getParameterInfo(ClangImporter::Implementation *impl,
// (https://github.com/apple/swift/issues/70124)
if (param->hasDefaultArg() && !isInOut &&
!isa<clang::CXXConstructorDecl>(param->getDeclContext()) &&
impl->isCxxInteropCompatVersionAtLeast(6) &&
impl->isDefaultArgSafeToImport(param)) {
SwiftDeclSynthesizer synthesizer(*impl);
if (CallExpr *defaultArgExpr = synthesizer.makeDefaultArgument(
Expand Down
11 changes: 3 additions & 8 deletions test/Interop/Cxx/class/conforms-to.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// RUN: %target-swift-frontend %S/Inputs/conforms-to-imported.swift -module-name ImportedModule -emit-module -emit-module-path %t/ImportedModule.swiftmodule

// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %t -I %S/Inputs -module-name SwiftTest -enable-experimental-cxx-interop
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %t -I %S/Inputs -module-name SwiftTest -cxx-interoperability-mode=swift-6 -D UPCOMING_SWIFT
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %t -I %S/Inputs -module-name SwiftTest -cxx-interoperability-mode=upcoming-swift -D UPCOMING_SWIFT
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %t -I %S/Inputs -module-name SwiftTest -cxx-interoperability-mode=swift-5.9
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %t -I %S/Inputs -module-name SwiftTest -cxx-interoperability-mode=swift-6
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %t -I %S/Inputs -module-name SwiftTest -cxx-interoperability-mode=upcoming-swift

import ConformsTo
import ImportedModule
Expand All @@ -23,11 +24,9 @@ func callee(_ _: Testable) {
func caller(_ x: HasTest) {
callee(x)
}
#if UPCOMING_SWIFT
func caller(_ x: DerivedFromHasTest) { callee(x) }
func caller(_ x: DerivedFromDerivedFromHasTest) { callee(x) }
func caller(_ x: DerivedFromDerivedFromHasTestWithDuplicateArg) { callee(x) }
#endif

func callee(_ _: Playable) {

Expand All @@ -36,7 +35,6 @@ func callee(_ _: Playable) {
func caller(_ x: Playable) {
callee(x)
}
#if UPCOMING_SWIFT
func caller(_ x: DerivedFromHasPlay) { callee(x) }
func caller(_ x: DerivedFromDerivedFromHasPlay) { callee(x) }

Expand All @@ -48,15 +46,12 @@ func caller(_ x: DerivedFromHasTestAndPlay) {
callee(x as Testable)
callee(x as Playable)
}
#endif

func callee(_ _: ProtocolFromImportedModule) {
}

func caller(_ x: HasImportedConf) {
callee(x)
}
#if UPCOMING_SWIFT
func caller(_ x: DerivedFromHasImportedConf) { callee(x) }
func caller(_ x: DerivedFromDerivedFromHasImportedConf) { callee(x) }
#endif
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=swift-5.9
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=swift-6
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=upcoming-swift

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=swift-5.9)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=swift-6)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=upcoming-swift)
//
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %target-swift-emit-ir -I %S/Inputs -cxx-interoperability-mode=upcoming-swift %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
// RUN: %target-swift-emit-ir -I %S/Inputs -cxx-interoperability-mode=swift-5.9 %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
// RUN: %target-swift-emit-ir -I %S/Inputs -cxx-interoperability-mode=swift-6 %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s

import VirtualMethods
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// RUN: %target-swift-ide-test -print-module -cxx-interoperability-mode=swift-5.9 -print-implicit-attrs -module-to-print=VirtualMethods -I %S/Inputs -source-filename=x | %FileCheck %s
// RUN: %target-swift-ide-test -print-module -cxx-interoperability-mode=swift-6 -print-implicit-attrs -module-to-print=VirtualMethods -I %S/Inputs -source-filename=x | %FileCheck %s
// RUN: %target-swift-ide-test -print-module -cxx-interoperability-mode=upcoming-swift -print-implicit-attrs -module-to-print=VirtualMethods -I %S/Inputs -source-filename=x | %FileCheck %s

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=upcoming-swift
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=swift-5.9
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=swift-6

import VirtualMethods
Expand Down
1 change: 1 addition & 0 deletions test/Interop/Cxx/class/inheritance/virtual-methods.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=swift-5.9)
// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=swift-6)
// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,7 @@ struct NonCopyableHolderDerivedDerived: NonCopyableHolderDerived {
}
};

inline NonCopyable *getNonCopyablePtr() { return nullptr; }
inline NonCopyableDerived *getNonCopyableDerivedPtr() { return nullptr; }

#endif // TEST_INTEROP_CXX_CLASS_MOVE_ONLY_VT_H
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=swift-6 %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=upcoming-swift %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=swift-6 -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s

import MoveOnlyCxxValueType

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=swift-6 %s -validate-tbd-against-ir=none | %FileCheck %s
// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=upcoming-swift %s -validate-tbd-against-ir=none | %FileCheck %s
// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=swift-6 -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none | %FileCheck %s
// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none | %FileCheck %s

import MoveOnlyCxxValueType

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// RUN: %target-swift-ide-test -print-module -module-to-print=MoveOnlyCxxValueType -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -source-filename=x | %FileCheck %s --check-prefix=CHECK-NO-NCG
// RUN: %target-swift-ide-test -print-module -module-to-print=MoveOnlyCxxValueType -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -source-filename=x -enable-experimental-feature NoncopyableGenerics | %FileCheck %s --check-prefix=CHECK-NCG

// CHECK-NO-NCG: func getNonCopyablePtr() -> OpaquePointer
// CHECK-NO-NCG: func getNonCopyableDerivedPtr() -> OpaquePointer

// CHECK-NCG: func getNonCopyablePtr() -> UnsafeMutablePointer<NonCopyable>
// CHECK-NCG: func getNonCopyableDerivedPtr() -> UnsafeMutablePointer<NonCopyableDerived>
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics -D HAS_NONCOPYABLE_GENERICS)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-5.9 -O)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-6 -O)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-6 -O -enable-experimental-feature NoncopyableGenerics -D HAS_NONCOPYABLE_GENERICS)

//
// REQUIRES: executable_test

import MoveOnlyCxxValueType
Expand All @@ -27,10 +29,12 @@ MoveOnlyCxxValueType.test("Test derived move only type member access") {
var k = c.method(-3)
expectEqual(k, -6)
expectEqual(c.method(1), 2)
#if HAS_NONCOPYABLE_GENERICS
k = c.x
expectEqual(k, 2)
c.x = 11
expectEqual(c.x, 11)
#endif
k = c.mutMethod(-13)
expectEqual(k, -13)
}
Expand All @@ -56,6 +60,7 @@ MoveOnlyCxxValueType.test("Test move only field access in holder") {
expectEqual(c.x.x, 5)
}

#if HAS_NONCOPYABLE_GENERICS
MoveOnlyCxxValueType.test("Test move only field access in derived holder") {
var c = NonCopyableHolderDerivedDerived(-11)
var k = borrowNC(c.x)
Expand All @@ -69,5 +74,6 @@ MoveOnlyCxxValueType.test("Test move only field access in derived holder") {
c.x.mutMethod(5)
expectEqual(c.x.x, 5)
}
#endif

runAllTests()
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Tests that a C++ class can conform to a Swift protocol.

// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-experimental-cxx-interop
// RUN: %target-typecheck-verify-swift -I %S/Inputs -D VIRTUAL_METHODS -cxx-interoperability-mode=swift-5.9
// RUN: %target-typecheck-verify-swift -I %S/Inputs -D VIRTUAL_METHODS -cxx-interoperability-mode=swift-6
// RUN: %target-typecheck-verify-swift -I %S/Inputs -D VIRTUAL_METHODS -cxx-interoperability-mode=upcoming-swift

Expand Down
Loading