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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to