MaskRay added a subscriber: compnerd.
MaskRay added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:2655
+
+def Retain : InheritableAttr {
+ let Spellings = [GCC<"retain">];
----------------
rjmccall wrote:
> MaskRay wrote:
> > aaron.ballman wrote:
> > > MaskRay wrote:
> > > > 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.
> > > If `retain` without `used` is an expected usage pattern, then I think we
> > > need a diagnostic when we ignore the `retain` attribute. I don't think it
> > > is reasonable to expect users to read the documentation because they
> > > won't know that they've misused the attribute when we silently ignore it.
> > >
> > > Alternatively, would it be possible to make `retain` useful on all
> > > targets? e.g., when `retain` is used by itself on a declaration compiled
> > > for macOS or Windows, the backend does whatever it would normally do for
> > > `used`?
> > There is the normal behavior: `__attribute__((retain)) static void f1() {}
> > // expected-warning {{unused function 'f1'}}`.
> >
> > Sema.cpp:1227 has the unused diagnostics. There are already many different
> > versions of diagnostics. Do you suggest another diagnostic line for `used
> > is needed`? If you think so, I'll need to figure out how to do that...
> >
> > Added some tests leveraging the existing diagnostic code.
> We could definitely support retain-without-used with the ELF semantics on all
> targets if we think they're useful.
This is the case where LangRef definition reflects the state on Mach-O but not
on ELF (and not on PE-COFF before 2018-01).
https://reviews.llvm.org/rG99f479abcf2c9b36daad04eb91cd0aafa659bb1d (CC
@compnerd) (2018-01) is the commit. We could let `!retain` (if we choose to use
`!retain` as the representation) lower to similar `/INCLUDE:` directives in the
`.drectve` section.
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
