[PATCH] D109192: [WIP/DNM] Support: introduce public API annotation support

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 3 08:18:24 PDT 2021


compnerd added a comment.

In D109192#2981682 <https://reviews.llvm.org/D109192#2981682>, @fodinabor wrote:

> Thanks for working on this!
> In general looks good to me, I'd opt for all CAPS macros, though, so e.g. `LLVM_SUPPORT_API`. The return variant `LLVM_SUPPORT_API(retTy)` is only beneficial when needing something like `extern "C" retTy __stdcall` iirc and is less ideal for marking classes as exported.

Sure, I can switch it to `LLVM_SUPPORT_API` just as well.  That is the easy part of the change.  The difficult part is the auditing of the interfaces.

Correct, the return variant is useful for the extra decoration.  While not particularly useful for the classes, it does allow writing the decorations more naturally when expanded:

`class __declspec(dllexport) exported { };` on Windows
`__attribute__((__visibility__("default"))) class exported { };` on other platforms

The places where it **is** however useful is is on variable and function declarations.  Exported functions really should be decorated with `extern "C"`, but that is something that I don't think we should conflate with the work here.  The goal here is to minimally attribute enough of LLVM with dll interface/visibility and switch to hidden visibility on non-Windows platforms while enabling a build where the core of LLVM can be a shared library.

> A parameterized version `LLVM_API(LLVMSupport)` would be fine for me as well, although I don't see any benefit.

I agree, it isn't particularly useful (and it is more complicated).  That was a suggestion to @tschuett's comment that we aim for a single macro rather than a library specific macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109192



More information about the llvm-commits mailing list