-
Notifications
You must be signed in to change notification settings - Fork 62
A2-10-1: add functions and types to identifier consideration #546
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
Conversation
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.
The generalization of identifiers needs some further consideration, because of the increased complexity and incomplete exclusions part of the first iteration.
Possible FN are class members where a derived class member hides a base class member. It is possibly because the outer/inner scope definition for this rule doesn't mention it, but it successor in Misra C++ 23 does include that explicitly in the definition of the outer/inner scope.
Our hides
logic now has to consider functions and their overloads. The query itself uses the strict version that requires a different scope which excludes this as problem. Not sure if we want to correct the non-strict version.
Given that the Misra C/C++ 23 version of this rule, rule 6.4.1 and 6.4.2, focuses solely on variables and functions, I wonder if we want to do the same here and exclude types.
…named namespaces exception
…omit intentional overloads
this is covered in A10-2-1 - so we should be ok to not worry about this exact case?
currently the only use of
done, great catch (didnt own the 23 version til this comment) |
Co-authored-by: Remco Vermeulen <[email protected]>
Co-authored-by: Remco Vermeulen <[email protected]>
cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
Outdated
Show resolved
Hide resolved
cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
Outdated
Show resolved
Hide resolved
cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
Outdated
Show resolved
Hide resolved
cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
Outdated
Show resolved
Hide resolved
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.
I left a few considerations for the lambda case and proposal for more reorder the parameters for consistency.
Handle lambda scope using `Scope` class
With the addition of other declarations the performance of this query has significantly decreased. My current hypothesis is that the number of declarations in the global namespace explodes the size of the |
…mbda hiding due to performance the simpler heurisitic is better
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.
LGTM.
Description
fixes #118
the query seems to maybe be slower than before, will need this confirmed
Change request type
.ql
,.qll
,.qls
or unit tests)Rules with added or modified queries
Release 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.