[clang] Clang tooling generated visibility macros for Clang (PR #109702)
Saleem Abdulrasool via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 25 12:48:47 PST 2024
compnerd wrote:
> > The default behaviour of `ids` is to assume that anything declared in the header is public and it will ensure that anything added in the headers is annotated properly to say it is exposed, making something private would be an explicit decision that the developer need to take.
>
> Is this the correct default though? It seems to me that usual best practices is to limit the visibility unless there's an opt in saying something is public (aka, the `class` model in C++ rather than the `struct` model which is rooted in C compatibility). I would have thought we'd do an initial pass to make everything public that needs to be public, and then require explicit opt-in rather than explicit opt-out, mostly because anything we list as being opted in is something someone, somewhere is likely to take as a promise that we'll always support it because it's part of the public interface.
I can absolutely see the value in the opposite - minimise the ABI surface. For many projects, there is a split of internal and external headers. The idea is that this tool is only run on the external headers, so those are meant to be ABI.
While the idea of making everything private and requiring opt-in to make it public is enticing, I don't see how to verify that the ABI surface is correct/complete. If we were to do that, then the default behaviour would require iteration until the minimal bounds is determined which would be frustrating for the developer as you might require O(function) iterations. On the other hand, it does help with ABI stability because additions to the ABI surface are okay, removal is not permitted for stability (Hyrum's Law).
> From what I'm seeing on the PR, it sounds like most of the outstanding concerns are around how to keep these annotations up to date. There have been a few requests for something like precommit CI or a github action. I think we should try to come to a decision on what we feel comfortable with there. (I tend to lean on the side of a github action that's run once a day as suggested by @tstellar
I agree that a GHA workflow would work for this. However, if/when we are able to get a Windows DLL build stood up, that would also likely be less interesting as that would immediately catch any regressions. Furthermore, I expect that once this work is fully complete, we should be able to switch to `-fvisbility=hidden -fvisbility-inlines-hidden` which would also provide a pretty good overlap on Linux and macOS builds. The remaining uncaught differences would be platform specific due to ABI specific constraints (e.g. RTTI representation and vtable layouts).
https://github.com/llvm/llvm-project/pull/109702
More information about the cfe-commits
mailing list