Skip to content

[cxx-interop] Shared references are considered safe #82203

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
Jun 17, 2025
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
32 changes: 30 additions & 2 deletions include/swift/ClangImporter/ClangImporterRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,37 @@ class ClangTypeEscapability
void simple_display(llvm::raw_ostream &out, EscapabilityLookupDescriptor desc);
SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc);

struct CxxDeclExplicitSafetyDescriptor final {
const clang::Decl *decl;
bool isClass;

CxxDeclExplicitSafetyDescriptor(const clang::Decl *decl, bool isClass)
: decl(decl), isClass(isClass) {}

friend llvm::hash_code
hash_value(const CxxDeclExplicitSafetyDescriptor &desc) {
return llvm::hash_combine(desc.decl, desc.isClass);
}

friend bool operator==(const CxxDeclExplicitSafetyDescriptor &lhs,
const CxxDeclExplicitSafetyDescriptor &rhs) {
return lhs.decl == rhs.decl && lhs.isClass == rhs.isClass;
}

friend bool operator!=(const CxxDeclExplicitSafetyDescriptor &lhs,
const CxxDeclExplicitSafetyDescriptor &rhs) {
return !(lhs == rhs);
}
};

void simple_display(llvm::raw_ostream &out,
CxxDeclExplicitSafetyDescriptor desc);
SourceLoc extractNearestSourceLoc(CxxDeclExplicitSafetyDescriptor desc);

/// Determine the safety of the given Clang declaration.
class ClangDeclExplicitSafety
: public SimpleRequest<ClangDeclExplicitSafety,
ExplicitSafety(SafeUseOfCxxDeclDescriptor),
ExplicitSafety(CxxDeclExplicitSafetyDescriptor),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;
Expand All @@ -592,7 +619,8 @@ class ClangDeclExplicitSafety
friend SimpleRequest;

// Evaluation.
ExplicitSafety evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const;
ExplicitSafety evaluate(Evaluator &evaluator,
CxxDeclExplicitSafetyDescriptor desc) const;
};

#define SWIFT_TYPEID_ZONE ClangImporter
Expand Down
2 changes: 1 addition & 1 deletion include/swift/ClangImporter/ClangImporterTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@ SWIFT_REQUEST(ClangImporter, ClangTypeEscapability,
CxxEscapability(EscapabilityLookupDescriptor), Cached,
NoLocationInfo)
SWIFT_REQUEST(ClangImporter, ClangDeclExplicitSafety,
ExplicitSafety(SafeUseOfCxxDeclDescriptor), Cached,
ExplicitSafety(CxxDeclExplicitSafetyDescriptor), Cached,
NoLocationInfo)
7 changes: 4 additions & 3 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1244,9 +1244,10 @@ ExplicitSafety Decl::getExplicitSafety() const {
// If this declaration is from C, ask the Clang importer.
if (auto clangDecl = getClangDecl()) {
ASTContext &ctx = getASTContext();
return evaluateOrDefault(ctx.evaluator,
ClangDeclExplicitSafety({clangDecl}),
ExplicitSafety::Unspecified);
return evaluateOrDefault(
ctx.evaluator,
ClangDeclExplicitSafety({clangDecl, isa<ClassDecl>(this)}),
ExplicitSafety::Unspecified);
}

// Inference: Check the enclosing context, unless this is a type.
Expand Down
36 changes: 28 additions & 8 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "swift/AST/Builtins.h"
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/ConcreteDeclRef.h"
#include "swift/AST/Decl.h"
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/DiagnosticsClangImporter.h"
#include "swift/AST/DiagnosticsSema.h"
Expand Down Expand Up @@ -8391,6 +8392,20 @@ SourceLoc swift::extractNearestSourceLoc(SafeUseOfCxxDeclDescriptor desc) {
return SourceLoc();
}

void swift::simple_display(llvm::raw_ostream &out,
CxxDeclExplicitSafetyDescriptor desc) {
out << "Checking if '";
if (auto namedDecl = dyn_cast<clang::NamedDecl>(desc.decl))
out << namedDecl->getNameAsString();
else
out << "<invalid decl>";
out << "' is explicitly safe.\n";
}

SourceLoc swift::extractNearestSourceLoc(CxxDeclExplicitSafetyDescriptor desc) {
return SourceLoc();
}

CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
Evaluator &evaluator, CustomRefCountingOperationDescriptor desc) const {
auto swiftDecl = desc.decl;
Expand Down Expand Up @@ -8468,9 +8483,11 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {

// Handle records recursively.
if (auto recordDecl = clangType->getAsTagDecl()) {
auto safety =
evaluateOrDefault(evaluator, ClangDeclExplicitSafety({recordDecl}),
ExplicitSafety::Unspecified);
// If we reached this point the types is not imported as a shared reference,
// so we don't need to check the bases whether they are shared references.
auto safety = evaluateOrDefault(
evaluator, ClangDeclExplicitSafety({recordDecl, false}),
ExplicitSafety::Unspecified);
switch (safety) {
case ExplicitSafety::Unsafe:
return true;
Expand All @@ -8485,10 +8502,9 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
return false;
}

ExplicitSafety ClangDeclExplicitSafety::evaluate(
Evaluator &evaluator,
SafeUseOfCxxDeclDescriptor desc
) const {
ExplicitSafety
ClangDeclExplicitSafety::evaluate(Evaluator &evaluator,
CxxDeclExplicitSafetyDescriptor desc) const {
// FIXME: Somewhat duplicative with importAsUnsafe.
// FIXME: Also similar to hasPointerInSubobjects
// FIXME: should probably also subsume IsSafeUseOfCxxDecl
Expand All @@ -8501,7 +8517,11 @@ ExplicitSafety ClangDeclExplicitSafety::evaluate(
// Explicitly safe.
if (hasSwiftAttribute(decl, "safe"))
return ExplicitSafety::Safe;


// Shared references are considered safe.
if (desc.isClass)
return ExplicitSafety::Safe;

// Enums are always safe.
if (isa<clang::EnumDecl>(decl))
return ExplicitSafety::Safe;
Expand Down
20 changes: 20 additions & 0 deletions test/Interop/Cxx/class/safe-interop-mode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ View safeFunc(View v1 [[clang::noescape]], View v2 [[clang::lifetimebound]]);
// Second non-escapable type is not annotated in any way.
void unsafeFunc(View v1 [[clang::noescape]], View v2);

class SharedObject {
private:
int *p;
} SWIFT_SHARED_REFERENCE(retainSharedObject, releaseSharedObject);

inline void retainSharedObject(SharedObject *) {}
inline void releaseSharedObject(SharedObject *) {}

struct DerivedFromSharedObject : SharedObject {};

//--- test.swift

import Test
Expand Down Expand Up @@ -134,3 +144,13 @@ func useUnsafeLifetimeAnnotated(v: View) {
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
unsafeFunc(v, v) // expected-note{{reference to unsafe global function 'unsafeFunc'}}
}

@available(SwiftStdlib 5.8, *)
func useSharedReference(frt: SharedObject) {
let _ = frt
}

@available(SwiftStdlib 5.8, *)
func useSharedReference(frt: DerivedFromSharedObject) {
let _ = frt
}