Skip to content

Commit 1601564

Browse files
author
Gabor Horvath
committed
[cxx-interop] Import rvalue references as consuming parameters
Unfortunately, importing them as is results in ambiguous call sites. E.g., std::vector::push_back has overloads for lvalue reference and rvalue reference and we have no way to distinguish them at the call site in Swift. To overcome this issue, functions with rvalue reference parameters are imported with 'consuming:' argument labels. Note that, in general, move only types and consuming is not properly supported in Swift yet. We do not invoke the dtor for the moved-from objects. This is a preexisting problem that can be observed with move only types before this PR, so the fix will be done in a separate PR. Fortunately, for most types, the moved-from objects do not require additional cleanups. rdar://125816354
1 parent b13544e commit 1601564

File tree

7 files changed

+55
-40
lines changed

7 files changed

+55
-40
lines changed

lib/ClangImporter/ImportName.cpp

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,10 +1478,14 @@ static bool suppressFactoryMethodAsInit(const clang::ObjCMethodDecl *method,
14781478
}
14791479

14801480
static void
1481-
addEmptyArgNamesForClangFunction(const clang::FunctionDecl *funcDecl,
1482-
SmallVectorImpl<StringRef> &argumentNames) {
1483-
for (size_t i = 0; i < funcDecl->param_size(); ++i)
1484-
argumentNames.push_back(StringRef());
1481+
addDefaultArgNamesForClangFunction(const clang::FunctionDecl *funcDecl,
1482+
SmallVectorImpl<StringRef> &argumentNames) {
1483+
for (size_t i = 0; i < funcDecl->param_size(); ++i) {
1484+
if (funcDecl->getParamDecl(i)->getType()->isRValueReferenceType())
1485+
argumentNames.push_back("consuming");
1486+
else
1487+
argumentNames.push_back(StringRef());
1488+
}
14851489
if (funcDecl->isVariadic())
14861490
argumentNames.push_back(StringRef());
14871491
}
@@ -1863,7 +1867,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
18631867
// If we couldn't find a constructor decl, bail.
18641868
if (!ctor)
18651869
return ImportedName();
1866-
addEmptyArgNamesForClangFunction(ctor, argumentNames);
1870+
addDefaultArgNamesForClangFunction(ctor, argumentNames);
18671871
break;
18681872
}
18691873

