The Wayback Machine - https://web.archive.org/web/20221120031329/https://github.com/github/codeql/pull/11311
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

C++: Repair MustFlow library for use-use flow #11311

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Nov 16, 2022

The MustFlow library was previously defined in terms of the IR-based dataflow nodes, and implicitly relied on the assumption that all relevant instructions had dataflow nodes.

On the feature branch, this is no longer the case. Thus, we cannot use the dataflow nodes, and we have to use the IR directly.

Sadly, this is a breaking change. I think this is okay, however, because:

  1. I haven't seen any uses of it outside the two queries we have in Code Scanning that use it.
  2. It's really easy to transition from the dataflow nodes to the instructions (since the library was already using the IR-based dataflow library).
@MathiasVP MathiasVP requested a review from a team as a code owner Nov 16, 2022
@jketema
Copy link
Contributor

jketema commented Nov 18, 2022

I wonder if this PR should just target main instead of the feature branch, as it's pretty independent of the use-use dataflow changes.

@MathiasVP
Copy link
Contributor Author

MathiasVP commented Nov 18, 2022

I wonder if this PR should just target main instead of the feature branch, as it's pretty independent of the use-use dataflow changes.

Indeed, I'd also be okay with this.

@MathiasVP MathiasVP marked this pull request as draft Nov 18, 2022
@MathiasVP MathiasVP marked this pull request as ready for review Nov 18, 2022
@MathiasVP
Copy link
Contributor Author

MathiasVP commented Nov 18, 2022

I've retargeted the PR to target main now. I managed to do so without pinging all teams 🎉, but I didn't manage to stop the bot from adding a bunch of unnecessary labels 😭. Oh well!

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