Possible Proposal: Foundation corelibs API change necessary for NSPredicate


(Kevin Lundberg) #1

I have a pending pull request that needs a little more work around NSPredicates, but in my testing on darwin foundation, I’ve discovered what appears to be an obj-c nullability annotation bug. When constructing a block predicate, the type of the block is this:

(AnyObject, [String: AnyObject]?) -> Bool

However, the type signature of evaluateObject(_:substitutionVariables:) is

(AnyObject?, [String: AnyObject]?) -> Bool

Note the optional AnyObject here. In Xcode 7.2 with swift 2.1, the following code causes an EXC_BAD_ACCESS signal when calling evaluateWithObject: in a playground:

let pred = NSPredicate(block: { (obj: AnyObject, bindings: [String: AnyObject]?) -> Bool in
    print(obj)
    return false
})
print(pred.evaluateWithObject(nil))

because obj is in fact optional here, but the type of the block does not allow for this.

There are two possible approaches here; removing the optional type from evaluateWithObject, or adding it to the block constructor for NSPredicate. Such a change is also presumably trivial to port back to darwin foundation, as that at minimum would need to merely change nullability annotations for these components of NSPredicate. These involve a public-api change which by my understanding needs to go through the swift evolution process.

Before sending this over to swift-evolution which is already pretty high-traffic, I wanted to float this here to make sure that this is appropriate for that process. Is it enough to draft a proposal outright or for comprehensiveness sake should I also send this out to that list to open discussion first?

Is there anyone on this list that has an opinion over which approach to take for changing the api here?

Thanks!

···

--
Kevin Lundberg
kevin@klundberg.com


(Philippe Hausler) #2

This might actually just be a bug in our annotations of what is nullable and what is not. I would have to double check but it seems pretty reasonable that it should have been nullable to begin with.

···

On Dec 14, 2015, at 10:06 AM, Kevin Lundberg via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

I have a pending pull request that needs a little more work around NSPredicates, but in my testing on darwin foundation, I’ve discovered what appears to be an obj-c nullability annotation bug. When constructing a block predicate, the type of the block is this:

(AnyObject, [String: AnyObject]?) -> Bool

However, the type signature of evaluateObject(_:substitutionVariables:) is

(AnyObject?, [String: AnyObject]?) -> Bool

Note the optional AnyObject here. In Xcode 7.2 with swift 2.1, the following code causes an EXC_BAD_ACCESS signal when calling evaluateWithObject: in a playground:

let pred = NSPredicate(block: { (obj: AnyObject, bindings: [String: AnyObject]?) -> Bool in
    print(obj)
    return false
})
print(pred.evaluateWithObject(nil))

because obj is in fact optional here, but the type of the block does not allow for this.

There are two possible approaches here; removing the optional type from evaluateWithObject, or adding it to the block constructor for NSPredicate. Such a change is also presumably trivial to port back to darwin foundation, as that at minimum would need to merely change nullability annotations for these components of NSPredicate. These involve a public-api change which by my understanding needs to go through the swift evolution process.

Before sending this over to swift-evolution which is already pretty high-traffic, I wanted to float this here to make sure that this is appropriate for that process. Is it enough to draft a proposal outright or for comprehensiveness sake should I also send this out to that list to open discussion first?

Is there anyone on this list that has an opinion over which approach to take for changing the api here?

Thanks!

--
Kevin Lundberg
kevin@klundberg.com <mailto:kevin@klundberg.com>

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Kevin Lundberg) #3

Great, should I file a radar on bugreport.apple.com for this? And if it’s simple enough to fix that can I apply the same fix to the SwiftFoundation API as a PR without any extra process?

···

--
Kevin Lundberg
kevin@klundberg.com

On Dec 14, 2015, at 1:09 PM, Philippe Hausler <phausler@apple.com> wrote:

This might actually just be a bug in our annotations of what is nullable and what is not. I would have to double check but it seems pretty reasonable that it should have been nullable to begin with.

On Dec 14, 2015, at 10:06 AM, Kevin Lundberg via swift-corelibs-dev <swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>> wrote:

