[clang] Clang tooling generated visibility macros for Clang (PR #109702)

[Aaron Ballman via cfe-commits]

AaronBallman 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. 

Well, we're currently approaching this from the angle of "expose everything and then the user can do whatever they want", but perhaps the discussion we should be having is "what use cases do we explicitly want to support?" and then we write plugins to demonstrate that we do support those use cases, exposing what's necessary as part of that process. This does mean that use cases we hadn't anticipated may be frustrating for users, but we can more easily expand the surface area of what we expose than we can claw it back once we've exposed it publicly.

But that does mean a lot more up front work on our part.

(Note, I don't insist on this, just having the discussion to see where the ideas lead.)

> 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).

Exactly; and I worry about the long-term maintenance impacts of exposing everything.

> > 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).
> 
> Note that this doesn't mean that I'm against the GHA - quite the opposite. I think that is a good solution and will help bridge us to the point where we can rely on regular iteration to identify these issues more quickly.

+1

https://github.com/llvm/llvm-project/pull/109702