[PATCH] D152051: libclang-cpp: Add external visibility attribute to all classes

Wed Jun 7 07:18:05 PDT 2023

tstellar added a comment.

> ! In D152051#4403167 <https://reviews.llvm.org/D152051#4403167>, @compnerd wrote:
>  Please do not use LLVM_EXTERNAL_VISIBILITY but rather introduce a new macro (this will prevent the use on Windows).

OK, so should I create a clang specific macro for this?  And should it behave the same as LLVM_EXTERNAL_VISIBILITLY or do I need to have it expand to different values depending on the OS?

> I did write a tool to help with this: https://GitHub.com/compnerd/ids

OK, thanks, I will check this out.

================
Comment at: clang/include/clang/Format/Format.h:4532
 /// Returns ``true`` if the Style has been set.
-bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language,
+LLVM_EXTERNAL_VISIBILITY bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language,
                         FormatStyle *Style);
----------------
HazardyKnusperkeks wrote:
> This doesn't look formatted.
I ran git-clang-format on the patch and it introduced a lot of unrelated changes, so I ended up dropping that part of the patch.  I can go through and manually discard some of the more intrusive format changes.  Any suggestions on which kind of formatting changes to keep and which ones to ignore?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152051