Skip to content

Revert "[interop] do not import functions whose return type is not imported" #67097

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

Merged
Merged
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
1 change: 0 additions & 1 deletion include/swift/AST/DiagnosticsClangImporter.def
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (c
NOTE(invoked_func_not_imported, none, "function %0 unavailable (cannot import)", (const clang::NamedDecl*))
NOTE(record_method_not_imported, none, "method %0 unavailable (cannot import)", (const clang::NamedDecl*))
NOTE(objc_property_not_imported, none, "property %0 unavailable (cannot import)", (const clang::NamedDecl*))
NOTE(unsupported_return_type, none, "C++ function %0 is unavailable: return type is unavailable in Swift", (const clang::NamedDecl*))

NOTE(placeholder_for_forward_declared_interface_member_access_failure, none,
"class '%0' will be imported as an opaque placeholder class and may be "
Expand Down
19 changes: 18 additions & 1 deletion lib/ClangImporter/ClangDerivedConformances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,23 @@ void swift::conformToCxxSequenceIfNeeded(
}
}

static bool isStdSetType(const clang::CXXRecordDecl *clangDecl) {
return isStdDecl(clangDecl, {"set", "unordered_set", "multiset"});
}

bool swift::isUnsafeStdMethod(const clang::CXXMethodDecl *methodDecl) {
auto parentDecl =
dyn_cast<clang::CXXRecordDecl>(methodDecl->getDeclContext());
if (!parentDecl)
return false;
if (!isStdSetType(parentDecl))
return false;
if (methodDecl->getDeclName().isIdentifier() &&
methodDecl->getName() == "insert")
return true;
return false;
}

void swift::conformToCxxSetIfNeeded(ClangImporter::Implementation &impl,
NominalTypeDecl *decl,
const clang::CXXRecordDecl *clangDecl) {
Expand All @@ -576,7 +593,7 @@ void swift::conformToCxxSetIfNeeded(ClangImporter::Implementation &impl,

// Only auto-conform types from the C++ standard library. Custom user types
// might have a similar interface but different semantics.
if (!isStdDecl(clangDecl, {"set", "unordered_set", "multiset"}))
if (!isStdSetType(clangDecl))
return;

auto valueType = lookupDirectSingleWithoutExtensions<TypeAliasDecl>(
Expand Down
2 changes: 2 additions & 0 deletions lib/ClangImporter/ClangDerivedConformances.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ namespace swift {

bool isIterator(const clang::CXXRecordDecl *clangDecl);

bool isUnsafeStdMethod(const clang::CXXMethodDecl *methodDecl);

/// If the decl is a C++ input iterator, synthesize a conformance to the
/// UnsafeCxxInputIterator protocol, which is defined in the Cxx module.
void conformToCxxIteratorIfNeeded(ClangImporter::Implementation &impl,
Expand Down
13 changes: 12 additions & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5711,7 +5711,13 @@ importName(const clang::NamedDecl *D,

Type ClangImporter::importFunctionReturnType(
const clang::FunctionDecl *clangDecl, DeclContext *dc) {
if (auto imported = Impl.importFunctionReturnType(clangDecl, dc).getType())
bool isInSystemModule =
cast<ClangModuleUnit>(dc->getModuleScopeContext())->isSystemModule();
bool allowNSUIntegerAsInt =
Impl.shouldAllowNSUIntegerAsInt(isInSystemModule, clangDecl);
if (auto imported =
Impl.importFunctionReturnType(dc, clangDecl, allowNSUIntegerAsInt)
.getType())
return imported;
return dc->getASTContext().getNeverType();
}
Expand Down Expand Up @@ -6794,6 +6800,11 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
method->getReturnType()->isReferenceType())
return false;

// Check if it's one of the known unsafe methods we currently
// mark as safe by default.
if (isUnsafeStdMethod(method))
return false;

// Try to figure out the semantics of the return type. If it's a
// pointer/iterator, it's unsafe.
if (auto returnType = dyn_cast<clang::RecordType>(
Expand Down
25 changes: 7 additions & 18 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3137,16 +3137,13 @@ namespace {

if (auto recordType = dyn_cast<clang::RecordType>(
decl->getReturnType().getCanonicalType())) {
if (recordHasReferenceSemantics(recordType->getDecl())) {
Impl.addImportDiagnostic(
decl,
Diagnostic(diag::reference_passed_by_value,
Impl.SwiftContext.AllocateCopy(
recordType->getDecl()->getNameAsString()),
"the return"),
decl->getLocation());
return true;
}
Impl.addImportDiagnostic(
decl, Diagnostic(diag::reference_passed_by_value,
Impl.SwiftContext.AllocateCopy(
recordType->getDecl()->getNameAsString()),
"the return"),
decl->getLocation());
return recordHasReferenceSemantics(recordType->getDecl());
}

return false;
Expand Down Expand Up @@ -3532,14 +3529,6 @@ namespace {
func->setAccess(AccessLevel::Public);
}

if (!isa<clang::CXXConstructorDecl>(decl) && !importedType) {
if (!Impl.importFunctionReturnType(decl, result->getDeclContext())) {
Impl.addImportDiagnostic(
decl, Diagnostic(diag::unsupported_return_type, decl),
decl->getSourceRange().getBegin());
return nullptr;
}
}
result->setIsObjC(false);
result->setIsDynamic(false);

Expand Down
9 changes: 0 additions & 9 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2075,15 +2075,6 @@ applyImportTypeAttrs(ImportTypeAttrs attrs, Type type,
return type;
}

ImportedType ClangImporter::Implementation::importFunctionReturnType(
const clang::FunctionDecl *clangDecl, DeclContext *dc) {
bool isInSystemModule =
cast<ClangModuleUnit>(dc->getModuleScopeContext())->isSystemModule();
bool allowNSUIntegerAsInt =
shouldAllowNSUIntegerAsInt(isInSystemModule, clangDecl);
return importFunctionReturnType(dc, clangDecl, allowNSUIntegerAsInt);
}

ImportedType ClangImporter::Implementation::importFunctionReturnType(
DeclContext *dc, const clang::FunctionDecl *clangDecl,
bool allowNSUIntegerAsInt) {
Expand Down
3 changes: 0 additions & 3 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1372,9 +1372,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
const clang::FunctionDecl *clangDecl,
bool allowNSUIntegerAsInt);

ImportedType importFunctionReturnType(const clang::FunctionDecl *clangDecl,
DeclContext *dc);

/// Import the parameter list for a function
///
/// \param clangDecl The underlying declaration, if any; should only be
Expand Down
65 changes: 0 additions & 65 deletions test/Interop/Cxx/class/returns-unavailable-class.swift

This file was deleted.