Skip to content

[cxx-interop] Import certain types as @unsafe #76429

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 1 commit into from
Sep 19, 2024
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
3 changes: 3 additions & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,9 @@ SUPPRESSIBLE_EXPERIMENTAL_FEATURE(AllowUnsafeAttribute, true)
/// Warn on use of unsafe constructs.
EXPERIMENTAL_FEATURE(WarnUnsafe, true)

/// Import unsafe C and C++ constructs as @unsafe.
EXPERIMENTAL_FEATURE(SafeInterop, true)

// Enable values in generic signatures, e.g. <let N: Int>
EXPERIMENTAL_FEATURE(ValueGenerics, true)

Expand Down
1 change: 1 addition & 0 deletions lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ static bool usesFeatureAllowUnsafeAttribute(Decl *decl) {
}

UNINTERESTING_FEATURE(WarnUnsafe)
UNINTERESTING_FEATURE(SafeInterop)

static bool usesFeatureValueGenerics(Decl *decl) {
auto genericContext = decl->getAsGenericContext();
Expand Down
4 changes: 4 additions & 0 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7391,6 +7391,10 @@ bool importer::hasNonEscapableAttr(const clang::RecordDecl *decl) {
return hasSwiftAttribute(decl, "~Escapable");
}

bool importer::hasEscapableAttr(const clang::RecordDecl *decl) {
return hasSwiftAttribute(decl, "Escapable");
}

/// Recursively checks that there are no pointers in any fields or base classes.
/// Does not check C++ records with specific API annotations.
static bool hasPointerInSubobjects(const clang::CXXRecordDecl *decl) {
Expand Down
27 changes: 25 additions & 2 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "swift/AST/PrettyStackTrace.h"
#include "swift/AST/ProtocolConformance.h"
#include "swift/AST/Stmt.h"
#include "swift/AST/Type.h"
#include "swift/AST/TypeCheckRequests.h"
#include "swift/AST/Types.h"
#include "swift/Basic/Assertions.h"
Expand Down Expand Up @@ -8154,6 +8155,21 @@ bool swift::importer::isMutabilityAttr(const clang::SwiftAttrAttr *swiftAttr) {
swiftAttr->getAttribute() == "nonmutating";
}

static bool importAsUnsafe(const ASTContext &context,
const clang::RecordDecl *decl,
const Decl *MappedDecl) {
if (!context.LangOpts.hasFeature(Feature::SafeInterop) ||
!context.LangOpts.hasFeature(Feature::AllowUnsafeAttribute) || !decl)
return false;

if (isa<ClassDecl>(MappedDecl))
return false;

// TODO: Add logic to cover structural rules.
return !importer::hasNonEscapableAttr(decl) &&
!importer::hasEscapableAttr(decl);
}

void
ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
auto ClangDecl =
Expand All @@ -8178,6 +8194,7 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
//
// __attribute__((swift_attr("attribute")))
//
bool seenUnsafe = false;
for (auto swiftAttr : ClangDecl->specific_attrs<clang::SwiftAttrAttr>()) {
// FIXME: Hard-code @MainActor and @UIActor, because we don't have a
// point at which to do name lookup for imported entities.
Expand Down Expand Up @@ -8287,8 +8304,7 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
if (swiftAttr->getAttribute() == "unsafe") {
if (!SwiftContext.LangOpts.hasFeature(Feature::AllowUnsafeAttribute))
continue;
auto attr = new (SwiftContext) UnsafeAttr(/*implicit=*/false);
MappedDecl->getAttrs().add(attr);
seenUnsafe = true;
continue;
}

Expand Down Expand Up @@ -8332,6 +8348,13 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
swiftAttr->getAttribute());
}
}

if (seenUnsafe ||
importAsUnsafe(SwiftContext, dyn_cast<clang::RecordDecl>(ClangDecl),
MappedDecl)) {
auto attr = new (SwiftContext) UnsafeAttr(/*implicit=*/!seenUnsafe);
MappedDecl->getAttrs().add(attr);
}
};
importAttrsFromDecl(ClangDecl);

Expand Down
2 changes: 2 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2031,6 +2031,8 @@ bool hasUnsafeAPIAttr(const clang::Decl *decl);

bool hasNonEscapableAttr(const clang::RecordDecl *decl);

bool hasEscapableAttr(const clang::RecordDecl *decl);

bool isViewType(const clang::CXXRecordDecl *decl);

} // end namespace importer
Expand Down
8 changes: 8 additions & 0 deletions lib/ClangImporter/SwiftBridging/swift/bridging
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@
#define SWIFT_NONESCAPABLE \
__attribute__((swift_attr("~Escapable")))

/// Specifies that a specific c++ type such class or struct should be imported
/// as a escapable Swift value. While this matches the default behavior,
/// in safe mode interop mode it ensures that the type is not marked as
/// unsafe.
#define SWIFT_ESCAPABLE \
__attribute__((swift_attr("Escapable")))

/// Specifies that the return value is passed as owned for C++ functions and
/// methods returning types annotated as `SWIFT_SHARED_REFERENCE`
#define SWIFT_RETURNS_RETAINED __attribute__((swift_attr("returns_retained")))
Expand All @@ -187,6 +194,7 @@
#define SWIFT_UNCHECKED_SENDABLE
#define SWIFT_NONCOPYABLE
#define SWIFT_NONESCAPABLE
#define SWIFT_ESCAPABLE
#define SWIFT_RETURNS_RETAINED
#define SWIFT_RETURNS_UNRETAINED

Expand Down
43 changes: 43 additions & 0 deletions test/Interop/Cxx/class/safe-interop-mode.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: %target-swift-frontend -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -enable-experimental-feature NonescapableTypes -enable-experimental-feature SafeInterop -cxx-interoperability-mode=default -diagnostic-style llvm 2>&1

// REQUIRES: objc_interop

//--- Inputs/module.modulemap
module Test {
header "nonescapable.h"
requires cplusplus
}

//--- Inputs/nonescapable.h
#include "swift/bridging"

struct SWIFT_NONESCAPABLE View {
__attribute__((swift_attr("@lifetime(immortal)")))
View() : member(nullptr) {}
__attribute__((swift_attr("@lifetime(p)")))
View(const int *p [[clang::lifetimebound]]) : member(p) {}
View(const View&) = default;
private:
const int *member;
};

struct SWIFT_ESCAPABLE Owner {};

struct Unannotated {};

//--- test.swift

import Test
import CoreFoundation

func useUnsafeParam(x: Unannotated) { // expected-warning{{reference to unsafe struct 'Unannotated'}}
}

func useSafeParams(x: Owner, y: View) {
}

func useCfType(x: CFArray) {
}