Skip to content

Commit ddacdf4

Browse files
author
Gabor Horvath
committed
[cxx-interop] Shared references are considered safe
This patch makes sure we don't get warnings in strict memory safe mode when using shared references. Those types are reference counted so we are unlikely to run into lifetime errors. rdar://151039766
1 parent 276261c commit ddacdf4

File tree

5 files changed

+83
-14
lines changed

5 files changed

+83
-14
lines changed

include/swift/ClangImporter/ClangImporterRequests.h

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,10 +576,37 @@ class ClangTypeEscapability
576576
void simple_display(llvm::raw_ostream &out, EscapabilityLookupDescriptor desc);
577577
SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc);
578578

579+
struct CxxDeclExplicitSafetyDescriptor final {
580+
const clang::Decl *decl;
581+
bool isClass;
582+
583+
CxxDeclExplicitSafetyDescriptor(const clang::Decl *decl, bool isClass)
584+
: decl(decl), isClass(isClass) {}
585+
586+
friend llvm::hash_code
587+
hash_value(const CxxDeclExplicitSafetyDescriptor &desc) {
588+
return llvm::hash_combine(desc.decl, desc.isClass);
589+
}
590+
591+
friend bool operator==(const CxxDeclExplicitSafetyDescriptor &lhs,
592+
const CxxDeclExplicitSafetyDescriptor &rhs) {
593+
return lhs.decl == rhs.decl && lhs.isClass == rhs.isClass;
594+
}
595+
596+
friend bool operator!=(const CxxDeclExplicitSafetyDescriptor &lhs,
597+
const CxxDeclExplicitSafetyDescriptor &rhs) {
598+
return !(lhs == rhs);
599+
}
600+
};
601+
602+
void simple_display(llvm::raw_ostream &out,
603+
CxxDeclExplicitSafetyDescriptor desc);
604+
SourceLoc extractNearestSourceLoc(CxxDeclExplicitSafetyDescriptor desc);
605+
579606
/// Determine the safety of the given Clang declaration.
580607
class ClangDeclExplicitSafety
581608
: public SimpleRequest<ClangDeclExplicitSafety,
582-
ExplicitSafety(SafeUseOfCxxDeclDescriptor),
609+
ExplicitSafety(CxxDeclExplicitSafetyDescriptor),
583610
RequestFlags::Cached> {
584611
public:
585612
using SimpleRequest::SimpleRequest;
@@ -592,7 +619,8 @@ class ClangDeclExplicitSafety
592619
friend SimpleRequest;
593620

594621
// Evaluation.
595-
ExplicitSafety evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const;
622+
ExplicitSafety evaluate(Evaluator &evaluator,
623+
CxxDeclExplicitSafetyDescriptor desc) const;
596624
};
597625

598626
#define SWIFT_TYPEID_ZONE ClangImporter

include/swift/ClangImporter/ClangImporterTypeIDZone.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,5 @@ SWIFT_REQUEST(ClangImporter, ClangTypeEscapability,
4646
CxxEscapability(EscapabilityLookupDescriptor), Cached,
4747
NoLocationInfo)
4848
SWIFT_REQUEST(ClangImporter, ClangDeclExplicitSafety,
49-
ExplicitSafety(SafeUseOfCxxDeclDescriptor), Cached,
49+
ExplicitSafety(CxxDeclExplicitSafetyDescriptor), Cached,
5050
NoLocationInfo)

