Skip to content

[Clang importer] Honor swift_bridged_typedef attribute. #16229

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
Apr 28, 2018
Merged

[Clang importer] Honor swift_bridged_typedef attribute. #16229

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
2 changes: 1 addition & 1 deletion lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2529,7 +2529,7 @@ namespace {
clang::QualType ClangType = Decl->getUnderlyingType();
SwiftType = Impl.importTypeIgnoreIUO(
ClangType, ImportTypeKind::Typedef, isInSystemModule(DC),
getTypedefBridgeability(ClangType), OTK_Optional);
getTypedefBridgeability(Decl, ClangType), OTK_Optional);
}

if (!SwiftType)
Expand Down
25 changes: 15 additions & 10 deletions lib/ClangImporter/ImportType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,12 +663,11 @@ namespace {
auto underlyingType = type->desugar();

// Figure out the bridgeability we would normally use for this typedef.
auto typedefBridgeability = getTypedefBridgeability(underlyingType);
auto typedefBridgeability =
getTypedefBridgeability(type->getDecl(), underlyingType);

// Figure out the typedef we should actually use.
auto underlyingBridgeability =
(Bridging == Bridgeability::Full
? typedefBridgeability : Bridgeability::None);
auto underlyingBridgeability = Bridging;
SwiftTypeConverter innerConverter(Impl, AllowNSUIntegerAsInt,
underlyingBridgeability);
auto underlyingResult = innerConverter.Visit(underlyingType);
Expand All @@ -685,7 +684,8 @@ namespace {
#ifndef NDEBUG
switch (underlyingResult.Hint) {
case ImportHint::Block:
// Blocks change in all sorts of ways, due to bridging.
case ImportHint::ObjCBridged:
// Bridging is fine for Objective-C and blocks.
break;
case ImportHint::NSUInteger:
// NSUInteger might be imported as Int rather than UInt depending
Expand Down Expand Up @@ -1088,7 +1088,6 @@ namespace {
static bool canBridgeTypes(ImportTypeKind importKind) {
switch (importKind) {
case ImportTypeKind::Abstract:
case ImportTypeKind::Typedef:
case ImportTypeKind::Value:
case ImportTypeKind::Variable:
case ImportTypeKind::AuditedVariable:
Expand All @@ -1104,6 +1103,7 @@ static bool canBridgeTypes(ImportTypeKind importKind) {
case ImportTypeKind::Property:
case ImportTypeKind::PropertyWithReferenceSemantics:
case ImportTypeKind::BridgedValue:
case ImportTypeKind::Typedef:
return true;
}

Expand Down Expand Up @@ -1286,7 +1286,7 @@ static ImportedType adjustTypeForConcreteImport(
// In a bridgeable context, or in the direct structure of a typedef,
// we would prefer to instead use the default Swift convention.
if (hint == ImportHint::Block) {
if (canBridgeTypes(importKind) || importKind == ImportTypeKind::Typedef) {
if (canBridgeTypes(importKind)) {
auto fTy = importedType->castTo<FunctionType>();
FunctionType::ExtInfo einfo = fTy->getExtInfo();
if (einfo.getRepresentation() != FunctionTypeRepresentation::Swift) {
Expand Down Expand Up @@ -1330,13 +1330,18 @@ static ImportedType adjustTypeForConcreteImport(
// bridge, do so.
if (hint == ImportHint::ObjCBridged &&
canBridgeTypes(importKind) &&
importKind != ImportTypeKind::PropertyWithReferenceSemantics) {
importKind != ImportTypeKind::PropertyWithReferenceSemantics &&
!(importKind == ImportTypeKind::Typedef &&
bridging == Bridgeability::None)) {
// id and Any can be bridged without Foundation. There would be
// bootstrapping issues with the ObjectiveC module otherwise.
if (hint.BridgedType->isAny()
|| impl.tryLoadFoundationModule()
|| impl.ImportForwardDeclarations) {
importedType = hint.BridgedType;

// Set the bridged type if it wasn't done already.
if (!importedType->isEqual(hint.BridgedType))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this check?

Copy link
Member Author

Choose a reason for hiding this comment

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

hint.BridgedType might be a sugared version of importedType, in which case we want to keep that sugar.

importedType = hint.BridgedType;
}
}

Expand Down Expand Up @@ -2484,7 +2489,7 @@ Type ClangImporter::Implementation::getSugaredTypeReference(TypeDecl *type) {
}

return NameAliasType::get(typealias, parentType, SubstitutionMap(),
typealias->getUnderlyingTypeLoc().getType());
typealias->getUnderlyingTypeLoc().getType());
}

return type->getDeclaredInterfaceType();
Expand Down
10 changes: 8 additions & 2 deletions lib/ClangImporter/ImporterImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,14 @@ enum class Bridgeability {
///
/// In either case we end up losing sugar at some uses sites, so this is more
/// about what the right default is.
static inline Bridgeability getTypedefBridgeability(clang::QualType type) {
return type->isBlockPointerType() ? Bridgeability::Full : Bridgeability::None;
static inline Bridgeability getTypedefBridgeability(
const clang::TypedefNameDecl *decl,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: LLVM style prefers double-indenting here rather than max-indenting.

clang::QualType type) {
return decl->hasAttr<clang::SwiftBridgedTypedefAttr>()
? Bridgeability::Full
: type->isBlockPointerType()
? Bridgeability::Full
: Bridgeability::None;
}

/// \brief Describes the kind of the C type that can be mapped to a stdlib
Expand Down
8 changes: 8 additions & 0 deletions test/IDE/print_clang_decls.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
// RUN: %target-swift-ide-test(mock-sdk: -sdk %S/../Inputs/clang-importer-sdk -I %t) -print-module -source-filename %s -module-to-print=nullability -function-definitions=false -prefer-type-repr=true > %t.printed.txt
// RUN: %FileCheck %s -check-prefix=CHECK-NULLABILITY -strict-whitespace < %t.printed.txt

// RUN: %target-swift-ide-test(mock-sdk: -sdk %S/../Inputs/clang-importer-sdk -I %t) -print-module -source-filename %s -module-to-print=bridged_typedef -function-definitions=false -prefer-type-repr=true > %t.printed.txt
// RUN: %FileCheck %s -check-prefix=CHECK-BRIDGED-TYPEDEF -strict-whitespace < %t.printed.txt

// TAG_DECLS_AND_TYPEDEFS: {{^}}struct FooStruct1 {{{$}}
// TAG_DECLS_AND_TYPEDEFS: {{^}} var x: Int32{{$}}
// TAG_DECLS_AND_TYPEDEFS: {{^}} var y: Double{{$}}
Expand Down Expand Up @@ -153,3 +156,8 @@
// CHECK-NULLABILITY: func optArrayMethod() -> [Any]?
// CHECK-NULLABILITY: }
// CHECK-NULLABILITY: func compare_classes(_ sc1: SomeClass, _ sc2: SomeClass, _ sc3: SomeClass!)

// CHECK-BRIDGED-TYPEDEF: typealias NSMyAmazingStringAlias = String
// CHECK-BRIDGED-TYPEDEF: func acceptNSMyAmazingStringAlias(_ param: NSMyAmazingStringAlias?)
// CHECK-BRIDGED-TYPEDEF: func acceptNSMyAmazingStringAliasArray(_ param: [NSMyAmazingStringAlias])
// CHECK-BRIDGED-TYPEDEF: func acceptIndirectedAmazingAlias(_ param: AutoreleasingUnsafeMutablePointer<NSString>?)
13 changes: 13 additions & 0 deletions test/Inputs/clang-importer-sdk/usr/include/bridged_typedef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#ifndef BRIDGED_TYPEDEF_H
#define BRIDGED_TYPEDEF_H

@import Foundation;


typedef NSString *NSMyAmazingStringAlias __attribute__((swift_bridged_typedef));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test a typedef that uses this typedef as well? If that typedef doesn't have the swift_bridged_typedef attribute, what behavior should it have?


void acceptNSMyAmazingStringAlias(NSMyAmazingStringAlias _Nullable param);
void acceptNSMyAmazingStringAliasArray(NSArray<NSMyAmazingStringAlias> * _Nonnull param);
void acceptIndirectedAmazingAlias(NSMyAmazingStringAlias _Nonnull * _Nullable param);

#endif
5 changes: 5 additions & 0 deletions test/Inputs/clang-importer-sdk/usr/include/module.map
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,8 @@ module OtherSubscripts {
header "OtherSubscripts.h"
export *
}

module bridged_typedef {
header "bridged_typedef.h"
export *
}