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

Reply via email to