Skip to content

[cxx-interop] convert CXXForeignReferenceTypeInitializers into SuppressCXXForeignReferenceTypeInitializers to be an opt-out flag #80659

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 2 commits into from
Apr 10, 2025
Merged

[cxx-interop] convert CXXForeignReferenceTypeInitializers into SuppressCXXForeignReferenceTypeInitializers to be an opt-out flag #80659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29eb408
[cxx-interop] convert CXXForeignReferenceTypeInitializers into Suppre…
fahadnayyar Apr 8, 2025
bb812ff
Fixed CI issues (abstract C++ class and SWIFT_RETURNS(UN)RETAINED dia…
fahadnayyar Apr 10, 2025
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
2 changes: 1 addition & 1 deletion include/swift/Basic/Features.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ EXPERIMENTAL_FEATURE(ImportNonPublicCxxMembers, true)

/// Synthesize static factory methods for C++ foreign reference types and import
/// them as Swift initializers.
EXPERIMENTAL_FEATURE(CXXForeignReferenceTypeInitializers, true)
EXPERIMENTAL_FEATURE(SuppressCXXForeignReferenceTypeInitializers, true)

// Isolated deinit
SUPPRESSIBLE_LANGUAGE_FEATURE(IsolatedDeinit, 371, "isolated deinit")
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/FeatureSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ UNINTERESTING_FEATURE(StrictMemorySafety)
UNINTERESTING_FEATURE(SafeInteropWrappers)
UNINTERESTING_FEATURE(AssumeResilientCxxTypes)
UNINTERESTING_FEATURE(ImportNonPublicCxxMembers)
UNINTERESTING_FEATURE(CXXForeignReferenceTypeInitializers)
UNINTERESTING_FEATURE(SuppressCXXForeignReferenceTypeInitializers)
UNINTERESTING_FEATURE(CoroutineAccessorsUnwindOnCallerError)
UNINTERESTING_FEATURE(AllowRuntimeSymbolDeclarations)

Expand Down
17 changes: 14 additions & 3 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2539,8 +2539,8 @@ namespace {
result->addMember(ctor);
}
} else {
if (Impl.SwiftContext.LangOpts.hasFeature(
Feature::CXXForeignReferenceTypeInitializers)) {
if (!Impl.SwiftContext.LangOpts.hasFeature(
Feature::SuppressCXXForeignReferenceTypeInitializers)) {
assert(
isa<ClassDecl>(result) &&
"Expected result to be a ClassDecl as it cannot be a StructDecl");
Expand Down Expand Up @@ -3609,7 +3609,18 @@ namespace {
isa<clang::FunctionDecl>(decl)
? cast<clang::FunctionDecl>(decl)->getReturnType()
: cast<clang::ObjCMethodDecl>(decl)->getReturnType();
if (isForeignReferenceTypeWithoutImmortalAttrs(retType)) {
clang::QualType pointeeType = retType;
Copy link
Contributor

Choose a reason for hiding this comment

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

This was the only use of isForeignReferenceTypeWithoutImmortalAttrs. I think we could have fixed that function. Now I think it is unused so let's remove it.

if (retType->isPointerType() || retType->isReferenceType()) {
pointeeType = retType->getPointeeType();
}

clang::RecordDecl *recordDecl = nullptr;
if (const auto *recordType = pointeeType->getAs<clang::RecordType>()) {
recordDecl = recordType->getDecl();
}

if (recordDecl && recordHasReferenceSemantics(recordDecl) &&
!hasImmortalAttrs(recordDecl)) {
if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
decl);
Expand Down
3 changes: 3 additions & 0 deletions lib/ClangImporter/SwiftDeclSynthesizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2538,6 +2538,9 @@ llvm::SmallVector<clang::CXXMethodDecl *, 4>
SwiftDeclSynthesizer::synthesizeStaticFactoryForCXXForeignRef(
const clang::CXXRecordDecl *cxxRecordDecl) {

if (cxxRecordDecl->isAbstract())
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a test covering this branch? I am ok with the change but would be nice to have it actually tested.

return {};

clang::ASTContext &clangCtx = cxxRecordDecl->getASTContext();
clang::Sema &clangSema = ImporterImpl.getClangSema();

Expand Down
5 changes: 2 additions & 3 deletions test/Interop/Cxx/class/constructors-diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs %s -cxx-interoperability-mode=default -enable-experimental-feature CXXForeignReferenceTypeInitializers -disable-availability-checking -verify-additional-file %S/Inputs/constructors.h -Xcc -Wno-nullability-completeness
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs %s -cxx-interoperability-mode=default -disable-availability-checking -verify-additional-file %S/Inputs/constructors.h -Xcc -Wno-nullability-completeness

// This test uses -verify-additional-file, which do not work well on Windows:
// This test uses -verify-additional-file, which do not work well on Windows.
// UNSUPPORTED: OS=windows-msvc
// REQUIRES: swift_feature_CXXForeignReferenceTypeInitializers

import Constructors

Expand Down
3 changes: 1 addition & 2 deletions test/Interop/Cxx/class/constructors-executable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature CXXForeignReferenceTypeInitializers -Xfrontend -disable-availability-checking)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default -Xfrontend -disable-availability-checking)
//
// REQUIRES: executable_test
// REQUIRES: swift_feature_CXXForeignReferenceTypeInitializers

import Constructors
import CxxStdlib
Expand Down
16 changes: 8 additions & 8 deletions test/Interop/Cxx/foreign-reference/Inputs/inheritance.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ __attribute__((swift_attr("release:immortal"))) ImmortalRefType {};
ImmortalRefType *returnImmortalRefType() { return new ImmortalRefType(); };

struct DerivedFromImmortalRefType : ImmortalRefType {};
DerivedFromImmortalRefType *returnDerivedFromImmortalRefType() {
DerivedFromImmortalRefType *returnDerivedFromImmortalRefType() { // expected-warning {{'returnDerivedFromImmortalRefType' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
return new DerivedFromImmortalRefType();
};

Expand Down Expand Up @@ -138,7 +138,7 @@ DerivedFromValueTypeAndAnnotated *returnDerivedFromValueTypeAndAnnotated() { //
}

struct DerivedFromRefType final : RefType {};
DerivedFromRefType *returnDerivedFromRefType() {
DerivedFromRefType *returnDerivedFromRefType() { // expected-warning {{'returnDerivedFromRefType' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
return new DerivedFromRefType();
}

Expand Down Expand Up @@ -209,7 +209,7 @@ __attribute__((swift_attr("release:RCRelease"))) RefType {};
RefType *returnRefType() { return new RefType(); }; // expected-warning {{'returnRefType' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENC}}

struct DerivedFromRefType final : RefType {};
DerivedFromRefType *returnDerivedFromRefType() { // TODO: rdar://145098078 Missing SWIFT_RETURNS_(UN)RETAINED annotation warning should trigger for inferred foreeign reference types as well
DerivedFromRefType *returnDerivedFromRefType() { // expected-warning {{'returnDerivedFromRefType' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
return new DerivedFromRefType();
};
} // namespace BasicInheritanceExample
Expand All @@ -236,7 +236,7 @@ DerivedFromBaseRef1AndBaseRef2 *returnDerivedFromBaseRef1AndBaseRef2() {
};

struct DerivedFromBaseRef3 : BaseRef3 {};
DerivedFromBaseRef3 *returnDerivedFromBaseRef3() {
DerivedFromBaseRef3 *returnDerivedFromBaseRef3() { // expected-warning {{'returnDerivedFromBaseRef3' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
return new DerivedFromBaseRef3();
};
} // namespace MultipleInheritanceExample1
Expand Down Expand Up @@ -312,7 +312,7 @@ __attribute__((swift_attr("release:samerelease"))) B2 {}; // expected-error {{m

struct D : B1, B2 {}; // expected-error {{multiple functions 'sameretain' found; there must be exactly one retain function for reference type 'D'}}
// expected-error@-1 {{multiple functions 'samerelease' found; there must be exactly one release function for reference type 'D'}}
D *returnD() { return new D(); };
D *returnD() { return new D(); }; // expected-warning {{'returnD' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
} // namespace OverloadedRetainRelease

void sameretain(OverloadedRetainRelease::B1 *v) {}
Expand All @@ -339,7 +339,7 @@ struct BVirtual : virtual A {};
struct CVirtual : virtual A {};

struct VirtualDiamond : BVirtual, CVirtual {};
VirtualDiamond *returnVirtualDiamond() { return new VirtualDiamond(); };
VirtualDiamond *returnVirtualDiamond() { return new VirtualDiamond(); }; // expected-warning {{'returnVirtualDiamond' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
} // namespace RefTypeDiamondInheritance

void retainA(RefTypeDiamondInheritance::A *a) {};
Expand All @@ -355,7 +355,7 @@ __attribute__((swift_attr("release:releaseB"))) B : A {};
struct C : A {};

struct Diamond : B, C {};
Diamond *returnDiamond() { return new Diamond(); };
Diamond *returnDiamond() { return new Diamond(); }; // expected-warning {{'returnDiamond' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}

} // namespace NonRefTypeDiamondInheritance

Expand Down Expand Up @@ -384,7 +384,7 @@ __attribute__((swift_attr("retain:forestRetain"))) __attribute__((
};

class Forest : public IntrusiveRefCountedTemplate<Forest> {};
Forest *returnForest() { return new Forest(); };
Forest *returnForest() { return new Forest(); }; // expected-warning {{'returnForest' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
} // namespace InheritingTemplatedRefType

void forestRetain(InheritingTemplatedRefType::IntrusiveRefCountedTemplate<
Expand Down
5 changes: 2 additions & 3 deletions ...xx/foreign-reference/no-ctor-errors.swift → ...ign-reference/cxx-frt-ctor-callable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: not %target-swift-frontend -typecheck -I %t/Inputs %t/test.swift -enable-experimental-cxx-interop 2>&1 | %FileCheck %s
// RUN: %target-swift-frontend -typecheck -verify -I %t/Inputs %t/test.swift -enable-experimental-cxx-interop -disable-availability-checking

//--- Inputs/module.modulemap
module Test {
Expand All @@ -21,5 +21,4 @@ HasCtor {

import Test

// CHECK: error: 'HasCtor' cannot be constructed because it has no accessible initializers
let x = HasCtor(42)
let x = HasCtor(42)
8 changes: 4 additions & 4 deletions test/Interop/Cxx/foreign-reference/move-only-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
// RUN: %target-swift-ide-test -print-module -module-to-print=MoveOnly -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s

// CHECK: class MoveOnly {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> MoveOnly
// CHECK: }
// CHECK-NOT: func moveIntoResult(_ x: MoveOnly) -> MoveOnly

// CHECK: class HasMoveOnlyChild {
// CHECK-NOT: init
// CHECK: init
// CHECK-NOT: var child: MoveOnly
// CHECK: class func create() -> HasMoveOnlyChild
// CHECK: }
// CHECK-NOT: func moveIntoResult(_ x: HasMoveOnlyChild) -> HasMoveOnlyChild

// CHECK: class PrivateCopyCtor {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> PrivateCopyCtor
// CHECK: }
// CHECK-NOT: func moveIntoResult(_ x: PrivateCopyCtor) -> PrivateCopyCtor

// CHECK: class BadCopyCtor {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> BadCopyCtor
Expand Down
15 changes: 7 additions & 8 deletions test/Interop/Cxx/foreign-reference/pod-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-ide-test -print-module -module-to-print=POD -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s

// CHECK: class Empty {
// CHECK-NOT: init
// CHECK: init
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think it is not the right indentation.

// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> Empty
Expand All @@ -12,14 +12,14 @@
// CHECK-NOT: func passThroughByValue(_ x: Empty) -> Empty

// CHECK: class MultipleAttrs {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> MultipleAttrs
// CHECK: }

// CHECK: class IntPair {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: func instancePassThroughByRef(_ ref: IntPair) -> IntPair
Expand All @@ -33,7 +33,7 @@
// CHECK: func passThroughByRef(_ x: IntPair) -> IntPair

// CHECK: class RefHoldingPair {
// CHECK-NOT: init
// CHECK: init
// CHECK-NOT: pair
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
Expand All @@ -42,7 +42,7 @@
// CHECK: }

// CHECK: class RefHoldingPairRef {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> RefHoldingPairRef
Expand All @@ -51,7 +51,7 @@
// CHECK: }

// CHECK: class RefHoldingPairPtr {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> RefHoldingPairPtr
Expand All @@ -60,7 +60,6 @@
// CHECK: }

// CHECK: struct ValueHoldingPair {
// CHECK-NOT: init
// CHECK-NOT: pair
// CHECK: init()
// CHECK: func test() -> Int32
Expand All @@ -77,7 +76,7 @@
// CHECK: }

// CHECK: class BigType {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> BigType
Expand Down
8 changes: 4 additions & 4 deletions test/Interop/Cxx/foreign-reference/singleton-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-ide-test -print-module -module-to-print=Singleton -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s

// CHECK: class DeletedDtor {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> DeletedDtor
Expand All @@ -10,7 +10,7 @@
// CHECK: func mutateIt(_ x: DeletedDtor)

// CHECK: class PrivateDtor {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> PrivateDtor
Expand All @@ -19,7 +19,7 @@
// CHECK: func mutateIt(_ x: PrivateDtor)

// CHECK: class DeletedSpecialMembers {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> DeletedSpecialMembers
Expand All @@ -28,7 +28,7 @@
// CHECK: func mutateIt(_ x: DeletedSpecialMembers)

// CHECK: class PrivateSpecialMembers {
// CHECK-NOT: init
// CHECK: init
// CHECK: func test() -> Int32
// CHECK: func testMutable() -> Int32
// CHECK: class func create() -> PrivateSpecialMembers
Expand Down
1 change: 1 addition & 0 deletions test/Interop/Cxx/symbolic-imports/print-symbolic-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ struct NonCopyable {
// CHECK-NEXT: }
// CHECK-NEXT: }
// CHECK: class MyImmortal {
// CHECK-NEXT: init
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: indent the same as the function below.

// CHECK-NEXT: func foo()
// CHECK-NEXT: }
// CHECK-NEXT: struct NonCopyable {
Expand Down