Skip to content

[cxx-interop] Enable foreign reference types in C interop #82680

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[cxx-interop] Enable foreign reference types in C interop #82680

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
2 changes: 0 additions & 2 deletions include/swift/Basic/LangOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,6 @@ namespace swift {
return CXXStdlib == PlatformDefaultCXXStdlib;
}

bool CForeignReferenceTypes = false;

/// Imports getters and setters as computed properties.
bool CxxInteropGettersSettersAsProperties = false;

Expand Down
2 changes: 1 addition & 1 deletion include/swift/Option/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ def cxx_interoperability_mode :
def experimental_c_foreign_reference_types :
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong feeling here, but in case someone used this their build now would fail due to unknown argument. Should we make it deprecated/noop for one release? I am also OK just removing it straight away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's a fair point, I just don't think anyone uses this flag at the moment. But I also don't feel strongly.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather leave the flag and ignore it (or emit a deprecation message). I've mentioned this flag in various places, so I wouldn't be surprised if it was used somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I brought back the flag, with a deprecation note.

Copy link
Contributor

Choose a reason for hiding this comment

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

Other deprecated flags also trigger a diag::warn_flag_deprecated diagnostic. I think we should do the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Flag<["-"], "experimental-c-foreign-reference-types">,
Flags<[FrontendOption, HelpHidden, ModuleInterfaceOption]>,
HelpText<"Enable experimental C foreign references types (with reference counting).">;
HelpText<"Enable experimental C foreign references types. Deprecated, has no effect.">;

def experimental_hermetic_seal_at_link:
Flag<["-"], "experimental-hermetic-seal-at-link">,
Expand Down
4 changes: 0 additions & 4 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,6 @@ void ClangImporter::Implementation::makeComputed(AbstractStorageDecl *storage,
bool importer::recordHasReferenceSemantics(
const clang::RecordDecl *decl,
ClangImporter::Implementation *importerImpl) {
if (!isa<clang::CXXRecordDecl>(decl) &&
!importerImpl->SwiftContext.LangOpts.CForeignReferenceTypes)
return false;

// At this point decl might not be fully imported into Swift yet, which
// means we might not have asked Clang to generate its implicit members, such
// as copy or move constructors. This would cause CxxRecordSemanticsRequest to
Expand Down
5 changes: 3 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1580,8 +1580,9 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
Args.hasFlag(OPT_enable_objc_interop, OPT_disable_objc_interop,
Target.isOSDarwin() && !Opts.hasFeature(Feature::Embedded));

Opts.CForeignReferenceTypes =
Args.hasArg(OPT_experimental_c_foreign_reference_types);
if (Args.hasArg(OPT_experimental_c_foreign_reference_types))
Diags.diagnose(SourceLoc(), diag::warn_flag_deprecated,
"-experimental-c-foreign-reference-types");

Opts.CxxInteropGettersSettersAsProperties = Args.hasArg(OPT_cxx_interop_getters_setters_as_properties);
Opts.RequireCxxInteropToImportCxxInteropModule =
Expand Down
5 changes: 5 additions & 0 deletions test/Interop/C/struct/foreign-reference-flag-deprecated.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// RUN: %target-build-swift %s -I %S/Inputs/ -Xfrontend -experimental-c-foreign-reference-types 2>&1 | %FileCheck %s

// CHECK: <unknown>:0: warning: flag '-experimental-c-foreign-reference-types' is deprecated

import ForeignReference
9 changes: 5 additions & 4 deletions test/Interop/C/struct/foreign-reference.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -validate-tbd-against-ir=none -Xfrontend -experimental-c-foreign-reference-types -Onone -D NO_OPTIMIZATIONS)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -validate-tbd-against-ir=none -Xfrontend -experimental-c-foreign-reference-types -O)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -validate-tbd-against-ir=none -Onone -D NO_OPTIMIZATIONS)
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -validate-tbd-against-ir=none -O)
//
// REQUIRES: executable_test
// TODO: This should work without ObjC interop in the future rdar://97497120
// REQUIRES: objc_interop

// XFAIL: OS=windows-msvc
// FIXME: Runtime support for C++ foreign reference types is missing on Windows (https://github.com/swiftlang/swift/issues/82643)

import StdlibUnittest
import ForeignReference
Expand Down