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

Fix pure functions #112

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix pure functions #112

wants to merge 2 commits into from

Conversation

lucic71
Copy link

@lucic71 lucic71 commented Sep 9, 2023

This drops utf8_pure where the functions do non-pure calls and adds utf8_pure to functions that are not already marked pure.

* utf8casecmp is calling utf8codepoint which is non-pure
* utf8chr is calling utf8str which calls utf8codepoint
* utf8ncasecmp is calling utf8codepoint
* utf8str calls utf8codepoint
* utf8casestr calls utf8codepoint
@@ -104,15 +104,15 @@ typedef char utf8_int8_t;

/* Return less than 0, 0, greater than 0 if src1 < src2, src1 == src2, src1 >
* src2 respectively, case insensitive. */
utf8_constexpr14 utf8_nonnull utf8_pure int
utf8_constexpr14 utf8_nonnull int
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry colour me dumb here - why is this not pure?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's calling utf8codepoint which is a non-pure function (utf8codepoint is writing to the pointer it receives as a parameter). This results in undefined behavior which is not very well handled by clang.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's annoying. I might just change that instead!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the definition of utf8codepoint? I think that would require something like:

utf8_constexpr14_impl struct { utf8_int8_t *str; utf8_int32_t out_codepoint; }
utf8codepoint(const utf8_int8_t *utf8_restrict str) 

so that the function becomes pure.

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.

None yet

2 participants