swaywm / wlroots

A modular Wayland compositor library
https://gitlab.freedesktop.org/wlroots/wlroots/
MIT License
2.15k stars 343 forks source link

Hide internal details from our API #1828

Open emersion opened 5 years ago

emersion commented 5 years ago

One thing that really pushes back stabilization for me is that all our struct fields are public. This makes sense in a lot of cases because we want compositors to have a lot of freedom. However exposing too many fields could paint us in an API compatibility corner. In particular, wl_listener fields shouldn't be exposed IMHO.

Additionally, our internal fields clobber the header files and make it difficult for compositor writers to know what fields they should use (example: xdg-shell geometry fields).


wlroots has migrated to gitlab.freedesktop.org. This issue has been moved to:

https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/1828

ammen99 commented 5 years ago

What I am not sure about is how do we decide which fields are useful and which aren't? Would we just apply libweston's strategy - hide all currently unused APIs and then expose them again if needed in the future?

ddevault commented 5 years ago

I agree, but would also posit that simply having some details available via structs is not necessarily a bad thing, even if they're read-only and writing them would be considered a programming error. I agree that our internal wl_listeners are a good candidate for privatization.

One thing I don't want to do is start guaranteeing ABI stability, ever.

emersion commented 5 years ago

I agree, but would also posit that simply having some details available via structs is not necessarily a bad thing, even if they're read-only and writing them would be considered a programming error.

Yes. I'm not advocating for adding a bunch of getters and setters everywhere. :P

Would we just apply libweston's strategy - hide all currently unused APIs and then expose them again if needed in the future?

Yeah. In case of doubt, it's always easier to make a field private, and then decide to make it public later. Doing it the other way around is an API breakage.

emersion commented 3 years ago

Some other possible (but not necessarily good or bad) ideas:

#ifdef WLR_SHOW_PRIVATE
#define WLR_PRIVATE_BEGIN
#define WLR_PRIVATE_END
#else
#define WLR_PRIVATE_BEGIN char private[sizeof(struct {
#define WLR_PRIVATE_END }];
#endif