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

Constify unsigned_read in ltc_math_descriptor #654

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

levitte
Copy link
Collaborator

@levitte levitte commented Aug 20, 2024

Drop the corresponding deconstying casts in code.

Fixes #581

@levitte
Copy link
Collaborator Author

levitte commented Aug 20, 2024

There's oh so much more to do regarding constification, but a lot of it breaks against tomsfastmath, so that will need to change first.

Drop the corresponding deconstying casts in code.

Fixes libtom#581
@sjaeckel
Copy link
Member

Thanks for the PR! I also believe we should constify the API where possible.

This PR is slightly related to #174 #175 and #176

Regarding breaking tfm ... we could also constify the ltc API now and cast the const for tfm away. I checked tfm and it uses the arguments as if they were const anyway.

Maybe we should then introduce a macro instead of adding the cast explicitely? So we don't have to manually fix it again once tfm has been adapted resp. we can keep it as is, if someone has to build against an old tfm.

#if TFM_VERSION_something_something
/* oh yeah LTC_TFM_UNCOST is such a bad name */
#define LTC_TFM_UNCOST(type) (type)
#else
#define LTC_TFM_UNCOST(type)
#endif

We'd then also need to introduce TFM_VERSION_something_something in tfm, so we know what to compare against ;) I've opened libtom/tomsfastmath#36 to tackle that.

For now we could just add the "old path" in ltc and add the version compatibility once it's done in tfm.

@sjaeckel
Copy link
Member

I checked tfm and it uses the arguments as if they were const anyway.

haha, naive me :D looking at something and then doing it is sometimes very different ...

@levitte
Copy link
Collaborator Author

levitte commented Sep 16, 2024

I hate having to cast.... but your idea is sane, and I do like the TFM_VERSION_3() thing you added over in tomfastmath. So, I'm game, and will have at least a draft PR submitted fairly soon, based on TFM_VERSION_3().

@levitte
Copy link
Collaborator Author

levitte commented Sep 17, 2024

Have a look at #667, which is the full constification of the math stuff, which also subsumes this PR.

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.

src of unsigned_read should be const
2 participants