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

Alternative C API #189

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Alternative C API #189

wants to merge 3 commits into from

Conversation

emmt
Copy link

@emmt emmt commented Apr 9, 2024

This PR implements the alternative C API discussed here and here. The changes come in two commits:

  • the first commit (0aa2069) is a simple modification of the existing C API so that all structures are passed by address (not by value);

  • the second commit (d90d0a3) implements a draft of the proposed alternative C API with:

    • a single opaque structure prima_context_t storing all the parameters,
    • a couple of functions (prima_context_create and prima_context_destroy to create this structure and, when no longer needed, destroy it as well as related resources),
    • a number of functions (mutators prima_context_set_... and accessors prima_context_get_...) to set and retrieve all parameters.

All C examples compile and run fine with the first commit. Example are needed for the 2nd commit but it is mainly there for being discussed.

The two king of API can coexist. In fact the latter one is implemented on top of the former one and is intended to facilitate the writing of bindings in other languages without requiring to directly access C structures in these languages.

@emmt
Copy link
Author

emmt commented Apr 9, 2024

Silly me, I forgot to change the tests too (done in 7b35c05). I should have made 2 different PRs (one for passing structures by address, the other for the alternative API) sorry for that.

@zaikunzhang
Copy link
Member

Thank you @emmt !

int prima_minimize(const prima_algorithm_t algorithm, const prima_problem_t *problem, const prima_options_t *options, prima_result_t *const result);

Why do we use pointers here for problem and options?

@emmt
Copy link
Author

emmt commented Apr 9, 2024

Why do we use pointers here for problem and options?

Because passing them by value is not necessary and is a waste of stack space. It is possible but rather unusual in C to pass structures by value (except maybe very small ones). The pointers of these are const so their contents is not supposed to be modified.

@zaikunzhang
Copy link
Member

zaikunzhang commented Apr 9, 2024

Then may I suggest

int prima_minimize(const prima_algorithm_t algorithm, const prima_problem_t *const problem, const prima_options_t *const options, prima_result_t *const result);

as neither the pointer itself nor its content is supposed to be changed in the function body?

Of course, any change to the pointer itself will be lost since itself is passed by value, but this will effectively prevent the developers (ourselves, future ourselves, and others) from making mistakes when composing or modifying the implementation of this function. I had a discussion with @amontoison about it and this was the consensus.

Thank you.

@emmt
Copy link
Author

emmt commented Apr 15, 2024

Honestly, the const qualifier in function prototypes is only useful for arrays and pointers to indicate that the corresponding content is not modified by the called function. Otherwise, it indicates that the value of the argument (not the content at that address if it is an array or a pointer) is not modified which is irrelevant for the caller and may only be relevant for the optimizer. Modern optimizers can infer this themselves and const is then just a nuisance for the caller when he/she attempt to decode the meaning of the function prototype. To add to the confusion, the const qualifier can be before or after the type. Hence, to simplify the task of understanding the function prototype for the user, I usually restrict the use of const to the former case. But I'll adapt to the consensus...

@amontoison
Copy link
Member

amontoison commented Apr 24, 2024

Why do we use pointers here for problem and options?

Because passing them by value is not necessary and is a waste of stack space. It is possible but rather unusual in C to pass structures by value (except maybe very small ones). The pointers of these are const so their contents is not supposed to be modified.

Thanks for the explanation with stack space!
I understand now why it's more relevant to pass structures by pointer.

For the const, we should think of a compromise between preventing he developers from making mistakes but also the readability of the headers.
@emmt is more expert than me on C, we should probably follow his advice.

For the new C API, it looks good to me. 👍

@zaikunzhang
Copy link
Member

To add to the confusion, the const qualifier can be before or after the type. Hence, to simplify the task of understanding the function prototype for the user, I usually restrict the use of const to the former case.

Then a user like me would need to check a documentation to make sure what the const really means, because, as you said, there are two different cases. If we use both, the user knows automatically that everything is supposed to be constant.

@emmt
Copy link
Author

emmt commented Apr 24, 2024

@zaikunzhang Except that when there is a single const for a pointer the user has to wonder whether it is the value of the pointer that remains constant (which is irrelevant for the user) or the values at the address given by the pointer that are left unchanged (which is of much importance for the caller). If you look at the prototypes of the functions of the C standard library, they follow what I suggest: reserve the const qualifier to indicate that a pointer argument is for read-only access to the pointed memory (without const, read-write access has to be assumed). You can check that this is also the case of many well known C library. This usage of the const qualifier is simpler to interpret and more clear.

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

Successfully merging this pull request may close these issues.

3 participants