Implement NSArray index(es)Of* and enumerateObjectsAtIndexes methods #2236
Conversation
Frameworks/Foundation/NSArray.mm
Outdated
| @@ -965,33 +941,44 @@ - (NSString*)descriptionWithLocale:(id)locale indent:(NSUInteger)level { | |||
| } | |||
|
|
|||
| /** | |||
| @Status Stub | |||
| @Status Interoperable | |||
| @Notes | |||
There was a problem hiding this comment.
nit: remove empty notes. <ESC>:g/@Notes$/d
Frameworks/Foundation/NSArray.mm
Outdated
| return StubReturn(); | ||
| if (range.location + range.length > [self count]) { | ||
| [NSException raise:NSRangeException | ||
| format:@"Range {%d, %d} larger than array of size %d.", range.location, range.length, [self count]]; |
There was a problem hiding this comment.
This doesn't match the other exception formats NSArray uses
There was a problem hiding this comment.
Reference platform throws an NSRangeException for invalid ranges, should I opt for consistency here?
There was a problem hiding this comment.
Sorry, I meant the wording was inconsistent :)
There was a problem hiding this comment.
Updated to reflect exception generated on reference platform
Frameworks/Foundation/NSArray.mm
Outdated
| format:@"Range {%d, %d} larger than array of size %d.", range.location, range.length, [self count]]; | ||
| } | ||
|
|
||
| NSUInteger end = NSMaxRange(range); |
There was a problem hiding this comment.
Move this above 1035 and use end in the if(...)
Frameworks/Foundation/NSArray.mm
Outdated
|
|
||
| NSUInteger end = NSMaxRange(range); | ||
| for (NSUInteger i = range.location; i < end; ++i) { | ||
| if ([self objectAtIndex:i] == anObject) { |
There was a problem hiding this comment.
This is curious; if you prefer getObjects:range:, a "fast" NSArray implementation can turn that into an n-wide buffer copy into your provided buffer. Right now, _NSCFArray doesn't do that... but it could!
Arguably, it should, for optimization purposes. It would call CFArrayGetValues under the hood.
There was a problem hiding this comment.
This can apply anywhere you iterate manually over a range.
There was a problem hiding this comment.
should I make that part of these changes? I'm wary of changing too much in NSArray for possibly unnecessary optimization
Frameworks/Foundation/NSArray.mm
Outdated
| @Notes | ||
| */ | ||
| - (NSUInteger)indexOfObjectAtIndexes:(NSIndexSet*)indexSet | ||
| options:(NSEnumerationOptions)opts | ||
| passingTest:(BOOL (^)(id, NSUInteger, BOOL*))predicate { | ||
| UNIMPLEMENTED(); | ||
| return StubReturn(); | ||
| NSIndexSet* matching = [self indexesOfObjectsAtIndexes:indexSet options:opts passingTest:predicate]; |
There was a problem hiding this comment.
Optimization:
If you add another level of block here, you can stop enumeration on the first hit instead of continuing to aggregate every index and then discarding the rest of them. #ByDesign
There was a problem hiding this comment.
Reference platform will blow up on invalid ranges, even if a valid element is found, and this gives us that.
There was a problem hiding this comment.
I fail to see what that has to do with this; using *stop = YES; to abort the iteration inside the block still has us calling indexesOfObjects... and getting the blowing-up behaviour.
Frameworks/Foundation/NSArray.mm
Outdated
| @@ -1031,8 +1031,10 @@ - (NSUInteger)indexOfObjectIdenticalTo:(id)anObject inRange:(NSRange)range { | |||
| [self count]]; | |||
| } | |||
|
|
|||
| id objects[range.length]; | |||
| [self getObjects:objects range:range]; | |||
| for (NSUInteger i = range.location; i < end; ++i) { | |||
There was a problem hiding this comment.
nit: why not i=0, < range.length?
There was a problem hiding this comment.
Considered it but this was the path of least change, didn't think it made a substantial difference in readability or perf
Frameworks/Foundation/NSArray.mm
Outdated
| NSUInteger end = NSMaxRange(range); | ||
| if (end > [self count]) { | ||
| [NSException raise:NSRangeException | ||
| format:@"[%s %s]: range {%d, %d} extends beyond bounds [ 0 .. %d]", |
Frameworks/Foundation/NSArray.mm
Outdated
| @Notes | ||
| */ | ||
| - (NSUInteger)indexOfObjectAtIndexes:(NSIndexSet*)indexSet | ||
| options:(NSEnumerationOptions)opts | ||
| passingTest:(BOOL (^)(id, NSUInteger, BOOL*))predicate { | ||
| UNIMPLEMENTED(); | ||
| return StubReturn(); | ||
| NSIndexSet* matching = [self indexesOfObjectsAtIndexes:indexSet options:opts passingTest:predicate]; |
There was a problem hiding this comment.
I fail to see what that has to do with this; using *stop = YES; to abort the iteration inside the block still has us calling indexesOfObjects... and getting the blowing-up behaviour.
fixes #2070
fixes #2072