Functions don't use const when possible

[kaadmy]

trafficstars

Due to functions not taking arguments as const when possible (ie. when a variable is never written to), it's very tough to integrate CGLM with code that passes vector/matrix types as const. Since constant variables cannot be cast to non-const, this means any code that calls CGLM can't use constant variables.

Since it's possible to cast to const, but not from, would it be possible to mark read-only function arguments as const?

[recp]

Hi @kaadmy

Do you use C++? Yes we could make readonly parameters const and out parameter const reference in C++ maybe to allow to pass initializer list as parameter

Something like this make that possible, I think:

glm_mat4_mul(GLM_IN(mat4) a, GLM_IN(mat4) b, GLM_OUT(mat4) dest)

it is more obvious which parameter is writable in this way

#ifdef __cplusplus
#define GLM_IN(X)    const X&
#define GLM_INOUT(X)       X& 
#define GLM_OUT(X)         X&
#else
#define GLM_IN(X)    const X
#define GLM_INOUT(X)       X
#define GLM_OUT(X)         X
#endif

[kaadmy]

I'm currently using C99.

Only parameters that aren't written to need to be const (in fact, it's an error to use const otherwise), which includes nearly all functions that have a src and dest parameter.

[recp]

I want to use const X& in C++ to allow something like this:

/* C and C++ version */
glm_vec3_norm((vec3){1, 2, 3});

/* C++ version if const X& is enabled otherwise: compiler error like 'No matching function for call to...' */
glm_vec3_norm({1, 2, 3});

I think GLM_IN, GLM_OUT, GLM_INOUT macros will make C and C++ happy

[heapseeker]

@recp The issue isn't C++ or not C++. The issue is that read-only variables that aren't mutated are not marked as const, which they should be. I may make a PR and do this, as it makes my own code a bit sloppy because I can't use const.

Also I would recommend against caring about or supporting C++. Realisitically no one would use this library over GLM if you're using C++. GLM is just a more mature, stable library. Keep your focus and energy on everything C related.

[recp]

Also I would recommend against caring about or supporting C++. Realisitically no one would use this library over GLM if you're using C++. GLM is just a more mature, stable library. Keep your focus and energy on everything C related.

Fair enough.

I think we need to do something like this instead of using GLM_IN, GLM_OUT, GLM_INOUTthen:

CGLM_INLINE
void
glm_vec4_add(vec4 a, vec4 b, vec4 dest);

to

CGLM_INLINE
void
glm_vec4_add(const vec4 a, const vec4 b, vec4 dest);

I may make a PR and do this, as it makes my own code a bit sloppy because I can't use const.

@heapseeker thank you, I may wait for you if you want to work on this?

[recp]

@heapseeker ping.

[recp]

Any feedbacks are welcome at https://github.com/recp/cglm/pull/86

[recp]

Now readonly parameters are marked as const at https://github.com/recp/cglm/pull/86

cc @kaadmy @heapseeker

[recp]

After marked readonly params as const, I got these warnings at Ubuntu:

warning: pointers to arrays with different qualifiers are incompatible in ISO C [-Wpedantic]

Helpful resources:

https://stackoverflow.com/questions/34488559/pointer-to-array-with-const-qualifier-in-c-c https://stackoverflow.com/questions/50164803/pointer-to-array-not-compatible-to-a-pointer-to-const-array/50165840

Any feedbacks would be helpful

[hartenfels]

In my experience, const is a nice idea, but in the end they cause many errors like the one you ran into, where types aren't compatible for pernicious reasons. Especially when you start dealing with arrays, pointers to pointers etc., you start fighting against the compiler a lot.

However, since there's now a struct API that doesn't take pointers directly, doesn't that kinda make this requirement moot? The struct functions don't modify their parameters anyway, they take them as values and return values. So if you want to use const, you can use the struct versions, which don't have any issues with it:

const mat4s m1 = GLMS_MAT4_IDENTITY_INIT;
const mat4s m2 = glms_mat4_copy(m1); /* no warnings here  */

So basically, the struct API already takes its read-only parameters as values, rather than pointers, and therefore "solves" this, albeit in a different way.

[Daniel-Schreiber-Mendes]

What is the current state of this? I cloned the latest version but still all the functions don't take const parameters. Did you decide not to implement this? Or whats the reason for this?

[recp]

Did you decide not to implement this?

@Daniel-Schreiber-Mendes not exactly, just I couldn't make it in a proper way. As @hartenfels said, since struct api takes parameters as read-only, the struct api's parameters can be marked as const

However, when you mark array api's params as const like const mat4 m1, const mat4 m2 then you will get these warnings/errors as I mentioned above:

warning: pointers to arrays with different qualifiers are incompatible in ISO C [-Wpedantic]

Helpful resources:

https://stackoverflow.com/questions/34488559/pointer-to-array-with-const-qualifier-in-c-c https://stackoverflow.com/questions/50164803/pointer-to-array-not-compatible-to-a-pointer-to-const-array/50165840

---

So if we can implement const parameters properly, then we should do that

[hartenfels]

For the above reason, I'd vote for closing this issue. You can't change the array parameters without causing trouble. Which means it'd break everyone that's currently using this API with non-const arrays and require terrible casts to make it work (or, more likely, cause people to stop using this library because it broke their code).

The struct API solves the issue of non-constness by taking everything as a value instead of by reference. The original problem of "I want const values" is resolved: use structs, make them const.

[recp]

I’ll re-investigate this later, maybe we can find a good or cool way to make IN params CONST without breaking the existing api, for instance mat4_const or vec4 * const m1 could be used. But I think it is more readable to use param type as mat4 or mat4_const (similar for other types)

@Daniel-Schreiber-Mendes how do you define const types ? Like const mat4 or like vec4 * const? Both may not be same

@hartenfels what if we define const types like mat4_const, vec3_const ... then use these types for const params? We could provide an option to enable this feature like :

#ifndef GLM_ENABLE_CONST
#  define mat4_const mat4
#else
/* real definition for mat4_const and other types */
#endif

Using mat4_const may make client code and api more readable, maybe

If we can’t find any good solution than we can close this issue 😔

[duarm]

this optional solution proposed by recp sounds good, using the struct API is incovenient to me, writing portable C99 code. The API i'm working makes heavy use const parameters, lots of discarded qualifier warnings.

[recp]

I'll take a look at this asap I hope, feedbacks are always welcome of course.

[CaspianA1]

@recp Your idea for adding the GLM_ENABLE_CONST flag seems solid. It would be enormously helpful for a project I'm working on; I'm sure it would be very helpful for others as well.

[sacereda]

@recp I can try to upload a PR if you want. I have something like this locally, but didn't check all the available functions yet:

const.patch

[CaspianA1]

@sacereda Yes please!!! That would be a godsend.

[recp]

@CaspianA1 wow, sorry I didn't see your comment until now or forgot to respond, ( it would be nice to get a ping for these cases :) )

@sacereda sure! One thing is mat4_const vs const_mat4? We can also discuss that at PR, we can also mark this feature with experimental label

---

Struct api is also continues to evolve: https://github.com/recp/cglm/pull/290 for who want struct api as const-aware cases

[duarm]

FYI: C23 will fix the issues highlighted by recp

...
An array and its element type are always considered to be identically qualified*)
...

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2607.pdf

[recp]

@duarm thanks, that's great!