Skip to content
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
6 changes: 6 additions & 0 deletions TestFixtures/Api/TNSApi.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ typedef UIColor NIKColor;

- (BOOL)method:(NSInteger)errorCode error:(NSError**)outError;
- (BOOL)methodNullable:(NSInteger)errorCode error:(NSError* _Nullable* _Nullable)outError;

// The typedef carries its own nullability, so the parameter type ends up with
// nested nullability annotations (like BNNSFilterCreateLayerPadding in the SDK)
typedef NSError* _Nullable TNSNullableError;
- (BOOL)methodTypedefNullable:(NSInteger)errorCode
error:(TNSNullableError _Nullable* _Nullable)outError;
@end

@interface TNSConflictingSelectorTypes1 : NSObject
Expand Down
12 changes: 12 additions & 0 deletions TestFixtures/Api/TNSApi.m
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ - (BOOL)methodNullable:(NSInteger)errorCode error:(NSError* _Nullable* _Nullable
return errorCode == 0;
}

- (BOOL)methodTypedefNullable:(NSInteger)errorCode
error:(TNSNullableError _Nullable* _Nullable)outError {
if (outError) {
if (errorCode != 0) {
*outError = [NSError errorWithDomain:@"TNSErrorDomain" code:errorCode userInfo:nil];
} else {
*outError = nil;
}
}
return errorCode == 0;
}

@end

@implementation TNSConflictingSelectorTypes1
Expand Down
19 changes: 19 additions & 0 deletions TestRunner/app/tests/Marshalling/ObjCTypesTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,4 +575,23 @@ describe(module.id, function () {
TNSApi.new().methodNullableError(1, errorRef);
expect(errorRef.value instanceof NSError).toBe(true);
});

it("NSErrorOutParameterWithNullabilityCarryingTypedef", function () {
expect(function () {
TNSApi.new().methodTypedefNullableError(0);
}).not.toThrow();

var isThrown = false;
try {
TNSApi.new().methodTypedefNullableError(1);
} catch (e) {
isThrown = true;
expect(e.stack).toEqual(jasmine.any(String));
}
expect(isThrown).toBe(true);

var errorRef = new interop.Reference();
TNSApi.new().methodTypedefNullableError(1, errorRef);
expect(errorRef.value instanceof NSError).toBe(true);
});
});
18 changes: 7 additions & 11 deletions metadata-generator/src/Meta/MetaFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ void MetaFactory::createFromFunction(const clang::FunctionDecl& function,
// Clang doesn't handle The Create Rule automatically like for methods, so we
// have to do it manually
if (!(returnsRetained || returnsNotRetained) &&
functionMeta.signature[0]->is(TypeBridgedInterface)) {
functionMeta.signature[0]->stripNullability()->is(TypeBridgedInterface)) {
if (function.hasAttr<clang::CFAuditedTransferAttr>()) {
std::string functionName = function.getNameAsString();
if (functionName.find("Create") != string::npos ||
Expand Down Expand Up @@ -490,23 +490,19 @@ void MetaFactory::createFromMethod(const clang::ObjCMethodDecl& method,
isNullTerminatedVariadic);

// set MethodHasErrorOutParameter flag
// Nullability annotations (`NSError * _Nullable * _Nullable`) are wrapper
// nodes in the type tree, so strip them before inspecting the structure.
if (method.parameters().size() > 0) {
clang::ParmVarDecl* lastParameter =
method.parameters()[method.parameters().size() - 1];
Type* type = _typeFactory.create(lastParameter->getType()).get();
if (type != nullptr && (type->is(TypeType::TypeNullable) ||
type->is(TypeType::TypeNonNullable))) {
type = type->is(TypeType::TypeNullable)
? type->as<NullableType>().innerType
: type->as<NonNullableType>().innerType;
if (type != nullptr) {
type = type->stripNullability();
}
if (type != nullptr && type->is(TypeType::TypePointer)) {
Type* innerType = type->as<PointerType>().innerType;
if (innerType != nullptr && (innerType->is(TypeType::TypeNullable) ||
innerType->is(TypeType::TypeNonNullable))) {
innerType = innerType->is(TypeType::TypeNullable)
? innerType->as<NullableType>().innerType
: innerType->as<NonNullableType>().innerType;
if (innerType != nullptr) {
innerType = innerType->stripNullability();
}
if (innerType != nullptr && innerType->is(TypeType::TypeInterface) &&
innerType->as<InterfaceType>().interface->jsName == "NSError") {
Expand Down
25 changes: 25 additions & 0 deletions metadata-generator/src/Meta/TypeEntities.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ class Type {

bool is(TypeType type) const { return this->type == type; }

// Returns the underlying type with any NullableType/NonNullableType wrappers
// removed (handles arbitrarily nested wrappers). Use this before inspecting
// the structure of a type (e.g. is(TypePointer), as<InterfaceType>()), since
// nullability annotations are represented as wrapper nodes in the type tree.
Type* stripNullability();
const Type* stripNullability() const {
return const_cast<Type*>(this)->stripNullability();
}

template <class T>
T visit(TypeVisitor<T>& visitor) const {
switch (this->type) {
Expand Down Expand Up @@ -335,4 +344,20 @@ class NonNullableType : public Type {

Type* innerType;
};

inline Type* Type::stripNullability() {
Type* current = this;
while (true) {
Type* inner = nullptr;
if (current->is(TypeType::TypeNullable)) {
inner = current->as<NullableType>().innerType;
} else if (current->is(TypeType::TypeNonNullable)) {
inner = current->as<NonNullableType>().innerType;
}
if (inner == nullptr) {
return current;
}
current = inner;
}
}
} // namespace Meta
8 changes: 6 additions & 2 deletions metadata-generator/src/Meta/TypeFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,14 @@ shared_ptr<Type> TypeFactory::createFromAttributedType(
const clang::AttributedType* type) {
auto innerType = this->create(type->getModifiedType());
if (auto nullability = type->getImmediateNullability()) {
// Strip any nullability wrapper the inner type may already carry (e.g. a
// typedef that has its own annotation) so wrappers never nest and the
// outermost annotation wins. The stripped type stays alive in _cache.
Type* unwrapped = innerType->stripNullability();
if (*nullability == clang::NullabilityKind::Nullable) {
return make_shared<NullableType>(innerType.get());
return make_shared<NullableType>(unwrapped);
} else if (*nullability == clang::NullabilityKind::NonNull) {
return make_shared<NonNullableType>(innerType.get());
return make_shared<NonNullableType>(unwrapped);
}
}
return innerType;
Expand Down
Loading
Loading