Skip to content

Commit 7753ad0

Browse files
fahadnayyarfahadnayyar
authored
Merge pull request #76131 from fahadnayyar/cxx-interop-frt-retain-release-diagnostics
Fixed diagnostics for incorrect parameters of retain/release function…
2 parents c49aeaf + f3f4e19 commit 7753ad0

File tree

5 files changed

+400
-65
lines changed

5 files changed

+400
-65
lines changed

‎include/swift/AST/DiagnosticsClangImporter.def‎

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ WARNING(libstdcxx_modulemap_not_found, none,
132132
(StringRef))
133133

134134
WARNING(api_pattern_attr_ignored, none,
135-
"'%0' swift attribute ignored on type '%1': type is not copyable or destructible",
135+
"'%0' Swift attribute ignored on type '%1': type is not copyable or destructible",
136136
(StringRef, StringRef))
137137

138138
ERROR(objc_implementation_two_impls, none,
@@ -208,30 +208,40 @@ NOTE(iterator_method_unavailable, none, "C++ method '%0' that returns an "
208208
NOTE(iterator_potentially_unsafe, none, "C++ methods that return iterators "
209209
"are potentially unsafe; try using Swift collection APIs instead", ())
210210

211-
ERROR(reference_type_must_have_retain_attr,none,
212-
"reference type '%0' must have 'retain:' swift attribute", (StringRef))
213-
ERROR(reference_type_must_have_release_attr,none,
214-
"reference type '%0' must have 'release:' swift attribute", (StringRef))
215-
ERROR(foreign_reference_types_cannot_find_retain,none,
216-
"cannot find retain function '%0' for reference type '%1'", (StringRef, StringRef))
217-
ERROR(foreign_reference_types_cannot_find_release,none,
218-
"cannot find release function '%0' for reference type '%1'", (StringRef, StringRef))
219-
ERROR(too_many_reference_type_retain_operations,none,
220-
"multiple functions '%0' found; there must be exactly one retain "
221-
"function for reference type '%1'", (StringRef, StringRef))
222-
ERROR(too_many_reference_type_release_operations,none,
223-
"multiple functions '%0' found; there must be exactly one release "
224-
"function for reference type '%1'", (StringRef, StringRef))
225-
ERROR(foreign_reference_types_invalid_retain,none,
226-
"specified retain function '%0' is invalid; retain function must have exactly one "
227-
"argument of type '%1'", (StringRef, StringRef))
228-
ERROR(foreign_reference_types_invalid_release,none,
229-
"specified release function '%0' is invalid; release function must have exactly "
230-
"one argument of type '%1'", (StringRef, StringRef))
231-
232-
233-
ERROR(conforms_to_missing_dot,none,
234-
"expected module name and protocol name separated by '.' in protocol conformance; '%0' is invalid", (StringRef))
211+
ERROR(reference_type_must_have_retain_release_attr, none,
212+
"reference type '%1' must have %select{'retain:'|'release:'}0 Swift "
213+
"attribute",
214+
(bool, StringRef))
215+
ERROR(too_many_reference_type_retain_release_attr, none,
216+
"reference type '%1' must have only one %select{'retain:'|'release:'}0 "
217+
"Swift "
218+
"attribute",
219+
(bool, StringRef))
220+
ERROR(foreign_reference_types_cannot_find_retain_release, none,
221+
"cannot find %select{retain|release}0 function '%1' for reference type "
222+
"'%2'",
223+
(bool, StringRef, StringRef))
224+
ERROR(too_many_reference_type_retain_release_operations, none,
225+
"multiple functions '%1' found; there must be exactly one "
226+
"%select{retain|release}0 function for reference type '%2'",
227+
(bool, StringRef, StringRef))
228+
ERROR(foreign_reference_types_invalid_retain_release, none,
229+
"specified %select{retain|release}0 function '%1' is invalid; "
230+
"%select{retain|release}0 function must have exactly one argument of "
231+
"type '%2'",
232+
(bool, StringRef, StringRef))
233+
234+
ERROR(foreign_reference_types_retain_release_non_void_return_type, none,
235+
"specified %select{retain|release}0 function '%1' is invalid; "
236+
"%select{retain|release}0 function must have 'void' return type",
237+
(bool, StringRef))
238+
ERROR(foreign_reference_types_retain_release_not_a_function_decl, none,
239+
"specified %select{retain|release}0 function '%1' is not a function",
240+
(bool, StringRef))
241+
ERROR(conforms_to_missing_dot, none,
242+
"expected module name and protocol name separated by '.' in protocol "
243+
"conformance; '%0' is invalid",
244+
(StringRef))
235245
ERROR(cannot_find_conforms_to_module,none,
236246
"module '%1' in specified protocol conformance '%0' is not found; did you mean to import it first?", (StringRef, StringRef))
237247

‎include/swift/ClangImporter/ClangImporterRequests.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ SourceLoc extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc);
465465
struct CustomRefCountingOperationResult {
466466
enum CustomRefCountingOperationResultKind {
467467
noAttribute,
468+
tooManyAttributes,
468469
immortal,
469470
notFound,
470471
tooManyFound,

‎lib/ClangImporter/ClangImporter.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7816,17 +7816,22 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
78167816
if (!decl->hasAttrs())
78177817
return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
78187818

7819-
auto retainFnAttr =
7820-
llvm::find_if(decl->getAttrs(), [&operationStr](auto *attr) {
7821-
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
7822-
return swiftAttr->getAttribute().starts_with(operationStr);
7823-
return false;
7824-
});
7825-
if (retainFnAttr == decl->getAttrs().end()) {
7819+
llvm::SmallVector<const clang::SwiftAttrAttr *, 1> retainReleaseAttrs;
7820+
for (auto *attr : decl->getAttrs()) {
7821+
if (auto swiftAttr = llvm::dyn_cast<clang::SwiftAttrAttr>(attr)) {
7822+
if (swiftAttr->getAttribute().starts_with(operationStr)) {
7823+
retainReleaseAttrs.push_back(swiftAttr);
7824+
}
7825+
}
7826+
}
7827+
7828+
if (retainReleaseAttrs.empty()) {
78267829
return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
7830+
} else if (retainReleaseAttrs.size() > 1) {
7831+
return {CustomRefCountingOperationResult::tooManyAttributes, nullptr, ""};
78277832
}
78287833

7829-
auto name = cast<clang::SwiftAttrAttr>(*retainFnAttr)
7834+
auto name = retainReleaseAttrs.front()
78307835
->getAttribute()
78317836
.drop_front(StringRef(operationStr).size())
78327837
.str();

‎lib/ClangImporter/ImportDecl.cpp‎

Lines changed: 106 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2548,22 +2548,48 @@ namespace {
25482548

25492549
void validateForeignReferenceType(const clang::CXXRecordDecl *decl,
25502550
ClassDecl *classDecl) {
2551-
auto isValidOperation = [&](ValueDecl *operation) -> bool {
2551+
2552+
enum class RetainReleaseOperatonKind {
2553+
notAfunction,
2554+
doesntReturnVoid,
2555+
invalidParameters,
2556+
valid
2557+
};
2558+
2559+
auto getOperationValidity =
2560+
[&](ValueDecl *operation) -> RetainReleaseOperatonKind {
25522561
auto operationFn = dyn_cast<FuncDecl>(operation);
25532562
if (!operationFn)
2554-
return false;
2563+
return RetainReleaseOperatonKind::notAfunction;
25552564

25562565
if (!operationFn->getResultInterfaceType()->isVoid())
2557-
return false;
2566+
return RetainReleaseOperatonKind::doesntReturnVoid;
25582567

25592568
if (operationFn->getParameters()->size() != 1)
2560-
return false;
2569+
return RetainReleaseOperatonKind::invalidParameters;
25612570

2562-
if (operationFn->getParameters()->get(0)->getInterfaceType()->isEqual(
2563-
classDecl->getInterfaceType()))
2564-
return false;
2571+
Type paramType =
2572+
operationFn->getParameters()->get(0)->getInterfaceType();
2573+
// Unwrap if paramType is an OptionalType
2574+
if (Type optionalType = paramType->getOptionalObjectType()) {
2575+
paramType = optionalType;
2576+
}
25652577

2566-
return true;
2578+
swift::NominalTypeDecl *paramDecl = paramType->getAnyNominal();
2579+
// The parameter of the retain/release function should be pointer to the
2580+
// same FRT or a base FRT.
2581+
if (paramDecl != classDecl) {
2582+
if (const clang::Decl *paramClangDecl = paramDecl->getClangDecl()) {
2583+
if (const auto *paramTypeDecl =
2584+
dyn_cast<clang::CXXRecordDecl>(paramClangDecl)) {
2585+
if (decl->isDerivedFrom(paramTypeDecl)) {
2586+
return RetainReleaseOperatonKind::valid;
2587+
}
2588+
}
2589+
}
2590+
return RetainReleaseOperatonKind::invalidParameters;
2591+
}
2592+
return RetainReleaseOperatonKind::valid;
25672593
};
25682594

25692595
auto retainOperation = evaluateOrDefault(
@@ -2574,24 +2600,50 @@ namespace {
25742600
if (retainOperation.kind ==
25752601
CustomRefCountingOperationResult::noAttribute) {
25762602
HeaderLoc loc(decl->getLocation());
2577-
Impl.diagnose(loc, diag::reference_type_must_have_retain_attr,
2578-
decl->getNameAsString());
2603+
Impl.diagnose(loc, diag::reference_type_must_have_retain_release_attr,
2604+
false, decl->getNameAsString());
2605+
} else if (retainOperation.kind ==
2606+
CustomRefCountingOperationResult::tooManyAttributes) {
2607+
HeaderLoc loc(decl->getLocation());
2608+
Impl.diagnose(loc, diag::too_many_reference_type_retain_release_attr,
2609+
false, decl->getNameAsString());
25792610
} else if (retainOperation.kind ==
25802611
CustomRefCountingOperationResult::notFound) {
25812612
HeaderLoc loc(decl->getLocation());
2582-
Impl.diagnose(loc, diag::foreign_reference_types_cannot_find_retain,
2583-
retainOperation.name, decl->getNameAsString());
2613+
Impl.diagnose(loc,
2614+
diag::foreign_reference_types_cannot_find_retain_release,
2615+
false, retainOperation.name, decl->getNameAsString());
25842616
} else if (retainOperation.kind ==
25852617
CustomRefCountingOperationResult::tooManyFound) {
25862618
HeaderLoc loc(decl->getLocation());
2587-
Impl.diagnose(loc, diag::too_many_reference_type_retain_operations,
2588-
retainOperation.name, decl->getNameAsString());
2619+
Impl.diagnose(loc,
2620+
diag::too_many_reference_type_retain_release_operations,
2621+
false, retainOperation.name, decl->getNameAsString());
25892622
} else if (retainOperation.kind ==
25902623
CustomRefCountingOperationResult::foundOperation) {
2591-
if (!isValidOperation(retainOperation.operation)) {
2592-
HeaderLoc loc(decl->getLocation());
2593-
Impl.diagnose(loc, diag::foreign_reference_types_invalid_retain,
2594-
retainOperation.name, decl->getNameAsString());
2624+
RetainReleaseOperatonKind operationKind =
2625+
getOperationValidity(retainOperation.operation);
2626+
HeaderLoc loc(decl->getLocation());
2627+
switch (operationKind) {
2628+
case RetainReleaseOperatonKind::notAfunction:
2629+
Impl.diagnose(
2630+
loc,
2631+
diag::foreign_reference_types_retain_release_not_a_function_decl,
2632+
false, retainOperation.name);
2633+
break;
2634+
case RetainReleaseOperatonKind::doesntReturnVoid:
2635+
Impl.diagnose(
2636+
loc,
2637+
diag::foreign_reference_types_retain_release_non_void_return_type,
2638+
false, retainOperation.name);
2639+
break;
2640+
case RetainReleaseOperatonKind::invalidParameters:
2641+
Impl.diagnose(loc,
2642+
diag::foreign_reference_types_invalid_retain_release,
2643+
false, retainOperation.name, classDecl->getNameStr());
2644+
break;
2645+
case RetainReleaseOperatonKind::valid:
2646+
break;
25952647
}
25962648
} else {
25972649
// Nothing to do.
@@ -2607,24 +2659,50 @@ namespace {
26072659
if (releaseOperation.kind ==
26082660
CustomRefCountingOperationResult::noAttribute) {
26092661
HeaderLoc loc(decl->getLocation());
2610-
Impl.diagnose(loc, diag::reference_type_must_have_release_attr,
2611-
decl->getNameAsString());
2662+
Impl.diagnose(loc, diag::reference_type_must_have_retain_release_attr,
2663+
true, decl->getNameAsString());
2664+
} else if (releaseOperation.kind ==
2665+
CustomRefCountingOperationResult::tooManyAttributes) {
2666+
HeaderLoc loc(decl->getLocation());
2667+
Impl.diagnose(loc, diag::too_many_reference_type_retain_release_attr,
2668+
true, decl->getNameAsString());
26122669
} else if (releaseOperation.kind ==
26132670
CustomRefCountingOperationResult::notFound) {
26142671
HeaderLoc loc(decl->getLocation());
2615-
Impl.diagnose(loc, diag::foreign_reference_types_cannot_find_release,
2616-
releaseOperation.name, decl->getNameAsString());
2672+
Impl.diagnose(loc,
2673+
diag::foreign_reference_types_cannot_find_retain_release,
2674+
true, releaseOperation.name, decl->getNameAsString());
26172675
} else if (releaseOperation.kind ==
26182676
CustomRefCountingOperationResult::tooManyFound) {
26192677
HeaderLoc loc(decl->getLocation());
2620-
Impl.diagnose(loc, diag::too_many_reference_type_release_operations,
2621-
releaseOperation.name, decl->getNameAsString());
2678+
Impl.diagnose(loc,
2679+
diag::too_many_reference_type_retain_release_operations,
2680+
true, releaseOperation.name, decl->getNameAsString());
26222681
} else if (releaseOperation.kind ==
26232682
CustomRefCountingOperationResult::foundOperation) {
2624-
if (!isValidOperation(releaseOperation.operation)) {
2625-
HeaderLoc loc(decl->getLocation());
2626-
Impl.diagnose(loc, diag::foreign_reference_types_invalid_release,
2627-
releaseOperation.name, decl->getNameAsString());
2683+
RetainReleaseOperatonKind operationKind =
2684+
getOperationValidity(releaseOperation.operation);
2685+
HeaderLoc loc(decl->getLocation());
2686+
switch (operationKind) {
2687+
case RetainReleaseOperatonKind::notAfunction:
2688+
Impl.diagnose(
2689+
loc,
2690+
diag::foreign_reference_types_retain_release_not_a_function_decl,
2691+
true, releaseOperation.name);
2692+
break;
2693+
case RetainReleaseOperatonKind::doesntReturnVoid:
2694+
Impl.diagnose(
2695+
loc,
2696+
diag::foreign_reference_types_retain_release_non_void_return_type,
2697+
true, releaseOperation.name);
2698+
break;
2699+
case RetainReleaseOperatonKind::invalidParameters:
2700+
Impl.diagnose(loc,
2701+
diag::foreign_reference_types_invalid_retain_release,
2702+
true, releaseOperation.name, classDecl->getNameStr());
2703+
break;
2704+
case RetainReleaseOperatonKind::valid:
2705+
break;
26282706
}
26292707
} else {
26302708
// Nothing to do.

0 commit comments

Comments
 (0)