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
Functions don't use const when possible #83
Comments
Hi @kaadmy Do you use C++? Yes we could make readonly parameters 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 |
I'm currently using C99. Only parameters that aren't written to need to be |
I want to use /* 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 |
@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. |
Fair enough. I think we need to do something like this instead of using 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);
@heapseeker thank you, I may wait for you if you want to work on this? |
@heapseeker ping. |
Any feedbacks are welcome at #86 |
Now readonly parameters are marked as |
After marked readonly params as 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 Any feedbacks would be helpful |
In my experience, 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 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. |
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? |
@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 However, when you mark array api's params as const like 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 So if we can implement const parameters properly, then we should do that |
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. |
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 @Daniel-Schreiber-Mendes how do you define const types ? Like @hartenfels what if we define const types like #ifndef GLM_ENABLE_CONST
# define mat4_const mat4
#else
/* real definition for mat4_const and other types */
#endif Using If we can’t find any good solution than we can close this issue 😔 |
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. |
I'll take a look at this asap I hope, feedbacks are always welcome of course. |
@recp Your idea for adding the |
@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: |
@sacereda Yes please!!! That would be a godsend. |
@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 Struct api is also continues to evolve: #290 for who want struct api as const-aware cases |
FYI: C23 will fix the issues highlighted by recp
|
@duarm thanks, that's great! |
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 asconst
. 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 asconst
?The text was updated successfully, but these errors were encountered: