Repository navigation
Respect initializer side effects in unused-variable queries - #22938
Merged
Merged
Conversation
Cover side-effecting initializers for local and static variables, recording the current alerts as spurious. Also cover constructor and destructor side effects for static global objects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jketema
force-pushed
the
copilot/unused-static-side-effect-test
branch
from
October 29, 2026 15:13
bf32c2e to
f985aee
Compare
Exclude local and static variables whose initializer may be impure, preserving existing constructor and destructor exemptions. Regenerate expected results and mark the range-for case as missing because its undefined iterator dereference may have side effects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a minor analysis change note for suppressing alerts on variables whose initializers may have side effects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Treat the unused range-for variable as intentionally unreported because its initializer may have side effects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jketema
changed the title
jketema
commented
Oct 29, 2026
| } | ||
|
|
||
| for (int v2 : myContainer) // $ Alert // BAD: v2 is not used | ||
| for (int v2 : myContainer) // GOOD: iterator dereference may have side effects |
Contributor
Author
There was a problem hiding this comment.
Note that the relevant member functions in MyContainer are missing function bodies, so we don't known if there may be side-effects.
Contributor
There was a problem hiding this comment.
I would hope that iterating through a container doesn't have side-effects in the vast majority of cases. But this is C++, people are bound to have come up with containers where they do.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the unused-variable query changes.
No open issues 🎉
What changed in this PR
This PR reduces false positives in the C++ unused-variable queries by preserving declarations whose initializers may have side effects, addressing the initializer-related behavior in #22928.
Changes:
- Uses
Expr.mayBeImpure()in both queries while retaining existing constructor/destructor exemptions. - Adds regression coverage for side-effecting initializers and static-object constructors/destructors, and updates the range-for expectation.
- Adds a minor analysis change note.
| File | Description |
|---|---|
cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticVariables/test.cpp |
Adds initializer, constructor, and destructor regression cases. |
cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedLocals/UnusedLocals.expected |
Removes the expected range-for alert. |
cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedLocals/code2.cpp |
Adds an initializer regression case and updates the range-for annotation. |
cpp/ql/src/change-notes/2026-10-29-unused-variable-initializers.md |
Documents the queries’ reduced false positives. |
cpp/ql/src/Best Practices/Unused Entities/UnusedStaticVariables.ql |
Excludes static variables with potentially impure initializers. |
cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql |
Excludes local variables with potentially impure initializers. |
Review effort: Balanced (auto)
jketema
commented
Oct 29, 2026
geoffw0
approved these changes
Oct 30, 2026
| } | ||
|
|
||
| for (int v2 : myContainer) // $ Alert // BAD: v2 is not used | ||
| for (int v2 : myContainer) // GOOD: iterator dereference may have side effects |
Contributor
There was a problem hiding this comment.
I would hope that iterating through a container doesn't have side-effects in the vast majority of cases. But this is C++, people are bound to have come up with containers where they do.
This file contains hidden or 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
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.
The
cpp/unused-local-variableandcpp/unused-static-variablequeries now exclude variables whose initializers may have side effects, usingExpr.mayBeImpure()while preserving the existing constructor/destructor exemptions.Adds regression coverage for side-effecting local and static initializers, plus separate constructor and destructor cases for static global objects. Updates the range-for expectation because iterator dereference may have side effects, regenerates expected results, and adds a minor analysis change note.
Fixes: #22928.
Validation: both affected query tests pass with CodeQL CLI 2.27.3.