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

SBCodepointSequence: Mark buffer as const #32

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

Conversation

glebm
Copy link
Contributor

@glebm glebm commented Feb 2, 2025

The functions never modify the buffer.
Marking it as const avoids the need for const_cast in C++ (as seen in https://github.com/diasurgical/devilutionX/pull/7698/files#diff-34f5922088a0ad010a634e20bd2e1497d6bf759621eec0e03322f99a8022b06e)

The const qualifier was introduced in C89.

The functions never modify the buffer.
Marking it as const avoids the need for `const_cast` in C++.
@mta452
Copy link
Member

mta452 commented Feb 8, 2025

I had also considered to mark it as const at one point but I don't remember exactly why I didn't go for it. I think it cased some compatibility issues with C and C++. I'll have to look into it again while making sure that it does not break the contract for existing users. Otherwise, there will be a need to publish a major version of the library which isn't justified for this single change only.

@glebm
Copy link
Contributor Author

glebm commented Feb 8, 2025

I'll have to look into it again while making sure that it does not break the contract for existing users

A non-const can used everywhere where const is expected as a field, so this change should be source-compatible and ABI-compatible with all existing code. I'm guessing you have some projects that use it in ways not covered by existing tests? Thanks for looking into it and testing.

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.

2 participants