Skip to content

[stdlib] Use memcmp to dispatch to optimized implementation in StringSlice#5624

Closed
YichengDWu wants to merge 1 commit into
modular:mainfrom
YichengDWu:lt
Closed

[stdlib] Use memcmp to dispatch to optimized implementation in StringSlice#5624
YichengDWu wants to merge 1 commit into
modular:mainfrom
YichengDWu:lt

Conversation

@YichengDWu
Copy link
Copy Markdown
Contributor

This PR updates the comparison logic in StringSlice to use the public memcmp API instead of the internal _memcmp_impl_unconstrained.

Previously, StringSlice was directly calling _memcmp_impl_unconstrained. By switching to the standard memcmp, we allow the call to be properly dispatched to the optimized implementation (_memcmp_impl_opt_unconstrained).

This ensures that string slice comparisons leverage the best available memory comparison implementation provided by the standard library.

Signed-off-by: Yicheng Wu <yichengdwu@outlook.com>
@YichengDWu YichengDWu requested a review from a team as a code owner November 30, 2025 06:47
Copilot AI review requested due to automatic review settings November 30, 2025 06:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes the StringSlice comparison logic by replacing a direct call to the internal _memcmp_impl_unconstrained function with the public memcmp API, enabling automatic dispatch to optimized memory comparison implementations.

  • Removed direct import of internal _memcmp_impl_unconstrained function
  • Updated __lt__ method to use public memcmp API instead

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JoeLoser
Copy link
Copy Markdown
Member

JoeLoser commented Dec 2, 2025

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Dec 2, 2025
@modularbot
Copy link
Copy Markdown
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the main branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added merged-internally Indicates that this pull request has been merged internally merged-externally Merged externally in public mojo repo labels Dec 2, 2025
@modularbot modularbot closed this in 3fe4f86 Dec 4, 2025
@modularbot
Copy link
Copy Markdown
Collaborator

Landed in 3fe4f86! Thank you for your contribution 🎉

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. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants