Skip to content

Add comparable conformance for C++ strings - #76223

Merged
egorzhdan merged 1 commit into
swiftlang:mainfrom
ishon19:76220-string-should-conform-to-comparable
Sep 4, 2024
Merged

egorzhdan merged 1 commit into
swiftlang:mainfrom
ishon19:76220-string-should-conform-to-comparable

Conversation

@ishon19

@ishon19 ishon19 commented Sep 3, 2024

Copy link
Copy Markdown
Contributor

This PR adds Comparable conformance for C++ strings.

Fixes #76220

@egorzhdan egorzhdan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @ishon19 for this PR.
Could you please also add tests for the comparison between strings to the existing test file for std::string (test/Interop/Cxx/stdlib/use-std-string.swift)?

@ishon19

ishon19 commented Sep 3, 2024

Copy link
Copy Markdown
Contributor Author

Thanks @ishon19 for this PR. Could you please also add tests for the comparison between strings to the existing test file for std::string (test/Interop/Cxx/stdlib/use-std-string.swift)?

Sure, working on it!

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Sep 3, 2024
@ishon19 ishon19 changed the title feat: add comparable conformance for C++ strings Add comparable conformance for C++ strings Sep 3, 2024
@ishon19
ishon19 requested a review from egorzhdan September 3, 2024 16:14

@egorzhdan egorzhdan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is looking great! I only have one comment on the implementation.

On a separate note, could you please also squash the two commits into a single commit?

Comment thread stdlib/public/Cxx/std/String.swift Outdated
@ishon19
ishon19 force-pushed the 76220-string-should-conform-to-comparable branch from 83d69e2 to 49d9067 Compare September 3, 2024 17:51
@ishon19
ishon19 requested a review from egorzhdan September 3, 2024 17:53
@ishon19

ishon19 commented Sep 3, 2024

Copy link
Copy Markdown
Contributor Author

This is looking great! I only have one comment on the implementation.

On a separate note, could you please also squash the two commits into a single commit?

Thanks @egorzhdan! Sure, merged them all in a single commit now.

@egorzhdan

Copy link
Copy Markdown
Contributor

@swift-ci please test

@egorzhdan egorzhdan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks!

@egorzhdan
egorzhdan merged commit 03abb1f into swiftlang:main Sep 4, 2024
@ishon19

ishon19 commented Sep 4, 2024

Copy link
Copy Markdown
Contributor Author

LGTM, thanks!

Thanks for the review @egorzhdan! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ interop Feature: Interoperability with C++

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C++ std::string should conform to Comparable in Swift

2 participants