@@ -1876,7 +1880,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
18761880
if (toType->isBooleanType()) {
18771881
isFunction = true;
18781882
baseName = "__convertToBool";
1879-
addEmptyArgNamesForClangFunction(conversionDecl, argumentNames);
1883+
addDefaultArgNamesForClangFunction(conversionDecl, argumentNames);
18801884
break;
18811885
}
18821886
return ImportedName();
@@ -1928,7 +1932,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19281932
: clang::getOperatorSpelling(op);
19291933
baseName = swiftCtx.getIdentifier(operatorName).str();
19301934
isFunction = true;
1931-
addEmptyArgNamesForClangFunction(functionDecl, argumentNames);
1935+
addDefaultArgNamesForClangFunction(functionDecl, argumentNames);
19321936
if (auto cxxMethod = dyn_cast<clang::CXXMethodDecl>(functionDecl)) {
19331937
if (op == clang::OverloadedOperatorKind::OO_Star &&
19341938
cxxMethod->param_empty()) {
@@ -1947,7 +1951,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19471951
case clang::OverloadedOperatorKind::OO_Call:
19481952
baseName = "callAsFunction";
19491953
isFunction = true;
1950-
addEmptyArgNamesForClangFunction(functionDecl, argumentNames);
1954+
addDefaultArgNamesForClangFunction(functionDecl, argumentNames);
19511955
break;
19521956
case clang::OverloadedOperatorKind::OO_Subscript: {
19531957
auto returnType = functionDecl->getReturnType();
@@ -1965,7 +1969,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19651969
result.info.accessorKind = ImportedAccessorKind::SubscriptSetter;
19661970
}
19671971
isFunction = true;
1968-
addEmptyArgNamesForClangFunction(functionDecl, argumentNames);
1972+
addDefaultArgNamesForClangFunction(functionDecl, argumentNames);
19691973
break;
19701974
}
19711975
default:
@@ -1996,7 +2000,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
19962000

19972001
if (auto function = dyn_cast<clang::FunctionDecl>(D)) {
19982002
isFunction = true;
1999-
addEmptyArgNamesForClangFunction(function, argumentNames);
2003+
addDefaultArgNamesForClangFunction(function, argumentNames);
20002004
}
20012005
break;
20022006

lib/ClangImporter/ImportType.cpp

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,6 +2415,7 @@ ClangImporter::Implementation::importParameterType(
24152415
auto attrs = getImportTypeAttrs(param, /*isParam=*/true);
24162416
Type swiftParamTy;
24172417
bool isInOut = false;
2418+
bool isConsuming = false;
24182419
bool isParamTypeImplicitlyUnwrapped = false;
24192420

24202421
// Sometimes we import unavailable typedefs as enums. If that's the case,
@@ -2448,7 +2449,7 @@ ClangImporter::Implementation::importParameterType(
24482449
return std::nullopt;
24492450
} else if (isa<clang::ReferenceType>(paramTy) &&
24502451
isa<clang::TemplateTypeParmType>(paramTy->getPointeeType())) {
2451-
// We don't support rvalue reference / universal perfect ref, bail.
2452+
// We don't support universal reference, bail.
24522453
if (paramTy->isRValueReferenceType()) {
24532454
addImportDiagnosticFn(Diagnostic(diag::rvalue_ref_params_not_imported));
24542455
return std::nullopt;
@@ -2469,26 +2470,25 @@ ClangImporter::Implementation::importParameterType(
24692470
if (!swiftParamTy) {
24702471
// C++ reference types are brought in as direct
24712472
// types most commonly.
2472-
auto refPointeeType =
2473-
importer::getCxxReferencePointeeTypeOrNone(paramTy.getTypePtr());
2474-
if (refPointeeType) {
2473+
if (auto refPointeeType =
2474+
getCxxReferencePointeeTypeOrNone(paramTy.getTypePtr())) {
24752475
// We don't support reference type to a dependent type, just bail.
24762476
if ((*refPointeeType)->isDependentType()) {
24772477
return std::nullopt;
24782478
}
24792479

2480-
// We don't support rvalue reference types, just bail.
2481-
if (paramTy->isRValueReferenceType()) {
2482-
addImportDiagnosticFn(Diagnostic(diag::rvalue_ref_params_not_imported));
2483-
return std::nullopt;
2484-
}
2485-
2480+
bool isRvalueRef = paramTy->isRValueReferenceType();
24862481
// A C++ parameter of type `const <type> &` or `<type> &` becomes `<type>`
2487-
// or `inout <type>` in Swift. Note that SILGen will use the indirect
2488-
// parameter convention for such a type.
2482+
// or `inout <type>`. Moreover, `const <type> &&` or `<type> &&`
2483+
// becomes `<type>` or `consuming <type>`. Note that SILGen will use the
2484+
// indirect parameter convention for such a type.
24892485
paramTy = *refPointeeType;
2490-
if (!paramTy.isConstQualified())
2491-
isInOut = true;
2486+
if (!paramTy.isConstQualified()) {
2487+
if (isRvalueRef)
2488+
isConsuming = true;
2489+
else
2490+
isInOut = true;
2491+
}
24922492
}
24932493
}
24942494

@@ -2552,7 +2552,7 @@ ClangImporter::Implementation::importParameterType(
25522552
if (isInOut && isDirectUseOfForeignReferenceType(paramTy, swiftParamTy))
25532553
isInOut = false;
25542554

2555-
return ImportParameterTypeResult{swiftParamTy, isInOut,
2555+
return ImportParameterTypeResult{swiftParamTy, isInOut, isConsuming,
25562556
isParamTypeImplicitlyUnwrapped};
25572557
}
25582558

@@ -2606,7 +2606,7 @@ static ParamDecl *getParameterInfo(ClangImporter::Implementation *impl,
26062606
const clang::ParmVarDecl *param,
26072607
const Identifier &name,
26082608
const swift::Type &swiftParamTy,
2609-
const bool isInOut,
2609+
const bool isInOut, const bool isConsuming,
26102610
const bool isParamTypeImplicitlyUnwrapped) {
26112611
// Figure out the name for this parameter.
26122612
Identifier bodyName = impl->importFullName(param, impl->CurrentVersion)
@@ -2632,10 +2632,12 @@ static ParamDecl *getParameterInfo(ClangImporter::Implementation *impl,
26322632

26332633
// Foreign references are already references so they don't need to be passed
26342634
// as inout.
2635-
paramInfo->setSpecifier(isInOut ? ParamSpecifier::InOut
2636-
: (param->getAttr<clang::LifetimeBoundAttr>()
2637-
? ParamSpecifier::Borrowing
2638-
: ParamSpecifier::Default));
2635+
paramInfo->setSpecifier(
2636+
isConsuming ? ParamSpecifier::Consuming
2637+
: (isInOut ? ParamSpecifier::InOut
2638+
: (param->getAttr<clang::LifetimeBoundAttr>()
2639+
? ParamSpecifier::Borrowing
2640+
: ParamSpecifier::Default)));
26392641
paramInfo->setInterfaceType(swiftParamTy);
26402642
impl->recordImplicitUnwrapForDecl(paramInfo, isParamTypeImplicitlyUnwrapped);
26412643

@@ -2702,6 +2704,7 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
27022704
}
27032705
auto swiftParamTy = swiftParamTyOpt->swiftTy;
27042706
bool isInOut = swiftParamTyOpt->isInOut;
2707+
bool isConsuming = swiftParamTyOpt->isConsuming;
27052708
bool isParamTypeImplicitlyUnwrapped =
27062709
swiftParamTyOpt->isParamTypeImplicitlyUnwrapped;
27072710

@@ -2710,8 +2713,9 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
27102713
if (index < argNames.size())
27112714
name = argNames[index];
27122715

2713-
auto paramInfo = getParameterInfo(this, param, name, swiftParamTy, isInOut,
2714-
isParamTypeImplicitlyUnwrapped);
2716+
auto paramInfo =
2717+
getParameterInfo(this, param, name, swiftParamTy, isInOut, isConsuming,
2718+
isParamTypeImplicitlyUnwrapped);
27152719
parameters.push_back(paramInfo);
27162720
++index;
27172721
}
@@ -3296,6 +3300,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
32963300
}
32973301
auto swiftParamTy = swiftParamTyOpt->swiftTy;
32983302
bool isInOut = swiftParamTyOpt->isInOut;
3303+
bool isConsuming = swiftParamTyOpt->isConsuming;
32993304
bool isParamTypeImplicitlyUnwrapped =
33003305
swiftParamTyOpt->isParamTypeImplicitlyUnwrapped;
33013306

@@ -3345,8 +3350,9 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
33453350
++nameIndex;
33463351

33473352
// Set up the parameter info
3348-
auto paramInfo = getParameterInfo(this, param, name, swiftParamTy, isInOut,
3349-
isParamTypeImplicitlyUnwrapped);
3353+
auto paramInfo =
3354+
getParameterInfo(this, param, name, swiftParamTy, isInOut, isConsuming,
3355+
isParamTypeImplicitlyUnwrapped);
33503356

33513357
// Determine whether we have a default argument.
33523358
if (kind == SpecialMethodKind::Regular ||

lib/ClangImporter/ImporterImpl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,6 +1445,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
14451445
swift::Type swiftTy;
14461446
/// If the parameter is or not inout.
14471447
bool isInOut;
1448+
/// If the parameter should be imported as consuming.
1449+
bool isConsuming;
14481450
/// If the parameter is implicitly unwrapped or not.
14491451
bool isParamTypeImplicitlyUnwrapped;
14501452
};

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3347,6 +3347,9 @@ void MoveOnlyAddressCheckerPImpl::rewriteUses(
33473347
bool isFinalConsume = consumes.claimConsume(destroyPair.first, bits);
33483348

33493349
// Remove destroys that are not the final consuming use.
3350+
// TODO: for C++ types we do not want to remove destroys as the caller is
3351+
// still responsible for invoking the dtor for the moved-from object.
3352+
// See GH Issue #77894.
33503353
if (!isFinalConsume) {
33513354
destroyPair.first->eraseFromParent();
33523355
continue;

test/Interop/Cxx/class/inheritance/using-base-members-module-interface.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
// CHECK-NEXT: }
2222

2323
// CHECK: struct UsingBaseConstructorWithParam {
24+
// CHECK-NEXT: init(consuming _: consuming IntBox)
2425
// CHECK-NEXT: init(_: IntBox)
2526
// CHECK-NEXT: init(_: UInt32)
2627
// CHECK-NEXT: init(_: Int32)
@@ -29,6 +30,7 @@
2930

3031
// CHECK: struct UsingBaseConstructorEmpty {
3132
// CHECK-NEXT: init()
33+
// CHECK-NEXT: init(consuming _: consuming Empty)
3234
// CHECK-NEXT: init(_: Empty)
3335
// CHECK-NEXT: var value: Int32
3436
// CHECK-NEXT: }

test/Interop/Cxx/operators/member-inline-typechecker.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ writeOnlyIntArray[2] = 654
3434
let writeOnlyValue = writeOnlyIntArray[2]
3535

3636
var readOnlyRvalueParam = ReadOnlyRvalueParam()
37-
let readOnlyRvalueVal = readOnlyRvalueParam[1] // expected-error {{value of type 'ReadOnlyRvalueParam' has no subscripts}}
37+
let readOnlyRvalueVal = readOnlyRvalueParam[consuming: 1]
3838

3939
var readWriteRvalueParam = ReadWriteRvalueParam()
40-
let readWriteRvalueVal = readWriteRvalueParam[1] // expected-error {{value of type 'ReadWriteRvalueParam' has no subscripts}}
40+
let readWriteRvalueVal = readWriteRvalueParam[consuming: 1]
4141

4242
var readWriteRvalueGetterParam = ReadWriteRvalueGetterParam()
43-
let readWriteRvalueGetterVal = readWriteRvalueGetterParam[1]
43+
let readWriteRvalueGetterVal = readWriteRvalueGetterParam[consuming: 1]
4444

4545
var diffTypesArray = DifferentTypesArray()
4646
let diffTypesResultInt: Int32 = diffTypesArray[0]

test/Interop/Cxx/reference/reference-cannot-import-diagnostic.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ import Test
2222

2323
public func test() {
2424
var x: CInt = 2
25-
acceptRValueRef(x) // expected-error {{cannot find 'acceptRValueRef' in scope}}
26-
// CHECK: note: function 'acceptRValueRef' unavailable (cannot import)
27-
// CHECK: note: C++ functions with rvalue reference parameters are unavailable in Swift
25+
acceptRValueRef(consuming: x)
2826

2927
notStdMove(x) // expected-error {{cannot find 'notStdMove' in scope}}
3028
// CHECK: note: function 'notStdMove' unavailable (cannot import)

0 commit comments

Comments
 (0)