Tooold2rock n roll/change func parameters to const

[tooold2rock-n-roll]

I tried to contain my refactoring to the include/cglm/ directory, but there were some dependencies that I had to keep digging.

Please let me know if I let anything slip o/

[recp]

There are still a lot of places where const/CGLM_IN() need to be added to keep the signatures consistent e.g. (src/) clipspace/*, quat.h, euler.h, affine-post.h, affine2d-post.h, project_zo.h, glm_mat4_ins3, glm_ivec3_norm/ norm2, glm_mat3x4_mul ..

We can use const directly on vec3 instead of CGLM_IN(vec3), CGLM_IN(T) must only be used for multi-dim arrays like matrices

also, could you squash similar commits and use clearer commit messages?

[tooold2rock-n-roll]

There are still a lot of places where const/CGLM_IN() need to be added to keep the signatures consistent

That is why I asked for a formal list of files to look at, I can't judge where is safe to change or not just by looking at it.

We can use const directly on vec3 instead of CGLM_IN(vec3), CGLM_IN(T) must only be used for multi-dim arrays like matrices

That is what I did, the CGLM_IN(vec3) you saw are vec3[2] that show up in some places, if I don´t use the macro it bugs on C11 like the matrices.
That kind of mistake tends to creep in long repetitive work though, so if you catch any specific case, just pin point it and I will fix it back.

also, could you squash similar commits and use clearer commit messages?

Didn't know too many commits where a problem, I thought breaking the commits into logical units (like all vecX files together) would make it easier for review. I will try to keep it to a minimum going forward.

Do you mean longer more descriptive messages? or the ones I wrote are just not fitting to the modifications being committed?

[recp]

That is what I did, the CGLM_IN(vec3) you saw are vec3[2]

ahh sorry, but IIRC I saw some places like:

bool
glm_aabb_point(CGLM_IN(vec3) box[2], CGLM_IN(vec3) point) {}

CGLM_IN(vec3) point plain const would be enough here

also, could you squash similar commits and use clearer commit messages?

Didn't know too many commits where a problem, I thought breaking the commits into logical units (like all vecX files together) would make it easier for review. I will try to keep it to a minimum going forward.

Do you mean longer more descriptive messages? or the ones I wrote are just not fitting to the modifications being committed?

Having many commits is not a problem and grouping them into logical units is helpful, sure.

I meant that repetitive or fixup commits should be squashed and each remaining commit should be distinct by task or scope. For instance, many commits have the generic message Changing function parameters to const, which doesn’t indicate which files were changed or what the scope was.

The messages don’t need to be longer, just more specific.

so if you catch any specific case, just pin point it and I will fix it back.

After finished I'll take a closer look, you can look at all public headers for readonly pointer params (vector/matrix...) as const candidates

thanks

[tooold2rock-n-roll]

Dude, I just tried starting /src and I was not prepared.......it broke my spirit a little!
(I was not aware of the call interface, you are telling me you have to replicate every refactoring 3x by hand for each entry? Plus tests???)

I estimate I will take at least a week to comb those files, not only it's 2x the effort, changing the correct variables is a guess game without the function's doxygen detailed comments. I have to change all the occurrences of a pattern and compile, just to discover most of them are [in/out] can't be const and go back undoing it one by one. This is serious masochism lol

I can only keep working on this till Friday, but I can look this though slowly after, just let me give you two suggestions that may be more interesting.

First, accept the changes to the heather only API (this will cover most of the problem) and open another ticket for the Call API.

Second, generating the Call API can be automated with some bash/Make shenanigans. This will be a complicated feature to implement, but at long term, it is a better solution than manually keeping 3 different interfaces updated to each other indefinitely.

What do you say?

[recp]

Hey, you’ve done a great job so far. Let’s keep your motivation high or const at least :)

Searching @param[in] may help to reveal const candidates more quickly in public headers, may not help for src/ but still hlepful

First, accept the changes to the heather only API (this will cover most of the problem) and open another ticket for the Call API.

Okay then, let’s scope this PR to the header-only array API.

We still need to apply the corresponding signature changes to the relevant include/cglm/simd/ headers, since the inline functions call those helpers directly.

We can handle the call api (include/cglm/call/ and src/) in a separate PR

but lets keep min target 202311L for safety

[tooold2rock-n-roll]

I'm not not unmotivated, if this was my project and I had to keep moving with utilitarianism, those would be just facts.
I'm a practical person :D

Searching @param[in] may help to reveal const candidates more quickly in public headers, may not help for src/ but still hlepful

Yes, that is part of the strategy so far, Vim has a lot more features for doing this kind of jobs, but my skills and time are never the less limited. Your project is almost, like 99,99%, chef's kiss, consistent whit variables names, makes it very easy after a couple of files to just batch replace every occurrence of the same patterns. The Doxygen headers (damn, I been writing that wrong this whole time) are very well written and make it easy to spot mismatches to the changes I made.

I have finished the files missing from last time, they were much easier, just didn't push to git because of our last conversation.

I will keep the current (wrong) C version till the last commit, otherwise I can't test on my computer that still uses it instead of the correct C23. I would argue that most people will have the same problem and since 202000L does not create any conflicts with previous versions, keep it that way. It will keep working as intended even when 202311L becomes default on most linux releases.

I will take a look at simd/ and return to you till the end of the day o/

[tooold2rock-n-roll]

Okkkkk went directory by directory and did what I could, some parts were not meant for my environment and I do not know the crimes I committed since it wont compile!

I stand here in shock watching the automated github tests results, apparently I made many mistakes regarding MacOS.

Anyway, I did not touch these files: arm.h, intrin.h, wasm.h, x86.h
A quick research showed me that those parameters are not arrays, instead some kind of big ass registers?
Are those passed by value or reference when used for inline functions???

I wait for your final considerations regarding the C standard value for C23 and how to proceed with those 4 files.
In the meanwhile, I will see what those errors are on the auto tools.