MaskRay added inline comments.
================ Comment at: clang/include/clang/Basic/Attr.td:2655 + +def Retain : InheritableAttr { + let Spellings = [GCC<"retain">]; ---------------- aaron.ballman wrote: > MaskRay wrote: > > aaron.ballman wrote: > > > Should this be a target-specific attribute as it only has effects on ELF > > > targets? > > As I understand it, GCC `retain` is not warned on unsupported targets. > > > > Regardless of GCC's choice, I think not having a `warning: unknown > > attribute 'retain' ignored [-Wunknown-attributes]` diagnostic makes sense. > > `retain` will be usually used together with `used`. In Clang, `used` > > already has "retain" semantics on macOS and Windows (I don't know what they > > do on GCC; GCC folks want orthogonality for ELF and I agree). If `retain` > > is silently ignored on macOS and Windows, then users don't need to > > condition compile for different targets. > The other side of that is a user who writes only `retain` and expects it to > do something when it's actually being silently ignored. While they may > usually use it in conjunction with `used`, I'm more worried about the > situation where the user is possibly confused. `retain` without `used` can be used on some external linkage definitions, as well as internal linkage definitions which are referenced by live sections. I agree there could be some confusion, but hope with mccall's suggestion (thanks) the documentation is clear. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96838/new/ https://reviews.llvm.org/D96838 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits