Skip to content

[cxx-interop] allow shared ref retain function to return self #77837

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
Dec 5, 2024
Merged

[cxx-interop] allow shared ref retain function to return self #77837

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
51fd394
[cxx-interop] fix typo in RetainReleaseOperationKind enum
lhoward Nov 25, 2024
da23bcf
[cxx-interop] allow shared ref retain function to return self
lhoward Nov 26, 2024
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
12 changes: 8 additions & 4 deletions include/swift/AST/DiagnosticsClangImporter.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,14 @@ ERROR(foreign_reference_types_invalid_retain_release, none,
"type '%2'",
(bool, StringRef, StringRef))

ERROR(foreign_reference_types_retain_release_non_void_return_type, none,
"specified %select{retain|release}0 function '%1' is invalid; "
"%select{retain|release}0 function must have 'void' return type",
(bool, StringRef))
ERROR(foreign_reference_types_retain_non_void_or_self_return_type, none,
"specified retain function '%0' is invalid; "
"retain function must have 'void' or parameter return type",
(StringRef))
ERROR(foreign_reference_types_release_non_void_return_type, none,
"specified release function '%0' is invalid; "
"release function must have 'void' return type",
(StringRef))
ERROR(foreign_reference_types_retain_release_not_a_function_decl, none,
"specified %select{retain|release}0 function '%1' is not a function",
(bool, StringRef))
Expand Down
66 changes: 39 additions & 27 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2530,24 +2530,23 @@ namespace {
void validateForeignReferenceType(const clang::CXXRecordDecl *decl,
ClassDecl *classDecl) {

enum class RetainReleaseOperatonKind {
enum class RetainReleaseOperationKind {
Copy link
Member

Choose a reason for hiding this comment

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

Ah, thank you for the typo fix here!

notAfunction,
doesntReturnVoid,
doesntReturnVoidOrSelf,
invalidParameters,
valid
};

auto getOperationValidity =
[&](ValueDecl *operation) -> RetainReleaseOperatonKind {
[&](ValueDecl *operation,
CustomRefCountingOperationKind operationKind)
-> RetainReleaseOperationKind {
auto operationFn = dyn_cast<FuncDecl>(operation);
if (!operationFn)
return RetainReleaseOperatonKind::notAfunction;

if (!operationFn->getResultInterfaceType()->isVoid())
return RetainReleaseOperatonKind::doesntReturnVoid;
return RetainReleaseOperationKind::notAfunction;

if (operationFn->getParameters()->size() != 1)
return RetainReleaseOperatonKind::invalidParameters;
return RetainReleaseOperationKind::invalidParameters;

Type paramType =
operationFn->getParameters()->get(0)->getInterfaceType();
Expand All @@ -2557,20 +2556,31 @@ namespace {
}

swift::NominalTypeDecl *paramDecl = paramType->getAnyNominal();

// The return type should be void (for release functions), or void
// or the parameter type (for retain functions).
auto resultInterfaceType = operationFn->getResultInterfaceType();
if (!resultInterfaceType->isVoid()) {
if (operationKind == CustomRefCountingOperationKind::release ||
!resultInterfaceType->lookThroughSingleOptionalType()->isEqual(paramType))
return RetainReleaseOperationKind::doesntReturnVoidOrSelf;
}

// The parameter of the retain/release function should be pointer to the
// same FRT or a base FRT.
if (paramDecl != classDecl) {
if (const clang::Decl *paramClangDecl = paramDecl->getClangDecl()) {
if (const auto *paramTypeDecl =
dyn_cast<clang::CXXRecordDecl>(paramClangDecl)) {
if (decl->isDerivedFrom(paramTypeDecl)) {
return RetainReleaseOperatonKind::valid;
return RetainReleaseOperationKind::valid;
}
}
}
return RetainReleaseOperatonKind::invalidParameters;
return RetainReleaseOperationKind::invalidParameters;
}
return RetainReleaseOperatonKind::valid;

return RetainReleaseOperationKind::valid;
};

auto retainOperation = evaluateOrDefault(
Expand Down Expand Up @@ -2607,28 +2617,29 @@ namespace {
false, retainOperation.name, decl->getNameAsString());
} else if (retainOperation.kind ==
CustomRefCountingOperationResult::foundOperation) {
RetainReleaseOperatonKind operationKind =
getOperationValidity(retainOperation.operation);
RetainReleaseOperationKind operationKind =
getOperationValidity(retainOperation.operation,
CustomRefCountingOperationKind::retain);
HeaderLoc loc(decl->getLocation());
switch (operationKind) {
case RetainReleaseOperatonKind::notAfunction:
case RetainReleaseOperationKind::notAfunction:
Impl.diagnose(
loc,
diag::foreign_reference_types_retain_release_not_a_function_decl,
false, retainOperation.name);
break;
case RetainReleaseOperatonKind::doesntReturnVoid:
case RetainReleaseOperationKind::doesntReturnVoidOrSelf:
Impl.diagnose(
loc,
diag::foreign_reference_types_retain_release_non_void_return_type,
false, retainOperation.name);
diag::foreign_reference_types_retain_non_void_or_self_return_type,
retainOperation.name);
break;
case RetainReleaseOperatonKind::invalidParameters:
case RetainReleaseOperationKind::invalidParameters:
Impl.diagnose(loc,
diag::foreign_reference_types_invalid_retain_release,
false, retainOperation.name, classDecl->getNameStr());
break;
case RetainReleaseOperatonKind::valid:
case RetainReleaseOperationKind::valid:
break;
}
} else {
Expand Down Expand Up @@ -2671,28 +2682,29 @@ namespace {
true, releaseOperation.name, decl->getNameAsString());
} else if (releaseOperation.kind ==
CustomRefCountingOperationResult::foundOperation) {
RetainReleaseOperatonKind operationKind =
getOperationValidity(releaseOperation.operation);
RetainReleaseOperationKind operationKind =
getOperationValidity(releaseOperation.operation,
CustomRefCountingOperationKind::release);
HeaderLoc loc(decl->getLocation());
switch (operationKind) {
case RetainReleaseOperatonKind::notAfunction:
case RetainReleaseOperationKind::notAfunction:
Impl.diagnose(
loc,
diag::foreign_reference_types_retain_release_not_a_function_decl,
true, releaseOperation.name);
break;
case RetainReleaseOperatonKind::doesntReturnVoid:
case RetainReleaseOperationKind::doesntReturnVoidOrSelf:
Impl.diagnose(
loc,
diag::foreign_reference_types_retain_release_non_void_return_type,
true, releaseOperation.name);
diag::foreign_reference_types_release_non_void_return_type,
releaseOperation.name);
break;
case RetainReleaseOperatonKind::invalidParameters:
case RetainReleaseOperationKind::invalidParameters:
Impl.diagnose(loc,
diag::foreign_reference_types_invalid_retain_release,
true, releaseOperation.name, classDecl->getNameStr());
break;
case RetainReleaseOperatonKind::valid:
case RetainReleaseOperationKind::valid:
break;
}
} else {
Expand Down
14 changes: 13 additions & 1 deletion test/Interop/Cxx/foreign-reference/invalid-retain-operation-errors.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ GoodRetainRelease {};
void goodRetain(GoodRetainRelease *v);
void goodRelease(GoodRetainRelease *v);

struct
__attribute__((swift_attr("import_reference")))
__attribute__((swift_attr("retain:goodRetainWithRetainReturningSelf")))
__attribute__((swift_attr("release:goodReleaseWithRetainReturningSelf")))
GoodRetainReleaseWithRetainReturningSelf {};

GoodRetainReleaseWithRetainReturningSelf *goodRetainWithRetainReturningSelf(GoodRetainReleaseWithRetainReturningSelf *v);
void goodReleaseWithRetainReturningSelf(GoodRetainReleaseWithRetainReturningSelf *v);

struct
__attribute__((swift_attr("import_reference")))
__attribute__((swift_attr("retain:goodRetainWithNullabilityAnnotations")))
Expand Down Expand Up @@ -226,7 +235,7 @@ public func test(x: NonExistent) { }
@available(macOS 13.3, *)
public func test(x: NoRetainRelease) { }

// CHECK: error: specified retain function 'badRetain' is invalid; retain function must have 'void' return type
// CHECK: error: specified retain function 'badRetain' is invalid; retain function must have 'void' or parameter return type
// CHECK: error: specified release function 'badRelease' is invalid; release function must have exactly one argument of type 'BadRetainRelease'
@available(macOS 13.3, *)
public func test(x: BadRetainRelease) { }
Expand All @@ -239,6 +248,9 @@ public func test(x: BadRetainReleaseWithNullabilityAnnotations) { }
@available(macOS 13.3, *)
public func test(x: GoodRetainRelease) { }

@available(macOS 13.3, *)
public func test(x: GoodRetainReleaseRetainReturningSelf) { }

@available(macOS 13.3, *)
public func test(x: GoodRetainReleaseWithNullabilityAnnotations) { }

Expand Down