lib/AST/Decl.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,9 +1244,10 @@ ExplicitSafety Decl::getExplicitSafety() const {
12441244
// If this declaration is from C, ask the Clang importer.
12451245
if (auto clangDecl = getClangDecl()) {
12461246
ASTContext &ctx = getASTContext();
1247-
return evaluateOrDefault(ctx.evaluator,
1248-
ClangDeclExplicitSafety({clangDecl}),
1249-
ExplicitSafety::Unspecified);
1247+
return evaluateOrDefault(
1248+
ctx.evaluator,
1249+
ClangDeclExplicitSafety({clangDecl, isa<ClassDecl>(this)}),
1250+
ExplicitSafety::Unspecified);
12501251
}
12511252

12521253
// Inference: Check the enclosing context, unless this is a type.

lib/ClangImporter/ClangImporter.cpp

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "swift/AST/Builtins.h"
2525
#include "swift/AST/ClangModuleLoader.h"
2626
#include "swift/AST/ConcreteDeclRef.h"
27+
#include "swift/AST/Decl.h"
2728
#include "swift/AST/DiagnosticEngine.h"
2829
#include "swift/AST/DiagnosticsClangImporter.h"
2930
#include "swift/AST/DiagnosticsSema.h"
@@ -8391,6 +8392,20 @@ SourceLoc swift::extractNearestSourceLoc(SafeUseOfCxxDeclDescriptor desc) {
83918392
return SourceLoc();
83928393
}
83938394

8395+
void swift::simple_display(llvm::raw_ostream &out,
8396+
CxxDeclExplicitSafetyDescriptor desc) {
8397+
out << "Checking if '";
8398+
if (auto namedDecl = dyn_cast<clang::NamedDecl>(desc.decl))
8399+
out << namedDecl->getNameAsString();
8400+
else
8401+
out << "<invalid decl>";
8402+
out << "' is explicitly safe.\n";
8403+
}
8404+
8405+
SourceLoc swift::extractNearestSourceLoc(CxxDeclExplicitSafetyDescriptor desc) {
8406+
return SourceLoc();
8407+
}
8408+
83948409
CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
83958410
Evaluator &evaluator, CustomRefCountingOperationDescriptor desc) const {
83968411
auto swiftDecl = desc.decl;
@@ -8468,9 +8483,11 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
84688483

84698484
// Handle records recursively.
84708485
if (auto recordDecl = clangType->getAsTagDecl()) {
8471-
auto safety =
8472-
evaluateOrDefault(evaluator, ClangDeclExplicitSafety({recordDecl}),
8473-
ExplicitSafety::Unspecified);
8486+
// If we reached this point the types is not imported as a shared reference,
8487+
// so we don't need to check the bases whether they are shared references.
8488+
auto safety = evaluateOrDefault(
8489+
evaluator, ClangDeclExplicitSafety({recordDecl, false}),
8490+
ExplicitSafety::Unspecified);
84748491
switch (safety) {
84758492
case ExplicitSafety::Unsafe:
84768493
return true;
@@ -8485,10 +8502,9 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
84858502
return false;
84868503
}
84878504

8488-
ExplicitSafety ClangDeclExplicitSafety::evaluate(
8489-
Evaluator &evaluator,
8490-
SafeUseOfCxxDeclDescriptor desc
8491-
) const {
8505+
ExplicitSafety
8506+
ClangDeclExplicitSafety::evaluate(Evaluator &evaluator,
8507+
CxxDeclExplicitSafetyDescriptor desc) const {
84928508
// FIXME: Somewhat duplicative with importAsUnsafe.
84938509
// FIXME: Also similar to hasPointerInSubobjects
84948510
// FIXME: should probably also subsume IsSafeUseOfCxxDecl
@@ -8501,7 +8517,11 @@ ExplicitSafety ClangDeclExplicitSafety::evaluate(
85018517
// Explicitly safe.
85028518
if (hasSwiftAttribute(decl, "safe"))
85038519
return ExplicitSafety::Safe;
8504-
8520+
8521+
// Shared references are considered safe.
8522+
if (desc.isClass)
8523+
return ExplicitSafety::Safe;
8524+
85058525
// Enums are always safe.
85068526
if (isa<clang::EnumDecl>(decl))
85078527
return ExplicitSafety::Safe;

test/Interop/Cxx/class/safe-interop-mode.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ View safeFunc(View v1 [[clang::noescape]], View v2 [[clang::lifetimebound]]);
6363
// Second non-escapable type is not annotated in any way.
6464
void unsafeFunc(View v1 [[clang::noescape]], View v2);
6565

66+
class SharedObject {
67+
private:
68+
int *p;
69+
} SWIFT_SHARED_REFERENCE(retainSharedObject, releaseSharedObject);
70+
71+
inline void retainSharedObject(SharedObject *) {}
72+
inline void releaseSharedObject(SharedObject *) {}
73+
74+
struct DerivedFromSharedObject : SharedObject {};
75+
6676
//--- test.swift
6777

6878
import Test
@@ -134,3 +144,13 @@ func useUnsafeLifetimeAnnotated(v: View) {
134144
// expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
135145
unsafeFunc(v, v) // expected-note{{reference to unsafe global function 'unsafeFunc'}}
136146
}
147+
148+
@available(SwiftStdlib 5.8, *)
149+
func useSharedReference(frt: SharedObject) {
150+
let _ = frt
151+
}
152+
153+
@available(SwiftStdlib 5.8, *)
154+
func useSharedReference(frt: DerivedFromSharedObject) {
155+
let _ = frt
156+
}

0 commit comments

Comments
 (0)