[PATCH] D18268: [Objective-c] Fix a crash in WeakObjectProfileTy::getBaseInfo

[Akira Hatanaka via cfe-commits]

Thanks Jordan. r264025.

> On Mar 21, 2016, at 1:08 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
> Yes, that looks good. For bonus points, add a similar test using the new property syntax
> 
> @property (class) NSBundle *foo2;
> 
> instead of the method. (I expect that version to behave nearly the same, including the "may" in the diagnostic.)
> 
> Jordan
> 
> 
>> On Mar 21, 2016, at 12:36, Akira Hatanaka <ahatanaka at apple.com <mailto:ahatanaka at apple.com>> wrote:
>> 
>> Jordan,
>> 
>> Does the attached patch look OK?
>> 
>>> On Mar 18, 2016, at 1:19 PM, Jordan Rose <jordan_rose at apple.com <mailto: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 <mailto:cfe-commits at lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>>>> 
>>> 
>> <p1.patch>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160321/da4950d2/attachment-0001.html>