Michaelrfairhurst/package deadcode 9 rule 0-2-3#1043
Open
MichaelRFairhurst wants to merge 4 commits intomainfrom
Open
Michaelrfairhurst/package deadcode 9 rule 0-2-3#1043MichaelRFairhurst wants to merge 4 commits intomainfrom
MichaelRFairhurst wants to merge 4 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds support for MISRA C++ 2023 RULE-0-2-3, which identifies types with limited visibility (local types or types in anonymous namespaces) that are not used. Additionally, it improves the type usage analysis shared by multiple rules (RULE-2-3 for C, A0-1-6 for C++ AUTOSAR, and the new RULE-0-2-3 for C++ MISRA) to more accurately identify type uses and exclude self-referential uses.
Changes:
- Added RULE-0-2-3 query implementation for detecting unused types with limited visibility
- Introduced new DeadCode9 package to categorize this rule
- Enhanced type usage analysis to better handle anonymous types, template specializations, hidden friends, and nested declarations
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rules.csv | Updated RULE-0-2-3 entry to use DeadCode9 package |
| rule_packages/cpp/DeadCode9.json | New rule package metadata for RULE-0-2-3 |
| cpp/misra/test/rules/RULE-0-2-3/test.cpp | Comprehensive test cases for RULE-0-2-3 covering various edge cases |
| cpp/misra/test/rules/RULE-0-2-3/UnusedTypeWithLimitedVisibility.qlref | Test reference file for the query |
| cpp/misra/test/rules/RULE-0-2-3/UnusedTypeWithLimitedVisibility.expected | Expected test results |
| cpp/misra/src/rules/RULE-0-2-3/UnusedTypeWithLimitedVisibility.ql | Main query implementation for RULE-0-2-3 |
| cpp/common/test/rules/unusedtypedeclarations/test.cpp | Additional test cases for nested types |
| cpp/common/test/rules/unusedtypedeclarations/UnusedTypeDeclarations.expected | Updated expected results |
| cpp/common/src/codingstandards/cpp/types/Uses.qll | Enhanced type usage analysis with isWithinTypeDefinition predicate to exclude self-referential uses |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Added DeadCode9 package integration |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode9.qll | New exclusion module for DeadCode9 package |
| cpp/common/src/codingstandards/cpp/ast/HiddenFriend.qll | New module to identify hidden friend functions (uses Class.qll) |
| cpp/common/src/codingstandards/cpp/ast/Class.qll | New utility module for identifying the last declaration in a class |
| change_notes/2026-02-18-unused-type-improvements.md | Documents improvements to type usage analysis |
| c/common/test/rules/unusedtypedeclarations/test.c | Additional test cases for C typedef handling |
| c/common/test/rules/unusedtypedeclarations/UnusedTypeDeclarations.expected | Updated expected results for C tests |
Comments suppressed due to low confidence (2)
cpp/common/src/codingstandards/cpp/ast/HiddenFriend.qll:66
- The code calls 'last.getDeclaringType()' where 'last' is of type LastClassDeclaration, but the LastClassDeclaration class in Class.qll does not define a getDeclaringType() method. This will cause a compilation error. A getDeclaringType() method needs to be added to LastClassDeclaration that returns the 'cls' field.
last.getDeclaringType() = this and
cpp/common/src/codingstandards/cpp/ast/HiddenFriend.qll:81
- The code calls 'last.getDeclaringType()' where 'last' is of type LastClassDeclaration, but the LastClassDeclaration class in Class.qll does not define a getDeclaringType() method. This will cause a compilation error. A getDeclaringType() method needs to be added to LastClassDeclaration that returns the 'cls' field.
last.getDeclaringType() = this and
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespace { | ||
| struct A1 { | ||
| A1 f(); | ||
| }; // COMPLIANT - A1 is used below |
There was a problem hiding this comment.
The test case comment states 'COMPLIANT - A1 is used below' referring to line 36 where 'A1 l1;' is declared. However, line 36 is inside an uninstantiated template function 'foo()'. Since the template is never instantiated, A1 is not actually used, and the expected result correctly identifies it as NON_COMPLIANT. The comment should be updated to 'NON_COMPLIANT - A1 is only referenced in an uninstantiated template' to match the expected behavior.
Suggested change
| }; // COMPLIANT - A1 is used below | |
| }; // NON_COMPLIANT - A1 is only referenced in an uninstantiated template |
Comment on lines
+92
to
+100
| //exists(TemplateClass tpl, ClassTemplateSpecialization spec | | ||
| // tpl.getAnInstantiation() = type and | ||
| // tpl = spec.getPrimaryTemplate() and | ||
| // isWithinTypeDefinition(loc, spec) | ||
| //) | ||
| //exists(ClassTemplateSpecialization spec | | ||
| // specializationSharesType(type, spec) and | ||
| // isWithinTypeDefinition(loc, spec) | ||
| //) |
There was a problem hiding this comment.
There is commented-out code in the isWithinTypeDefinition predicate (lines 92-100). If this code is no longer needed, it should be removed. If it's being kept for future reference or debugging, a comment should be added explaining why it's commented out and whether it will be used in the future.
Suggested change
| //exists(TemplateClass tpl, ClassTemplateSpecialization spec | | |
| // tpl.getAnInstantiation() = type and | |
| // tpl = spec.getPrimaryTemplate() and | |
| // isWithinTypeDefinition(loc, spec) | |
| //) | |
| //exists(ClassTemplateSpecialization spec | | |
| // specializationSharesType(type, spec) and | |
| // isWithinTypeDefinition(loc, spec) | |
| //) |
Collaborator
Author
|
/test-performance |
|
🏁 Beep Boop! Performance testing for this PR has been initiated. Please check back later for results. Note that the query package generation step must complete before testing will start so it might be a minute. |
|
🏁 Beep Boop! Performance testing complete! See below for performance of the last 3 runs vs your PR. Times are based on predicate performance. You can find full graphs and stats in the PR that was created for this test in the release engineering repo. 🏁 Below are the slowest predicates for the last 2 releases vs this PR. |
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
1 participant
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.
Description
Updates to find type uses
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
RULE 0-2-31RULE 2-3A0-1-6Release change checklist
A change note (development_handbook.md#change-notes) is required for any pull request which modifies:
If you are only adding new rule queries, a change note is not required.
Author: Is a change note required?
🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.
Reviewer: Confirm that either a change note is not required or the change note is required and has been added.
Query development review checklist
For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:
Author
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Reviewer
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.