Skip to content

Simplify C locations handling #12

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

Merged
merged 3 commits into from
Aug 15, 2018
Merged

Simplify C locations handling #12

merged 3 commits into from
Aug 15, 2018

Conversation

pavgust
Copy link
Contributor

@pavgust pavgust commented Aug 3, 2018

The first commit simplifies the various bits of location handling to refer to a shared new predicate (Location::fullLocationInfo, which is like hasLocationInfo/5 but exposes the actual container entity). This avoids a certain amount of repetition, as all the predicates that explicitly called the three locations_* tables can now just refer to the single place that unions those tables.

The second commit is a minor simplification to defintions.qll, following recent changes to location handling (and more things becoming Containers rather than Files). In particular, it allows us to find a much better way of evaluating that part of the predicate.

@pavgust pavgust requested a review from a team August 3, 2018 15:40
nickrolfe pushed a commit to nickrolfe/codeql that referenced this pull request Aug 6, 2018
JavaScript: cryptographic keys as sinks for js/insecure-randomness
@jbj
Copy link
Contributor

jbj commented Aug 7, 2018

@pavgust wrote on Slack:

For the record, #12 is somewhat invasive, and I'm profiling it on our usual set of projects.

Let's postpone merging until we hear how the profiling went.

@pavgust
Copy link
Contributor Author

pavgust commented Aug 9, 2018

Profiling still going on; this is likely to pick up a few more changes and tweaks shortly.

@pavgust pavgust added WIP This is a work-in-progress, do not merge yet! and removed WIP This is a work-in-progress, do not merge yet! labels Aug 10, 2018
@pavgust pavgust force-pushed the imp/c-locations branch 2 times, most recently from 1bef161 to 41a36e1 Compare August 13, 2018 21:41
@pavgust pavgust requested a review from a team as a code owner August 13, 2018 21:41
@pavgust pavgust removed the WIP This is a work-in-progress, do not merge yet! label Aug 14, 2018
@pavgust
Copy link
Contributor Author

pavgust commented Aug 14, 2018

@jbj Profiling looks good, results unchanged. I think this is good to go.

@pavgust pavgust removed the request for review from a team August 15, 2018 00:44
@jbj jbj added the C++ label Aug 15, 2018
@jbj jbj self-assigned this Aug 15, 2018
@jbj jbj merged commit 6225fcf into github:master Aug 15, 2018
aibaars added a commit that referenced this pull request Oct 14, 2021
Use tree-sitter-ruby crate instead of vendoring it
smowton pushed a commit to smowton/codeql that referenced this pull request Oct 28, 2021
Kotlin: Use a TrapWriter for the invocation TRAP
hohn pushed a commit to hohn/codeql that referenced this pull request Dec 13, 2021
RasmusWL pushed a commit that referenced this pull request May 30, 2022
Move tests back into `frameworks/` folder
dbartol pushed a commit that referenced this pull request Dec 18, 2024
feat(bash-step): Improve bash step accuracy
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