Skip to content

[RemoteAddress] Handle comparison of addresses in different spaces #82995

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

Merged
merged 1 commit into from
Jul 11, 2025

Conversation

augusto2112
Copy link
Contributor

Sometimes it makes sense to compares addresses from different address spaces.

rdar://148361743

@augusto2112 augusto2112 requested a review from a team as a code owner July 11, 2025 16:30
@augusto2112
Copy link
Contributor Author

@swift-ci smoke test

@@ -64,6 +64,11 @@ TypeRefBuilder::ReflectionTypeDescriptorFinder::
.TypeReference.startAddress()
.getRemoteAddress();

// Sort first by address space, then by address.
Copy link
Contributor

Choose a reason for hiding this comment

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

oh I guess this is the comment. Should RemoteAddress implement <? Or should this logic be encapsulated somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RemoteAddress does implement <, but by default we assert if the address spaces are different. For ordering purposes it makes sense to compare the address space too though, as long as we are consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added oderedLessThan and orderedLessThanOrEqual to encapsulate this logic.

Sometimes it makes sense to compares addresses from different address
spaces.

rdar://148361743
@augusto2112
Copy link
Contributor Author

@swift-ci smoke test

@augusto2112 augusto2112 merged commit d9d7db9 into swiftlang:main Jul 11, 2025
3 checks passed
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.

3 participants