Skip to content

[Clang importer] Implement support for importing _Nullable_result. #34973

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 2 commits into from
Dec 9, 2020
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
8 changes: 7 additions & 1 deletion lib/ClangImporter/ClangAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,12 +607,18 @@ bool importer::hasNativeSwiftDecl(const clang::Decl *decl) {

/// Translate the "nullability" notion from API notes into an optional type
/// kind.
OptionalTypeKind importer::translateNullability(clang::NullabilityKind kind) {
OptionalTypeKind importer::translateNullability(
clang::NullabilityKind kind, bool stripNonResultOptionality) {
if (stripNonResultOptionality &&
kind != clang::NullabilityKind::NullableResult)
return OptionalTypeKind::OTK_None;

switch (kind) {
case clang::NullabilityKind::NonNull:
return OptionalTypeKind::OTK_None;

case clang::NullabilityKind::Nullable:
case clang::NullabilityKind::NullableResult:
return OptionalTypeKind::OTK_Optional;

case clang::NullabilityKind::Unspecified:
Expand Down
6 changes: 5 additions & 1 deletion lib/ClangImporter/ClangAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,11 @@ bool isNSString(clang::QualType);
bool hasNativeSwiftDecl(const clang::Decl *decl);

/// Translation API nullability from an API note into an optional kind.
OptionalTypeKind translateNullability(clang::NullabilityKind kind);
///
/// \param stripNonResultOptionality Whether strip optionality from
/// \c _Nullable but not \c _Nullable_result.
OptionalTypeKind translateNullability(
clang::NullabilityKind kind, bool stripNonResultOptionality = false);

/// Determine whether the given method is a required initializer
/// of the given class.
Expand Down
78 changes: 62 additions & 16 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,18 @@ namespace {
ClangImporter::Implementation &Impl;
bool AllowNSUIntegerAsInt;
Bridgeability Bridging;
const clang::FunctionType *CompletionHandlerType;
Optional<unsigned> CompletionHandlerErrorParamIndex;

public:
SwiftTypeConverter(ClangImporter::Implementation &impl,
bool allowNSUIntegerAsInt,
Bridgeability bridging)
Bridgeability bridging,
const clang::FunctionType *completionHandlerType,
Optional<unsigned> completionHandlerErrorParamIndex)
: Impl(impl), AllowNSUIntegerAsInt(allowNSUIntegerAsInt),
Bridging(bridging) {}
Bridging(bridging), CompletionHandlerType(completionHandlerType),
CompletionHandlerErrorParamIndex(completionHandlerErrorParamIndex) {}

using TypeVisitor::Visit;
ImportResult Visit(clang::QualType type) {
Expand Down Expand Up @@ -612,8 +617,19 @@ namespace {
for (auto param = type->param_type_begin(),
paramEnd = type->param_type_end();
param != paramEnd; ++param) {
// Determine whether we have a result parameter of a completion
// handler that can also express a thrown error.
ImportTypeKind paramImportKind = ImportTypeKind::Parameter;
unsigned paramIdx = param - type->param_type_begin();
if (CompletionHandlerType &&
Impl.getClangASTContext().hasSameType(
CompletionHandlerType, type) &&
paramIdx != CompletionHandlerErrorParamIndex) {
paramImportKind = ImportTypeKind::CompletionHandlerResultParameter;
}

auto swiftParamTy = Impl.importTypeIgnoreIUO(
*param, ImportTypeKind::Parameter, AllowNSUIntegerAsInt, Bridging,
*param, paramImportKind, AllowNSUIntegerAsInt, Bridging,
OTK_Optional);
if (!swiftParamTy)
return Type();
Expand Down Expand Up @@ -1191,6 +1207,7 @@ static bool canBridgeTypes(ImportTypeKind importKind) {
case ImportTypeKind::Result:
case ImportTypeKind::AuditedResult:
case ImportTypeKind::Parameter:
case ImportTypeKind::CompletionHandlerResultParameter:
case ImportTypeKind::CFRetainedOutParameter:
case ImportTypeKind::CFUnretainedOutParameter:
case ImportTypeKind::Property:
Expand Down Expand Up @@ -1218,6 +1235,7 @@ static bool isCFAudited(ImportTypeKind importKind) {
case ImportTypeKind::AuditedVariable:
case ImportTypeKind::AuditedResult:
case ImportTypeKind::Parameter:
case ImportTypeKind::CompletionHandlerResultParameter:
case ImportTypeKind::CFRetainedOutParameter:
case ImportTypeKind::CFUnretainedOutParameter:
case ImportTypeKind::Property:
Expand Down Expand Up @@ -1520,7 +1538,8 @@ static ImportedType adjustTypeForConcreteImport(
ImportedType ClangImporter::Implementation::importType(
clang::QualType type, ImportTypeKind importKind, bool allowNSUIntegerAsInt,
Bridgeability bridging, OptionalTypeKind optionality,
bool resugarNSErrorPointer) {
bool resugarNSErrorPointer,
Optional<unsigned> completionHandlerErrorParamIndex) {
if (type.isNull())
return {Type(), false};

Expand Down Expand Up @@ -1555,11 +1574,28 @@ ImportedType ClangImporter::Implementation::importType(
// If nullability is provided as part of the type, that overrides
// optionality provided externally.
if (auto nullability = type->getNullability(clangContext)) {
optionality = translateNullability(*nullability);
bool stripNonResultOptionality =
importKind == ImportTypeKind::CompletionHandlerResultParameter;

optionality = translateNullability(*nullability, stripNonResultOptionality);
}

// If this is a completion handler parameter, record the function type whose
// parameters will act as the results of the completion handler.
const clang::FunctionType *completionHandlerType = nullptr;
if (completionHandlerErrorParamIndex) {
if (auto blockPtrType = type->getAs<clang::BlockPointerType>()) {
completionHandlerType =
blockPtrType->getPointeeType()->castAs<clang::FunctionType>();

type = clang::QualType(blockPtrType, 0);
}
}

// Perform abstract conversion, ignoring how the type is actually used.
SwiftTypeConverter converter(*this, allowNSUIntegerAsInt, bridging);
SwiftTypeConverter converter(
*this, allowNSUIntegerAsInt, bridging,
completionHandlerType, completionHandlerErrorParamIndex);
auto importResult = converter.Visit(type);

// Now fix up the type based on how we're concretely using it.
Expand Down Expand Up @@ -2085,13 +2121,7 @@ static Type decomposeCompletionHandlerType(
paramIdx == *info.completionHandlerErrorParamIndex())
continue;

// If there is an error parameter, remove nullability.
Type paramType = param.getPlainType();
// TODO: Clang should gain a nullability form that overrides this.
if (info.completionHandlerErrorParamIndex())
paramType = paramType->lookThroughAllOptionalTypes();

resultTypeElts.push_back(paramType);
resultTypeElts.push_back(param.getPlainType());
}

switch (resultTypeElts.size()) {
Expand Down Expand Up @@ -2266,6 +2296,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
}

// Special case for NSDictionary's subscript.
ImportTypeKind importKind = ImportTypeKind::Parameter;
Type swiftParamTy;
bool paramIsIUO;
if (kind == SpecialMethodKind::NSDictionarySubscriptGetter &&
Expand All @@ -2276,12 +2307,19 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(

paramIsIUO = optionalityOfParam == OTK_ImplicitlyUnwrappedOptional;
} else {
ImportTypeKind importKind = ImportTypeKind::Parameter;
if (param->hasAttr<clang::CFReturnsRetainedAttr>())
importKind = ImportTypeKind::CFRetainedOutParameter;
else if (param->hasAttr<clang::CFReturnsNotRetainedAttr>())
importKind = ImportTypeKind::CFUnretainedOutParameter;

// Figure out if this is a completion handler parameter whose error
// parameter is used to indicate throwing.
Optional<unsigned> completionHandlerErrorParamIndex;
if (paramIsCompletionHandler) {
completionHandlerErrorParamIndex =
asyncInfo->completionHandlerErrorParamIndex();
}

// If this is the throws error parameter, we don't need to convert any
// NSError** arguments to the sugared NSErrorPointer typealias form,
// because all that is done with it is retrieving the canonical
Expand All @@ -2293,7 +2331,8 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
auto importedParamType =
importType(paramTy, importKind, allowNSUIntegerAsIntInParam,
Bridgeability::Full, optionalityOfParam,
/*resugarNSErrorPointer=*/!paramIsError);
/*resugarNSErrorPointer=*/!paramIsError,
completionHandlerErrorParamIndex);
paramIsIUO = importedParamType.isImplicitlyUnwrapped();
swiftParamTy = importedParamType.getType();
}
Expand Down Expand Up @@ -2321,7 +2360,14 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
if (Type replacedSwiftResultTy =
decomposeCompletionHandlerType(swiftParamTy, *asyncInfo)) {
swiftResultTy = replacedSwiftResultTy;
completionHandlerType = swiftParamTy->getCanonicalType();

// Import the original completion handler type without adjustments.
Type origSwiftParamTy = importType(
paramTy, importKind, allowNSUIntegerAsIntInParam,
Bridgeability::Full, optionalityOfParam,
/*resugarNSErrorPointer=*/!paramIsError, None).getType();
completionHandlerType = mapGenericArgs(origDC, dc, origSwiftParamTy)
->getCanonicalType();
continue;
}

Expand Down
10 changes: 9 additions & 1 deletion lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,13 @@ enum class ImportTypeKind {
/// Parameters are always considered CF-audited.
Parameter,

/// Import the type of a parameter to a completion handler that can indicate
/// a thrown error.
///
/// Special handling:
/// * _Nullable_result is treated as _Nonnull rather than _Nullable_result.
CompletionHandlerResultParameter,

/// Import the type of a parameter declared with
/// \c CF_RETURNS_RETAINED.
///
Expand Down Expand Up @@ -1036,7 +1043,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
importType(clang::QualType type, ImportTypeKind kind,
bool allowNSUIntegerAsInt, Bridgeability topLevelBridgeability,
OptionalTypeKind optional = OTK_ImplicitlyUnwrappedOptional,
bool resugarNSErrorPointer = true);
bool resugarNSErrorPointer = true,
Optional<unsigned> completionHandlerErrorParamIndex = None);

/// Import the given Clang type into Swift.
///
Expand Down
9 changes: 9 additions & 0 deletions test/ClangImporter/objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ func testSlowServer(slowServer: SlowServer) async throws {
let _: Bool = await slowServer.checkAvailability()
let _: String = await try slowServer.findAnswer()
let _: String = await try slowServer.findAnswerFailingly()

let (aOpt, b) = await try slowServer.findQAndA()
if let a = aOpt { // make sure aOpt is optional
print(a)
}
let _: String = b // make sure b is non-optional

let _: String = await try slowServer.findAnswer()

let _: Void = await slowServer.doSomethingFun("jump")
let _: (Int) -> Void = slowServer.completionHandler

Expand Down
2 changes: 2 additions & 0 deletions test/IDE/print_clang_objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
// CHECK-DAG: func findAnswer() async throws -> String
// CHECK-DAG: func findAnswerFailingly(completionHandler handler: @escaping (String?, Error?) -> Void) throws
// CHECK-DAG: func findAnswerFailingly() async throws -> String
// CHECK-DAG: func findQAndA() async throws -> (String?, String)
// CHECK-DAG: func findQuestionableAnswers() async throws -> (String, String?)
// CHECK-DAG: func doSomethingFun(_ operation: String) async
// CHECK: {{^[}]$}}

Expand Down
4 changes: 4 additions & 0 deletions test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
-(void)allOperationsWithCompletionHandler:(void (^)(NSArray<NSString *> *))completion;
@end

typedef void (^CompletionHandler)(NSString * _Nullable, NSString * _Nullable_result, NSError * _Nullable);

@interface SlowServer : NSObject <ServiceProvider>
-(void)doSomethingSlow:(NSString *)operation completionHandler:(void (^)(NSInteger))handler;
-(void)doSomethingDangerous:(NSString *)operation completionHandler:(void (^ _Nullable)(NSString *_Nullable, NSError * _Nullable))handler;
Expand All @@ -17,6 +19,8 @@
-(void)replyingOperation:(NSString *)operation replyTo:(void (^)(NSString *))block;
-(void)findAnswerAsynchronously:(void (^)(NSString *_Nullable, NSError * _Nullable))handler __attribute__((swift_name("findAnswer(completionHandler:)")));
-(BOOL)findAnswerFailinglyWithError:(NSError * _Nullable * _Nullable)error completion:(void (^)(NSString *_Nullable, NSError * _Nullable))handler __attribute__((swift_name("findAnswerFailingly(completionHandler:)")));
-(void)findQAndAWithCompletionHandler:(void (^)(NSString *_Nullable_result, NSString *_Nullable answer, NSError * _Nullable))handler;
-(void)findQuestionableAnswersWithCompletionHandler:(CompletionHandler)handler;
-(void)doSomethingFun:(NSString *)operation then:(void (^)(void))completionHandler;
-(void)getFortuneAsynchronouslyWithCompletionHandler:(void (^)(NSString *_Nullable, NSError * _Nullable))handler;
-(void)getMagicNumberAsynchronouslyWithSeed:(NSInteger)seed completionHandler:(void (^)(NSInteger, NSError * _Nullable))handler;
Expand Down