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

[Jordan Rose via cfe-commits]

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> 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
>>> 
>> 
> <p1.patch>

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