Skip to content

C++: Modernize PrintIR for local dataflow#13266

Merged
jketema merged 5 commits into
github:mainfrom
MathiasVP:modernize-print-ir-local-flow
May 24, 2023
Merged

C++: Modernize PrintIR for local dataflow#13266
jketema merged 5 commits into
github:mainfrom
MathiasVP:modernize-print-ir-local-flow

Conversation

@MathiasVP
Copy link
Copy Markdown
Contributor

The PrintIRLocalFlow.qll file had bit-rotted quite a bit since no one was really using it, and after #13260 was merged it's the only remaining old-style dataflow configuration in the C/C++ library.

I had this PR lying around locally which updated the PrintIRLocalFlow.qll file (and related files) after the use-use flow changes, and coincidentally it also removed the old-style dataflow configuration.

cc @jketema

@MathiasVP MathiasVP requested review from a team as code owners May 23, 2023 17:06
string nodeId(DataFlow::Node node, int order1, int order2) {
exists(Instruction instruction | instruction = node.asInstruction() |
result = instruction.getResultId() and
string nodeId(Node node, int order1, int order2) {

Check warning

Code scanning / CodeQL

Missing QLDoc for parameter

The QLDoc has no documentation for order1, or order2, but the QLDoc mentions m128
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label May 23, 2023
Copy link
Copy Markdown
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

This seems to change the behaviour somewhat, e.g., no reference to any configuration anymore, but given that the alternative was to completely delete this internal, I'm not to worried about this, and it's always something we can add back later.

@jketema jketema merged commit 5dc3789 into github:main May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# 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