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] Warning unannotated C++ APIs returning SWIFT_SHARED_REF… #76798

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
12 changes: 10 additions & 2 deletions include/swift/AST/DiagnosticsClangImporter.def
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,16 @@ ERROR(failed_base_method_call_synthesis,none,
"failed to synthesize call to the base method %0 of type %0",
(ValueDecl *, ValueDecl *))

ERROR(both_returns_retained_returns_unretained,none,
"%0 cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes", (const clang::NamedDecl*))
ERROR(both_returns_retained_returns_unretained, none,
"%0 cannot be annotated with both swift_attr('returns_retained') and "
"swift_attr('returns_unretained') attributes",
Copy link
Contributor

Choose a reason for hiding this comment

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

@fahadnayyar I think we should suggest people to use the macro instead of directly using swift_attr. What are other similar diagnostics doing?

(const clang::NamedDecl *))

WARNING(no_returns_retained_returns_unretained, none,
"%0 is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated "
"with either swift_attr('returns_retained') or "
"swift_attr('returns_unretained') attributes",
(const clang::NamedDecl *))

NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (const clang::NamedDecl*))
Expand Down
44 changes: 37 additions & 7 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2684,7 +2684,7 @@ namespace {
Impl.diagnoseTopLevelValue(
DeclName(Impl.SwiftContext.getIdentifier(releaseOperation.name)));
}
}else if (releaseOperation.kind ==
} else if (releaseOperation.kind ==
CustomRefCountingOperationResult::tooManyFound) {
HeaderLoc loc(decl->getLocation());
Impl.diagnose(loc,
Expand Down Expand Up @@ -3356,9 +3356,9 @@ namespace {
// swift_attr("returns_unretained") then emit an error in the swift
// compiler. Note: this error is not emitted in the clang compiler because
// these attributes are used only for swift interop.
bool returnsRetainedAttrIsPresent = false;
bool returnsUnretainedAttrIsPresent = false;
if (decl->hasAttrs()) {
bool returnsRetainedAttrIsPresent = false;
bool returnsUnretainedAttrIsPresent = false;
for (const auto *attr : decl->getAttrs()) {
if (const auto *swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
if (swiftAttr->getAttribute() == "returns_unretained") {
Expand All @@ -3368,18 +3368,48 @@ namespace {
}
}
}
}

HeaderLoc loc(decl->getLocation());
if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
decl);
}

if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
HeaderLoc loc(decl->getLocation());
Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
decl);
if ((!returnsRetainedAttrIsPresent) && (!returnsUnretainedAttrIsPresent)) {
if (isForeignReferenceType(decl->getReturnType())) {
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
decl);
}
}

return importFunctionDecl(decl, importedName, correctSwiftName,
std::nullopt);
}

static bool isForeignReferenceType(const clang::QualType type) {
if (!type->isPointerType())
return false;

auto pointeeType =
dyn_cast<clang::RecordType>(type->getPointeeType().getCanonicalType());
if (pointeeType == nullptr)
return false;

return hasImportAsRefAttr(pointeeType->getDecl());
}

static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
return swiftAttr->getAttribute() == "import_reference" ||
// TODO: Remove this once libSwift hosttools no longer
// requires it.
swiftAttr->getAttribute() == "import_as_ref";
return false;
});
}
Comment on lines +3390 to +3411
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like these functions are copied-and-pasted from elsewhere in ClangImporter. Let's use the existing API instead.


/// Handles special functions such as subscripts and dereference operators.
bool
processSpecialImportedFunc(FuncDecl *func, ImportedName importedName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@ struct
__attribute__((swift_attr("returns_unretained")));
};

// C++ APIs returning FRTs but not having annotations
struct
StructWithoutReturnsAnnotations {
static FRTStruct *_Nonnull StaticMethodReturningFRT();
};

FRTStruct *_Nonnull global_function_returning_FRT_WithoutReturnsAnnotations();

struct NonFRTStruct {};

// Global/free C++ functions returning non-FRT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,9 @@ let frtLocalVar1 = global_function_returning_FRT_with_both_attrs_returns_retaine

let frtLocalVar2 = StructWithStaticMethodsReturningFRTWithBothAttributesReturnsRetainedAndReturnsUnretained.StaticMethodReturningFRT()
// CHECK: error: 'StaticMethodReturningFRT' cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes

let frtLocalVar3 = global_function_returning_FRT_WithoutReturnsAnnotations()
// CHECK: warning: 'global_function_returning_FRT_WithoutReturnsAnnotations' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes

let frtLocalVar4 = StructWithoutReturnsAnnotations.StaticMethodReturningFRT()
// CHECK: warning: 'StaticMethodReturningFRT' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes