Skip to content

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Sep 3, 2018

Previously it wouldn't return a result if the function didn't have parameter names. This affected anyone working with the library on builtin functions.

@geoffw0 geoffw0 added the C++ label Sep 3, 2018
private import TaintTracking

private
bindingset[index]
Copy link
Contributor

Choose a reason for hiding this comment

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

Using bindingset makes the function inline, which can make it harder to predict how it'll be optimised in future contexts. Do these unnamed parameters at least have a Parameter object? Then you should be able to add exists(func.getParameter(index)) after the or and remove the bindingset annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that and I didn't get the same results (and the test fails). I believe getParameter had no results.

Because this predicate is private, any non-local performance effects due to inlining should be localized to within this library.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@jbj jbj merged commit ab944f3 into github:master Sep 4, 2018
aibaars added a commit that referenced this pull request Oct 14, 2021
printAst: use the user-facing AST library
smowton pushed a commit to smowton/codeql that referenced this pull request Dec 6, 2021
MathiasVP added a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
…cfg-successors

PS: Fix multiple CFG successors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants