Introduce duplication function for Objective-C objects #143
Conversation
| // | ||
|
|
||
| #if __has_feature(objc_arc) | ||
| #error CHLDuplicateDetection.m expects ARC to be disabled. |
There was a problem hiding this comment.
This file doesn't need ARC disabled. And actually, it needs ARC enabled because there are no -release calls, so the objects allocated in here are leaking.
There was a problem hiding this comment.
Ah you are right, in the previous version I used MRR intentionally but end-solution definitely does not need it. I'll clean it up
There was a problem hiding this comment.
Ok I tested again, and I think it should be MRR after all. Reason for that is that object iterator will return anything that is an object, but cannot be retained. Example:
*** -[NSTaggedPointerStringCStringContainer retain] called, not supposed to happen
| NSMutableArray *objects = [NSMutableArray new]; | ||
|
|
||
| CHLEnumerateObjectsWithBlock(^(id object) { | ||
| if (object_getClass(object) == cls) { |
There was a problem hiding this comment.
I wonder if we should use [object isKindOfClass:cls] here? It will be pretty common to look for duplicates of NSString, NSArray, NSDictionary, etc, but those are all implemented as class clusters and as such are essentially abstract classes and no instances will be found. These collection objects will actually be internal subclasses, and I we definitely don't want to force callers to refer to internal subclasses for many reasons.
On the flip side, there might be cases where the caller wants to find duplicates based on exact class matching, not subclasses. I suspect this will be the less common case, so I would be fine with not supporting this case right now, and dealing with it later.
There was a problem hiding this comment.
I'm fine with both. I think using isKindOfClass for now might actually be better, since we might want to look at subclasses right away. Let me change that.
| return; | ||
| } | ||
|
|
||
| if (range.size > class_getInstanceSize(class_) && object_getIndexedIvars(object) == NULL) { |
There was a problem hiding this comment.
This might not work, it will have to be tested or double checked. My question is around the nature of the range values returned by the malloc system. Does range.size indicate the class instance size, or does it indicate the full size of the block of memory that malloc returned.
This commit introduces very basic duplicate detection for Objective-C objects.