Skip to content

C++: Change countNumberOfBranchesUsingParameter to match qldoc closer.#18908

Merged
MathiasVP merged 1 commit intogithub:mainfrom
aschackmull:cpp/branchlimit-adjustment-refactor
Mar 5, 2025
Merged

C++: Change countNumberOfBranchesUsingParameter to match qldoc closer.#18908
MathiasVP merged 1 commit intogithub:mainfrom
aschackmull:cpp/branchlimit-adjustment-refactor

Conversation

@aschackmull
Copy link
Copy Markdown
Contributor

@aschackmull aschackmull commented Mar 3, 2025

This changes one ad-hoc fieldFlowBranchLimit tweak to another. I tried to do some comparisons on the computed counts, and there are plenty of discrepancies, but that's possibly completely fine - let's see what dca says.

The primary motivation for this change is to get rid of the dependence on SSA Phi Read nodes, which I plan to hide within the SSA library.

@github-actions github-actions Bot added the C++ label Mar 3, 2025
@aschackmull
Copy link
Copy Markdown
Contributor Author

Dca looks fine, so opening this for review. @MathiasVP looks like there was an issue with QA - do you want to retry that or is the dca run sufficient?

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Mar 4, 2025
@aschackmull aschackmull marked this pull request as ready for review March 4, 2025 07:38
Copilot AI review requested due to automatic review settings March 4, 2025 07:38
@aschackmull aschackmull requested a review from a team as a code owner March 4, 2025 07:38
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.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@MathiasVP
Copy link
Copy Markdown
Contributor

Ì think we should redo QA. I'll restart it now

@aschackmull
Copy link
Copy Markdown
Contributor Author

QA looks sufficiently uneventful, I think?

Copy link
Copy Markdown
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

Indeed 😄 LGTM!

@MathiasVP MathiasVP merged commit 38bf9c6 into github:main Mar 5, 2025
@aschackmull aschackmull deleted the cpp/branchlimit-adjustment-refactor branch March 5, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants