Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use C11 _Generic instead of void * in public API #13

Open
emersion opened this issue Sep 18, 2024 · 6 comments
Open

Use C11 _Generic instead of void * in public API #13

emersion opened this issue Sep 18, 2024 · 6 comments

Comments

@emersion
Copy link

emersion commented Sep 18, 2024

The public API allows the same functions to be used on different kinds of objects. This makes it pretty error-prone: it's easy to call a function on an invalid type without any compile-time error (either an aml type not accepted by the function in which case it's a runtime error, or a completely foreign type in which case it's just fireworks). C11 _Generic could be used to indicate which types are accepted by a function, e.g:

int _aml_get_fd(const void* obj);

#define aml_get_fd(obj) _Generic(obj, \
    const struct aml*: _aml_get_fd, \
    const struct aml_handler*: _aml_get_fd, \
)(obj)

(Note that in general I am not a fan of functions accepting multiple types of arguments, and would just recommend duplicating the functions into multiple wrappers.)

@any1
Copy link
Owner

any1 commented Sep 19, 2024

This would probably by an improvement. It could even be done without breaking ABI.

Another approach that I've considered is to approximate type traits via struct members. The interface would still be partially opaque, but the first members for the structs would be exposed. For example:

struct aml_handler {
    struct aml_obj base;
    struct aml_trait_startable startable;
    struct aml_trait_pollable  pollable;
};

int aml_get_fd(struct aml_pollable* obj);

Then you could do...

struct aml_handler handler;
struct aml aml;

...

int handler_fd = aml_get_fd(&handler->pollable);
int aml_fd = aml_get_fd(&aml->pollable);

aml_start(aml_get_default(), &handler->startable);

At least, this would be an interesting experiment in interface design.

@emersion
Copy link
Author

It could even be done without breaking ABI.

Technically speaking using _Generic would be an API break, for instance this would fail compilation after the change:

struct aml_handler *handler = ...;
void *obj = handler;
return aml_get_fd(obj);

Though probably nobody does this in practice.

approximate type traits via struct members

This would be better. I can see these potential downsides:

  • The struct aml_handler struct definition needs to be public. If you want private fields, you need to re-define a private struct wrapping the public one, and cast.
  • There is no way to fish back a struct aml_obj base from struct aml_trait_startable or struct aml_trait_pollable. Maybe that'd be fine for simple getters, but probably wouldn't work for more involved functions. Could be worked around by storing a pointer to the struct aml_obj base inside the "trait" structs perhaps.

@any1
Copy link
Owner

any1 commented Sep 19, 2024

Technically speaking using _Generic would be an API break, for instance this would fail compilation after the change

I don't mind breaking API as much as ABI.

That being said, I've been wanting to redesign the interface for a while anyway based on some observations that I've made:

  • Passing ownership of userdata to an aml object is almost always the wrong thing to do. What's in userdata should own the aml object, not the other way around. This means that those cleanup functions aren't really needed.
  • Reference counting is needed to be able to safely pass objects between threads, but this is generally not done outside the library itself, so it doesn't need to be exposed in the API.
  • Both of the above features were added as convenience for writing C++ wrappers, but I haven't used this in C++ yet and I don't really intend to.
  • The worker interface is counter-productive. Code that utilises it very often ends up becoming more complicated and error prone than having to deal with multi-threading in the first place.

If/when I do redesign it, I think I'll just do the right thing and duplicate those functions across different types. I did it this way originally because of laziness and a sudden urge to question conventional wisdom.

@emersion
Copy link
Author

I agree with all of the points above.

@any1
Copy link
Owner

any1 commented Sep 27, 2024

I haven't removed the superfluous things from the API, but at least type safety is addressed: #14

@emersion
Copy link
Author

Very nice, thanks for that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants