Skip to content

[C++] Fix double pointers to foreign ref types #73811

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.

Sign up for GitHub

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
May 22, 2024
Merged

[C++] Fix double pointers to foreign ref types #73811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 23 additions & 7 deletions lib/ClangImporter/ImportType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@ namespace {
: ImportHint::OtherPointer};
}

static bool
isDirectUseOfForeignReferenceType(clang::QualType clangPointeeType,
Type swiftPointeeType) {
return swiftPointeeType && swiftPointeeType->isForeignReferenceType() &&
!clangPointeeType->isPointerType();
}

class SwiftTypeConverter :
public clang::TypeVisitor<SwiftTypeConverter, ImportResult>
{
Expand Down Expand Up @@ -494,9 +501,10 @@ namespace {
pointeeQualType, ImportTypeKind::Value, addImportDiagnostic,
AllowNSUIntegerAsInt, Bridgeability::None, ImportTypeAttrs());

// If this is imported as a reference type, ignore the pointer.
if (pointeeType && pointeeType->isForeignReferenceType())
return {pointeeType, ImportHint::OtherPointer};
// If this is imported as a reference type, ignore the innermost pointer.
// (`T *` becomes `T`, but `T **` becomes `UnsafeMutablePointer<T>`.)
if (isDirectUseOfForeignReferenceType(pointeeQualType, pointeeType))
return {pointeeType, ImportHint::OtherPointer};

// If the pointed-to type is unrepresentable in Swift, or its C
// alignment is greater than the maximum Swift alignment, import as
Expand Down Expand Up @@ -592,7 +600,7 @@ namespace {
if (!pointeeType)
return Type();

if (pointeeType->isForeignReferenceType())
if (isDirectUseOfForeignReferenceType(pointeeQualType, pointeeType))
return {pointeeType, ImportHint::None};

if (pointeeQualType->isFunctionType()) {
Expand Down Expand Up @@ -2531,6 +2539,15 @@ ClangImporter::Implementation::importParameterType(
swiftParamTy = importedType.getType();
}

// `isInOut` is set above if we stripped off a mutable `&` before importing
// the type. Normally, we want to use an `inout` parameter in this situation.
// However, if the parameter belongs to a foreign reference type *and* the
// reference we stripped out was directly to that type (rather than to a
// pointer to that type), the foreign reference type should "eat" the
// indirection of the `&`, so we *don't* want to use an `inout` parameter.
if (isInOut && isDirectUseOfForeignReferenceType(paramTy, swiftParamTy))
isInOut = false;

return ImportParameterTypeResult{swiftParamTy, isInOut,
isParamTypeImplicitlyUnwrapped};
}
Expand Down Expand Up @@ -2611,9 +2628,8 @@ static ParamDecl *getParameterInfo(ClangImporter::Implementation *impl,

// Foreign references are already references so they don't need to be passed
// as inout.
paramInfo->setSpecifier(isInOut && !swiftParamTy->isForeignReferenceType()
? ParamSpecifier::InOut
: ParamSpecifier::Default);
paramInfo->setSpecifier(isInOut ? ParamSpecifier::InOut
: ParamSpecifier::Default);
paramInfo->setInterfaceType(swiftParamTy);
impl->recordImplicitUnwrapForDecl(paramInfo, isParamTypeImplicitlyUnwrapped);

Expand Down
39 changes: 39 additions & 0 deletions test/Interop/Cxx/ergonomics/swift-bridging-annotations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ public protocol Proto {
#if BRIDGING_HEADER_TEST
func f() -> SharedObject { return SharedObject.create() }

func g() {
var logger: LoggerSingleton?
var loggerPtr: UnsafeMutablePointer<LoggerSingleton?>?
var loggerPtrPtr: UnsafeMutablePointer<UnsafeMutablePointer<LoggerSingleton?>?>?

takeLoggersByPointer(logger, &logger, &loggerPtr)
takeLoggersByPointer(logger, loggerPtr, loggerPtrPtr)
takeLoggersByPointer(nil, nil, nil)

takeLoggersByReference(logger!, &logger, &loggerPtr)
takeLoggersByReference(logger!, &loggerPtr!.pointee, &loggerPtrPtr!.pointee)
}

func releaseSharedObject(_: SharedObject) { }
#endif

Expand Down Expand Up @@ -71,6 +84,12 @@ public:
static LoggerSingleton *getInstance();
};

void takeLoggersByPointer(LoggerSingleton *ptr, LoggerSingleton **ptr_ptr, LoggerSingleton ***ptr_ptr_ptr);
void takeLoggersByReference(LoggerSingleton &ref, LoggerSingleton *&ref_ptr, LoggerSingleton **&ref_ptr_ptr);

void takeLoggersByConstPointer(const LoggerSingleton **pointee0, LoggerSingleton const **pointee1, LoggerSingleton *const *pointer);
void takeLoggersByConstReference(const LoggerSingleton *&pointee0, LoggerSingleton const *&pointee1, LoggerSingleton *const &pointer);

class SWIFT_UNSAFE_REFERENCE UnsafeNonCopyable {
public:
UnsafeNonCopyable(UnsafeNonCopyable &) = delete;
Expand Down Expand Up @@ -110,6 +129,26 @@ private:
// CHECK: class func getInstance() -> LoggerSingleton!
// CHECK: }

// CHECK-LABEL: func takeLoggersByPointer(
// CHECK-SAME: _ ptr: LoggerSingleton!,
// CHECK-SAME: _ ptr_ptr: UnsafeMutablePointer<LoggerSingleton?>!,
// CHECK-SAME: _ ptr_ptr_ptr: UnsafeMutablePointer<UnsafeMutablePointer<LoggerSingleton?>?>!)

// CHECK-LABEL: func takeLoggersByReference(
// CHECK-SAME: _ ref: LoggerSingleton,
// CHECK-SAME: _ ref_ptr: inout LoggerSingleton!,
// CHECK-SAME: _ ref_ptr_ptr: inout UnsafeMutablePointer<LoggerSingleton?>!)

// CHECK-LABEL: func takeLoggersByConstPointer(
// CHECK-SAME: _ pointee0: UnsafeMutablePointer<LoggerSingleton?>!,
// CHECK-SAME: _ pointee1: UnsafeMutablePointer<LoggerSingleton?>!,
// CHECK-SAME: _ pointer: UnsafePointer<LoggerSingleton?>!)

// CHECK-LABEL: func takeLoggersByConstReference(
// CHECK-SAME: _ pointee0: inout LoggerSingleton!,
// CHECK-SAME: _ pointee1: inout LoggerSingleton!,
// CHECK-SAME: _ pointer: LoggerSingleton!)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems correct to me and matches the way we hide the const& indirection for value types.


// CHECK: class UnsafeNonCopyable {
// CHECK: }
// CHECK: func returnsPointerToUnsafeReference() -> UnsafeNonCopyable!
Expand Down