[PATCH] D18268: [Objective-c] Fix a crash in WeakObjectProfileTy::getBaseInfo
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 21 12:36:49 PDT 2016
Jordan,
Does the attached patch look OK?
> On Mar 18, 2016, at 1:19 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> No, that case worked already. The case you fixed is the one where Base is 'foo' and Property is 'prop'…and actually, thinking more about it, this should not be considered "exact". *sigh* The point of "exact" is "if you see this Base and Property again, are you sure it's really the same Base?". I thought the answer was yes because the receiver is a class and the property identifies the class, but unfortunately it could be a subclass (i.e. "NSResponder.classProp.prop" vs. "NSView.classProp.prop"). These might use the same declaration (and even definition) for 'classProp' but nonetheless return different values.
>
> We could ignore this whole thing if we stored an arbitrary-length key, but there's diminishing returns there and this is already not a cheap check.
>
> Please change it to set IsExact to false (and update the table).
>
> Jordan
>
>
>> On Mar 18, 2016, at 12:21 , Akira Hatanaka <ahatanaka at apple.com <mailto:ahatanaka at apple.com>> wrote:
>>
>> Thanks Jordan. I’ve committed the patch in r263818.
>>
>> I didn’t understand your comment on WeakObjectProfileTy’s table (I’m assuming you are talking about the table in ScopeInfo.h:183). It looks like the entry MyClass.prop in the table already covers the case this patch fixed (in the test case I added, Base is NSBundle and Property is the method “foo”)?
>>
>>> On Mar 18, 2016, at 9:55 AM, Jordan Rose via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>>
>>> jordan_rose accepted this revision.
>>> jordan_rose added a comment.
>>> This revision is now accepted and ready to land.
>>>
>>> Ah, of course! Thanks for catching this, Akira. Can you add this case to the table in the doc comment for WeakObjectProfileTy? (That's how I convinced myself it was correct.)
>>>
>>>
>>> http://reviews.llvm.org/D18268 <http://reviews.llvm.org/D18268>
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160321/7d443747/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p1.patch
Type: application/octet-stream
Size: 1931 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160321/7d443747/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160321/7d443747/attachment-0003.html>
More information about the cfe-commits
mailing list