Patch is OK. It is safe for us to be checked in. - Thanks, Fariborz
On May 7, 2013, at 1:48 PM, Nico Weber <[email protected]> wrote: > On Tue, May 7, 2013 at 1:39 PM, jahanian <[email protected]> wrote: > > On May 7, 2013, at 1:20 PM, Nico Weber <[email protected]> wrote: > >> Thanks for the review! >> >> >> On Tue, May 7, 2013 at 1:12 PM, jahanian <[email protected]> wrote: >> Patch is OK. But I am concerned about producing ABI incompatibility with >> older clangs. >> >> That's a valid concern. It _does_ restore ABI compatibility with gcc; some >> code in Chromium apparently relied on the original behavior. >> >> Also, similar problem exists >> with the ‘copy’ attribute (and probably others I haven’t tried). >> >> 'copy' and 'retain' can't be used together as far as I know. > > Sorry for not being clear. I meant when attribute is ‘copy’ (instead of > ‘retain’). > > Ah, good point. Tweaked patch attached. > > > - Fariborz > >> >> Let me run this by our runtime guys to see its impact. >> >> Thanks! >> >> >> - Fariborz >> >> On May 7, 2013, at 12:25 PM, Nico Weber <[email protected]> wrote: >> >>> Hi, >>> >>> clang omits the '&' for retained properties if the property is also >>> readonly. The attached patch fixes this – it makes clang match gcc's >>> behavior and fixes PR15928. >>> >>> Ok? >>> >>> Nico >>> <clang-retain.patch>_______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >> > > > <clang-retain.patch>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
