const qualifiers in pubic API
martinjrobins opened this issue · 7 comments
Is your feature request related to a problem? Please describe.
I'm writing a rust sys crate that uses bindgen to automatically generate rust bindings for suitesparse. At the moment when wrapping a function like klu_analyze
in the KLU solver, raw pointers are wrapped as mutable pointers *mut T
, e.g: the C function
KLU_symbolic *KLU_analyze /* returns NULL if error, or a valid
KLU_symbolic object if successful */
(
/* inputs, not modified */
Int n, /* A is n-by-n */
Int Ap [ ], /* size n+1, column pointers */
Int Ai [ ], /* size nz, row indices */
/* -------------------- */
KLU_common *Common
)
becomes
extern "C" {
pub fn klu_analyze(
n: i32,
Ap: *mut i32,
Ai: *mut i32,
Common: *mut klu_common_,
) -> *mut klu_symbolic_;
}
This is an issue because the arguments Ap
and Ai
are incorrectly marked as mutable, causing problems downstream for users who are then required to have mutable handles to Ap
and Ai
. This can be corrected by marking the arguments Ap
and Ai
in the C signature as const
, for example:
KLU_symbolic *KLU_analyze /* returns NULL if error, or a valid
KLU_symbolic object if successful */
(
/* inputs, not modified */
Int n, /* A is n-by-n */
Int Ap [const], /* size n+1, column pointers */
Int Ai [const], /* size nz, row indices */
/* -------------------- */
KLU_common *Common
)
which would then generate the correct rust function signature with Ap
and Ai
marked as `const.
Are there any plans to explicitly use const
qualifiers in suitesparse rather than relying on comments like this? Would you be happy if I put together a PR adding this to the functions that I need?
Describe the solution you'd like
Use explicit const
qualifiers in public facing functions.
Describe alternatives you've considered
The alternative is for the bindings for suitesparse be manually implemented, rather than relying on automated tools.
I agree with your point. It would probably be best if function arguments that are pointers (or arrays) to variables that aren't allowed to be changed by the function were marked as const
.
However, are you sure that Int Ap[const]
is valid C syntax? Shouldn't it be const Int Ap[]
instead?
I believe you can do Int Ap[const]
since C99 (https://en.cppreference.com/w/c/language/const#:~:text=In%20a%20function%20declaration%2C%20the,the%20array%20type%20is%20transformed.), but I'm not sure as this is the first time I've come across this syntax. Presumably const Int Ap[]
is equivilent?
UPDATE: actually, after looking at https://en.cppreference.com/w/c/language/array I think const Int Ap[]
is a pointer to an array of const Int
values, whereas Int Ap[const]
is a const pointer to an array of Int
values, so I think you are correct it should be const Int Ap[]
, or perhaps even const Int Ap[const]
.
This will take me quite a while to resolve, since I would need to revise all APIs to essentially all my packages. It might also require an SO change to all my packages as well. But I'll consider it for the future.