C++: Work around two false positive issues with the UnusedLocals.ql query #4592
Conversation
|
I had a quick look at the first one, and I can't immediately see what's going wrong. Could you file a couple of tickets please? Thanks! |
| not exists(Literal l | | ||
| l.getEnclosingFunction() = f and | ||
| not exists(l.getValue()) | ||
| ) and | ||
| not any(ConditionDeclExpr cde).getEnclosingFunction() = f |
There was a problem hiding this comment.
Please add some comments that'll help us know when to delete these extra conditions in the future.
There was a problem hiding this comment.
Done. The details aren't really fresh in my mind any more, but at least its documented that they should at some point be removed.
Co-authored-by: Jonas Jensen <jbj@github.com>
Done: https://github.com/github/codeql-c-extractor-team/issues/150 and https://github.com/github/codeql-c-extractor-team/issues/151. It's a while since I looked at this so apologies for any inaccuracies - particularly on the second one. |
|
The qlformat test failed |
|
Thanks, fixed. |
Test cases and workaround fixes for the issues highlighted by #4569 and #4570.
Sample of results from LGTM that will now be excluded: https://lgtm.com/query/6496877322277687392/
@igfoo I believe both cases may be caused by limitations in the extractor where a variable is accessed in a particular context, but no
VariableAccessin generated.;(aConditionDeclExpr) appears not to have been extracted correctly. If this is fixed I would like to remove the exclusion forConditionDeclExprfrom this query (affects up to ~0.2% of functions).VariableAccessinside the expression that is currently an 'unknown literal', so we don't need to exclude all functions containing unknown literals from the test (affects up to ~10% of functions).Are you happy to deal with these, should I create issues, or are they already on your radar?