[PATCH] D97447: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:74
+COFF and Mach-O targets (Windows and Apple platforms), this attribute prevents
+the definition and its symbol from being removed by the linker.  On ELF
+targets, it has no effect on its own, and the linker may remove the definition
----------------
rnk wrote:
> I think it would be helpful to add more context here, since each linker has its own terminology to describe section GC. Here's my attempt at starting this paragraph:
> 
> Whether this attribute has any effect on the linker depends on the target and the linker. Most linkers support the feature of section garbage collection (`--gc-sections`), also known as "dead stripping" (`ld64 -dead_strip`) or discarding unreferenced sections (`link.exe /OPT:REF`). On COFF and Mach-O targets (Windows and Apple platforms), the ``used`` attribute prevents symbols from being removed by linker section GC. On ELF targets, ...
> 
> I'm not sure how to link to the docs for the various command line flags from rst, but I think it would be helpful.
Adopted. Thanks!


================
Comment at: clang/include/clang/Basic/AttrDocs.td:86
+This attribute, when attached to a function or variable definition, prevents
+the linker from discarding the definition.  If the compiler does not emit the
+definition, e.g. because it was not used in the translation unit or the
----------------
rnk wrote:
> The linker can discard definitions for various reasons, like comdats, weak symbols, archive semantics, etc etc. Is `SHF_GNU_RETAIN` specific to section GC? Can you say so if so?
Thanks for spotting this confusing part. I'll add archive member selection and COMDAT group resolution.

Weak symbols are not related. Weak symbols are part of symbol resolution, that does not affect the sections backing up the symbols.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97447/new/

https://reviews.llvm.org/D97447