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

[NFC] [PrintAsClang] Add tiebreaking rule to sort #74597

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

beccadax
Copy link
Contributor

Because the underlying API for fetching top-level decls returns them in an unspecified order, PrintAsClang sorts the decls before printing them to make the output order more stable. However, the rules currently implemented have at least one known defect (they compare only the unqualified name of a nested class, so two nested classes with the same Swift name sort in an arbitrary order), and there are likely many more.

Add a fallback rule which sorts declarations by their mangled name; this should at least distinguish all non-colliding ValueDecls from each other, albeit according to fairly opaque criteria. Additionally add a rule to help distinguish extensions with very similar content, and tweak other logic so that the comparison function is less likely to give up early rather than continuing to look for a usable difference.

Fixes rdar://129485103.

@beccadax beccadax requested a review from tshortli June 20, 2024 22:55
@beccadax
Copy link
Contributor Author

@swift-ci please test

Because the underlying API for fetching top-level decls returns them in an unspecified order, PrintAsClang sorts the decls before printing them to make the output order more stable. However, the rules currently implemented have at least one known defect (they compare only the unqualified name of a nested class, so two nested classes with the same Swift name sort in an arbitrary order), and there are likely many more.

Add a fallback rule which sorts declarations by their mangled name; this should at least distinguish all non-colliding ValueDecls from each other, albeit according to fairly opaque criteria. Additionally add a rule to help distinguish extensions with very similar content, and tweak other logic so that the comparison function is less likely to give up early rather than continuing to look for a usable difference.

Fixes rdar://129485103.
// member instead.
{
auto mismatch =
std::mismatch(cast<ExtensionDecl>(*lhs)->getMembers().begin(),
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about std::mismatch

Copy link
Contributor Author

@beccadax beccadax Jun 21, 2024

Choose a reason for hiding this comment

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

It was news to me too!

@beccadax
Copy link
Contributor Author

@swift-ci please test

@beccadax beccadax enabled auto-merge June 21, 2024 23:53
@beccadax beccadax merged commit c3b57f2 into swiftlang:main Jun 22, 2024
5 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