Skip to content
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

[cxx-interop] Mark C++ reference parameters @addressable #78759

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3891,6 +3891,9 @@ namespace {
}
if (selfIdx) {
func->setSelfIndex(selfIdx.value());
if (func->getASTContext().LangOpts.hasFeature(
Feature::AddressableParameters))
func->getImplicitSelfDecl()->setAddressable();
} else {
func->setStatic();
func->setImportAsStaticMember();
Expand Down
12 changes: 10 additions & 2 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2676,17 +2676,25 @@ static ParamDecl *getParameterInfo(ClangImporter::Implementation *impl,
impl->importSourceLoc(param->getLocation()), bodyName,
impl->ImportedHeaderUnit);

auto &ASTContext = paramInfo->getASTContext();
// If SendingArgsAndResults are enabled and we have a sending argument,
// set that the param was sending.
if (paramInfo->getASTContext().LangOpts.hasFeature(
Feature::SendingArgsAndResults)) {
if (ASTContext.LangOpts.hasFeature(Feature::SendingArgsAndResults)) {
if (auto *attr = param->getAttr<clang::SwiftAttrAttr>()) {
if (attr->getAttribute() == "sending") {
paramInfo->setSending();
}
}
}

// C++ types taking a reference might return a reference/pointer to a
// subobject of the referenced storage. In those cases we need to prevent the
// Swift compiler to pass in a temporary copy to prevent dangling.
if (ASTContext.LangOpts.hasFeature(Feature::AddressableParameters) &&
!param->getType().isNull() && param->getType()->isReferenceType()) {
paramInfo->setAddressable();
}

// Foreign references are already references so they don't need to be passed
// as inout.
paramInfo->setSpecifier(
Expand Down
18 changes: 18 additions & 0 deletions test/Interop/Cxx/class/nonescapable-lifetimebound.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: %target-swift-frontend -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs -emit-sil %t/test.swift -enable-experimental-feature LifetimeDependence -cxx-interoperability-mode=default -diagnostic-style llvm 2>&1 | %FileCheck %s
// RUN: %target-swift-frontend -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs -emit-sil %t/test.swift -enable-experimental-feature AddressableParameters -enable-experimental-feature LifetimeDependence -cxx-interoperability-mode=default -diagnostic-style llvm 2>&1 | %FileCheck %s -check-prefix=CHECK-ADDRESSABLE
// RUN: not %target-swift-frontend -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs -emit-sil %t/test.swift -cxx-interoperability-mode=default -diagnostic-style llvm 2>&1 | %FileCheck %s -check-prefix=CHECK-NO-LIFETIMES

// REQUIRES: swift_feature_AddressableParameters
// REQUIRES: swift_feature_LifetimeDependence

//--- Inputs/module.modulemap
module Test {
Expand Down Expand Up @@ -120,6 +123,21 @@ CaptureView getCaptureView(const Owner& owner [[clang::lifetimebound]]) {
// CHECK: sil [clang CaptureView.captureView] {{.*}} : $@convention(cxx_method) (View, @lifetime(copy 0) @inout CaptureView) -> ()
// CHECK: sil [clang CaptureView.handOut] {{.*}} : $@convention(cxx_method) (@lifetime(copy 1) @inout View, @in_guaranteed CaptureView) -> ()

// CHECK-ADDRESSABLE: sil [clang makeOwner] {{.*}}: $@convention(c) () -> Owner
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jckarter do you know what would be the best way to test if @addressable is properly applied? I do not see this reflected in the emitted SIL. Is this an oversight or intentional?

// CHECK-ADDRESSABLE: sil [clang getView] {{.*}} : $@convention(c) (@in_guaranteed Owner) -> @lifetime(borrow 0) @owned View
// CHECK-ADDRESSABLE: sil [clang getViewFromFirst] {{.*}} : $@convention(c) (@in_guaranteed Owner, @in_guaranteed Owner) -> @lifetime(borrow 0) @owned View
// CHECK-ADDRESSABLE: sil [clang getViewFromEither] {{.*}} : $@convention(c) (@in_guaranteed Owner, @in_guaranteed Owner) -> @lifetime(borrow 0, borrow 1) @owned View
// CHECK-ADDRESSABLE: sil [clang Owner.handOutView] {{.*}} : $@convention(cxx_method) (@in_guaranteed Owner) -> @lifetime(borrow 0) @owned View
// CHECK-ADDRESSABLE: sil [clang Owner.handOutView2] {{.*}} : $@convention(cxx_method) (View, @in_guaranteed Owner) -> @lifetime(borrow 1) @owned View
// CHECK-ADDRESSABLE: sil [clang getViewFromEither] {{.*}} : $@convention(c) (@guaranteed View, @guaranteed View) -> @lifetime(copy 0, copy 1) @owned View
// CHECK-ADDRESSABLE: sil [clang View.init] {{.*}} : $@convention(c) () -> @lifetime(immortal) @out View
// CHECK-ADDRESSABLE: sil [clang OtherView.init] {{.*}} : $@convention(c) (@guaranteed View) -> @lifetime(copy 0) @out OtherView
// CHECK-ADDRESSABLE: sil [clang returnsImmortal] {{.*}} : $@convention(c) () -> @lifetime(immortal) @owned View
// CHECK-ADDRESSABLE: sil [clang copyView] {{.*}} : $@convention(c) (View, @lifetime(copy 0) @inout View) -> ()
// CHECK-ADDRESSABLE: sil [clang getCaptureView] {{.*}} : $@convention(c) (@in_guaranteed Owner) -> @lifetime(borrow 0) @owned CaptureView
// CHECK-ADDRESSABLE: sil [clang CaptureView.captureView] {{.*}} : $@convention(cxx_method) (View, @lifetime(copy 0) @inout CaptureView) -> ()
// CHECK-ADDRESSABLE: sil [clang CaptureView.handOut] {{.*}} : $@convention(cxx_method) (@lifetime(copy 1) @inout View, @in_guaranteed CaptureView) -> ()

// CHECK-NO-LIFETIMES: nonescapable.h:36:6: error: returning ~Escapable type requires '-enable-experimental-feature LifetimeDependence'
// CHECK-NO-LIFETIMES: nonescapable.h:40:6: error: returning ~Escapable type requires '-enable-experimental-feature LifetimeDependence'
// CHECK-NO-LIFETIMES: nonescapable.h:46:6: error: returning ~Escapable type requires '-enable-experimental-feature LifetimeDependence'
Expand Down