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 <ahatan...@apple.com> wrote: > > Jordan, > > Does the attached patch look OK? > >> On Mar 18, 2016, at 1:19 PM, Jordan Rose <jordan_r...@apple.com >> <mailto:jordan_r...@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 <ahatan...@apple.com >>> <mailto:ahatan...@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@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> > <p1.patch>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
