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

[stdlib] Replacing StringRef's local _memcmp with memory.memcmp for string comparisons. #2740

Closed
wants to merge 3 commits into from

Conversation

siitron
Copy link
Contributor

@siitron siitron commented May 18, 2024

Continuation of #2620.
A local _memcmp-method has previously been used when comparing strings in order to avoid indirect recursions. Unless this issue is still present then we may use memory.memcmp instead which would be preferred since the local _memcmp isn't vectorized.

Signed-off-by: Simon Hellsten <simonhellsten@hotmail.com>
@siitron siitron marked this pull request as ready for review May 18, 2024 14:52
@siitron siitron requested a review from a team as a code owner May 18, 2024 14:52
@laszlokindrat laszlokindrat self-assigned this May 20, 2024
@laszlokindrat
Copy link
Contributor

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label May 21, 2024
@laszlokindrat
Copy link
Contributor

I checked, this is breaking some internal stuff due to the recursion we discussed in #2620. Not sure how to fix it, but I think it has to do with the error printing of the constraint in memcmp. If you want you can try to refactor that to have a (potentially hidden) version that is unsafe and not constrained explicitly.

modularbot added a commit that referenced this pull request Jun 5, 2024
[External] [stdlib] `memcmp` refactor for string comparisons

This is a follow-up to #2740.

Refactored the `memcmp` implementation to give `StringRef` a
`constrained`-less version for vectorized string comparisons that avoid
an internal recursion bug.

ORIGINAL_AUTHOR=Simon Hellsten
<56205346+siitron@users.noreply.github.com>
PUBLIC_PR_LINK=#2926

Co-authored-by: Simon Hellsten <56205346+siitron@users.noreply.github.com>
Closes #2926
MODULAR_ORIG_COMMIT_REV_ID: 53e819bfd8ec6f6d85610677131edefa4207087f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported-internally Signals that a given pull request has been imported internally.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants