Skip to content

upgrade Swift plugin to use llvm::Expected for GetIndexOfChildWithName #10593

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

Conversation

charles-zablit
Copy link

In llvm/llvm-project, this PR changed the return type of GetIndexOfChildWithName.

This PR reflects those changes in the Swift plugin for LLDB.

if (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors())
return UINT32_MAX;
if ((idx == UINT32_MAX) ||
(idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))

Choose a reason for hiding this comment

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

idx < UINT32_MAX is now redundant, right?

Copy link
Author

Choose a reason for hiding this comment

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

You are right, I assumed that the value could go higher but it's a uint32 not an uint.

if (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors())
return UINT32_MAX;
if ((idx == UINT32_MAX) ||
(idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))

Choose a reason for hiding this comment

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

same idea here

Copy link

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

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

LGTM! Just make sure to squash the "review commits" before merging

@charles-zablit charles-zablit force-pushed the charles-zablit/fix-merge-136693 branch 2 times, most recently from bf75a76 to e12f551 Compare May 1, 2025 15:17
@charles-zablit charles-zablit force-pushed the charles-zablit/fix-merge-136693 branch from e12f551 to 105d084 Compare May 1, 2025 15:19
@felipepiovezan felipepiovezan merged commit 4589caf into swiftlang:next May 1, 2025
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.

2 participants