-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
C++: Add missing parent scope cases #17776
Conversation
@@ -14,7 +17,11 @@ | |||
| 1 | parents.cpp:4:10:4:10 | f | parents.cpp:4:19:13:5 | { ... } | | |||
| 1 | parents.cpp:4:19:13:5 | { ... } | parents.cpp:5:11:5:11 | j | | |||
| 1 | parents.cpp:4:19:13:5 | { ... } | parents.cpp:6:11:10:7 | { ... } | | |||
| 1 | parents.cpp:4:19:13:5 | { ... } | parents.cpp:11:18:11:18 | e | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like the parent of e
should be something associated with the catch block on line 11, rather than the entire function. I can see that this wouldn't fit the current definition of what getParentScope
can return. And what you have is probably good enough for what getParentScope
is used for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, agreed. I struggled a bit with that one initially. What I do now seems to be most in line with all the other cases that are there, and what we want when looking for shadowing.
Which query is this? |
Yep, discussing the intent with one of the authors might help to untangle what's going on there. |
I've reported the issue on the coding standards repo. I assume they'll have a look once they upgrade to CodeQL 2.19.3 or later. |
This is mostly used to detect the shadowing of variable declarations.
DCA shows no changes. This does break some coding standards tests. I'll update the test results there after this is merged, and open an issue that informs the coding standards people of this. I briefly looked at fixing the query, but (a) it's not clearly documented what the query should do, and (b) this requires digging through about 400 lines of query code, which I find very hard to follow.
Pull Request checklist
All query authors
.qhelp
. See the documentation in this repository.Internal query authors only
.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).