I have a pending pull request that needs a little more work around NSPredicates, but in my testing on darwin foundation, I’ve discovered what appears to be an obj-c nullability annotation bug. When constructing a block predicate, the type of the block is this:

(AnyObject, [String: AnyObject]?) -> Bool

However, the type signature of evaluateObject(_:substitutionVariables:) is

(AnyObject?, [String: AnyObject]?) -> Bool

Note the optional AnyObject here. In Xcode 7.2 with swift 2.1, the following code causes an EXC_BAD_ACCESS signal when calling evaluateWithObject: in a playground:

let pred = NSPredicate(block: { (obj: AnyObject, bindings: [String: AnyObject]?) -> Bool in
    print(obj)
    return false
})
print(pred.evaluateWithObject(nil))

because obj is in fact optional here, but the type of the block does not allow for this.

There are two possible approaches here; removing the optional type from evaluateWithObject, or adding it to the block constructor for NSPredicate. Such a change is also presumably trivial to port back to darwin foundation, as that at minimum would need to merely change nullability annotations for these components of NSPredicate. These involve a public-api change which by my understanding needs to go through the swift evolution process.

Before sending this over to swift-evolution which is already pretty high-traffic, I wanted to float this here to make sure that this is appropriate for that process. Is it enough to draft a proposal outright or for comprehensiveness sake should I also send this out to that list to open discussion first?

Is there anyone on this list that has an opinion over which approach to take for changing the api here?

Thanks!

--
Kevin Lundberg
kevin@klundberg.com <mailto:kevin@klundberg.com>

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


(Philippe Hausler) #4

As a general rule of thumb we :heart:️radars. In this case I have already filed a bug with the component owner since it clearly looks like this is perhaps a incorrectly annotated API. So a PR for this is quite reasonable to change to ? since it would be a bit difficult to implement else wise.

···

On Dec 14, 2015, at 10:22 AM, Kevin Lundberg <kevin@klundberg.com> wrote:

Great, should I file a radar on bugreport.apple.com <http://bugreport.apple.com/> for this? And if it’s simple enough to fix that can I apply the same fix to the SwiftFoundation API as a PR without any extra process?

--
Kevin Lundberg
kevin@klundberg.com <mailto:kevin@klundberg.com>

On Dec 14, 2015, at 1:09 PM, Philippe Hausler <phausler@apple.com <mailto:phausler@apple.com>> wrote:

This might actually just be a bug in our annotations of what is nullable and what is not. I would have to double check but it seems pretty reasonable that it should have been nullable to begin with.

On Dec 14, 2015, at 10:06 AM, Kevin Lundberg via swift-corelibs-dev <swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>> wrote:

I have a pending pull request that needs a little more work around NSPredicates, but in my testing on darwin foundation, I’ve discovered what appears to be an obj-c nullability annotation bug. When constructing a block predicate, the type of the block is this:

(AnyObject, [String: AnyObject]?) -> Bool

However, the type signature of evaluateObject(_:substitutionVariables:) is

(AnyObject?, [String: AnyObject]?) -> Bool

Note the optional AnyObject here. In Xcode 7.2 with swift 2.1, the following code causes an EXC_BAD_ACCESS signal when calling evaluateWithObject: in a playground:

let pred = NSPredicate(block: { (obj: AnyObject, bindings: [String: AnyObject]?) -> Bool in
    print(obj)
    return false
})
print(pred.evaluateWithObject(nil))

because obj is in fact optional here, but the type of the block does not allow for this.

There are two possible approaches here; removing the optional type from evaluateWithObject, or adding it to the block constructor for NSPredicate. Such a change is also presumably trivial to port back to darwin foundation, as that at minimum would need to merely change nullability annotations for these components of NSPredicate. These involve a public-api change which by my understanding needs to go through the swift evolution process.

Before sending this over to swift-evolution which is already pretty high-traffic, I wanted to float this here to make sure that this is appropriate for that process. Is it enough to draft a proposal outright or for comprehensiveness sake should I also send this out to that list to open discussion first?

Is there anyone on this list that has an opinion over which approach to take for changing the api here?

Thanks!

--
Kevin Lundberg
kevin@klundberg.com <mailto:kevin@klundberg.com>

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev