Go: make data flow consistency checks available (and fix one) #14547
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This can be run on an existing database to check for any assumptions of the data flow library which do not hold.
It wasn't updated when MkImplicitVarargsSlice was added as a branch of TNode. This meant that it gave no result for `ImplicitVarargsSlice`s in function calls used to initialise variables declared at file level.
mbg
reviewed
Oct 20, 2023
go/ql/lib/change-notes/2023-10-20-enclosing-callable-for-external-files.md
Outdated
Show resolved
Hide resolved
go/ql/consistency-queries/qlpack.yml
Outdated
There was a problem hiding this comment.
Question: why does this get its own qlpack?
There was a problem hiding this comment.
Reason 1: all the other languages do it...
…nal-files.md Co-authored-by: Michael B. Gale <[email protected]>
|
It isn't currently run in CI. I'm not sure what other languages do. I stumbled across some experimental tests using it for python. It's a bit unclear to me what test code it should be run on. I've just been running it on some databases I already had handy. |
|
It seems the standard way to run it in CI is to add it to the consistency queries, so it's run on all tests. But we can't really do that till we've made all the queries have zero results. |
|
Performance testing showed so significant difference. I think this is ready to merge. |
owen-mc
changed the title
smowton
approved these changes
Oct 25, 2023
owen-mc
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes the data flow consistency checks for go available. They currently have to be manually. When we have done the work to reduce the number of expected results to zero we can add them to the consistency checks so that we will notice when there is a regression.
This PR also fixes a problem which was highlighted that not all nodes have a result for
nodeGetEnclosingCallable. Two steps were needed: (1) fixing an accidental exclusion of ImplicitVarArgsSlice nodes and (2) making a new branch ofTDataFlowCallablecalledTExternalFileScopewhich is used only as a return value fornodeGetEnclosingCallablewhen a node exists in an external file (i.e. one which is not in the project being analyzed).