-
Notifications
You must be signed in to change notification settings - Fork 69
Add new library, backported to a small handful of queries, for resolving types. #991
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
base: main
Are you sure you want to change the base?
Conversation
…ing types.
Expands upon `PossiblySpecified<T>` module, which was nothing but a renaming of
`.stripSpeciifers` that was intended to be clearer (and not resolve typedefs).
However, there is almost never a reason not to resolve _certain_ typedefs.
Assume we are writing a query to detect usages of a typedef such as `uint8_t`.
If we have a candidate type and wish to check if it is a `uint8_t`, we may do so
via `candidate.(TypedefType).hasName("uint8_t")`. However, this will not match
`const uint8_t`. The type methods such as `stripSpecifiers`,
`getUnderlyingType`, can be used but are sometimes vague, and some implicitly
resolve typedefs while others do not, and some even have incorrect
documentation.
Furthermore, if a user has declared `typedef uint8_t uchar_t` or
`typedef const uint8_t uint8_const_t`, then we _must_ resolve typedefs to find
these usages of `uint8_t` in the project. However, `candidate.resolveTypedefs()`
will also unwrap the `uint8_t`. In C++ the problem is worse too, as we would
need to detect cases such as `const uint8_t&`, and decltypes.
This project needs, IMO, a better API for incrementally resolving typedefs,
decltypes, specifiers, and references. This PR adds what is hopefully a decent
starting point for this.
Instead of writing `candidate.resolveTypedefs() instanceof FooType` or
`candidate.stripTopLevelSpecifiers() instanceof FooType` or
`candidate.getUnderlyingType() instanceof FooType`, the `cpp.types.Resolve`
import now allows `candidate instanceof ResolvesTo<FooType>::Exactly` to only
resolve typedefs and decltypes, or `::IgnoringSpecifiers` to resolve typedefs,
decltypes, and unwrap `SpecifiedType`s as well. These types have a `.resolve()`
member predicate to unwrap everything and get the resolved `FooType`.
The API basically intends to define an interterface for how to unwrap type A in
search of a type B. Types are like linked lists, with flags on certain links.
We really just want a means of traversing different kinds of links in different
contexts, sometimes requiring the presence of a certain kind of link, and
sometimes aggregating properties about what links were traversed.
We also want an API that provides some guard rails and funnels people towards
correct usage of itself. Hopefully `::Exactly` comes across as too strict, and
hopefully `CvConst` makes it clear that a type may be `const` or
`const volatile`.
Also added some types that compliment this API well such as
`PointerTo<T>::Type` and `ReferenceOf<T>::Type`.
Also integrated this library into a few example queries so the PR includes
example usage.
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.
Pull request overview
This PR introduces a new type resolution library (ResolvesTo, PointerTo, ReferenceOf) to replace the existing PossiblySpecified API. The new library provides clearer semantics for resolving typedefs, decltypes, and cv-qualifiers, and handles edge cases where certain type combinations don't exist in the database. The new API has been integrated into five queries across CERT and MISRA rule sets.
Key Changes
- New comprehensive type resolution module with classes like
ResolvesTo<T>::Exactly,ResolvesTo<T>::IgnoringSpecifiers,ResolvesTo<T>::CvConst,ResolvesTo<T>::Ref, andResolvesTo<T>::CvConstRef - Support for pointer and reference type construction via
PointerTo<T>::TypeandReferenceOf<T>::Type - Removal of the
PossiblySpecifiedAPI fromType.qll - Migration of five queries to use the new API with improved typedef handling
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
cpp/common/src/codingstandards/cpp/types/Resolve.qll |
Core type resolution library with parameterized modules for matching types through typedef/decltype chains |
cpp/common/src/codingstandards/cpp/types/Specifiers.qll |
Helper classes for matching const-specified types |
cpp/common/src/codingstandards/cpp/types/Type.qll |
Removes deprecated PossiblySpecified module |
cpp/misra/src/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.ql |
Migrated to use ResolvesTo<PointerTo<NonConstType>>::IgnoringSpecifiers and ResolvesTo<ReferenceOf<NonConstType>>::IgnoringSpecifiers |
c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql |
Migrated from PossiblySpecified to ResolvesTo<T>::IgnoringSpecifiers for mutex and condition types |
c/misra/src/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.ql |
Migrated to use ResolvesTo<C11ThreadingObjectType>::IgnoringSpecifiers |
c/misra/src/rules/RULE-22-12/NonstandardUseOfThreadingObject.ql |
Refactored isThreadingObject predicate to use new type resolution |
c/cert/src/rules/INT36-C/ConvertingAPointerToIntegerOrIntegerToPointer.ql |
Comprehensive migration replacing getUnderlyingType() with ResolvesTo<T>::IgnoringSpecifiers for int, pointer, intptr_t, and void pointer types |
cpp/common/test/library/codingstandards/cpp/types/Resolve/ResolveTest.ql |
Test query validating the new type resolution API |
cpp/common/test/library/codingstandards/cpp/types/Resolve/test.cpp |
Comprehensive test cases covering typedefs, decltypes, references, and cv-qualifiers |
cpp/common/test/library/codingstandards/cpp/types/options |
Extractor options for test compilation |
change_notes/2025-12-03-type-resolution-tracking-changes.md |
Change notes documenting query modifications |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * // Examples types that are not `ResolvesTo<FooType>::Ref` types: | ||
| * const FooType &cf; // does not match (specified references to FooTypes) | ||
| * FooType rf = f; // does not match (non-rerefence FooTypes) |
Copilot
AI
Dec 3, 2025
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.
Typo: "rerefence" should be "reference".
| * FooType rf = f; // does not match (non-rerefence FooTypes) | |
| * FooType rf = f; // does not match (non-reference FooTypes) |
Description
There are problems with the APIs on types for handling typedefs, decltypes, and specifiers, etc. Some of them resolve typedefs and some of them don't, and its not clear. Some of them have incorrect documentation. Some of them have bugs (for instance,
ArrayType.resolveTypedefs()). These APIs also have a fundamental flaw: they assume that a given type exists in the database. For instance, if typeThas a typedefT_t, thenconst T_tmay exist in the database andconst Tmay not.At one point I decided to make a new API for these methods that was more clear, and I created
PossiblySpecified<T>::Type. One of the important points inPossiblySpecified<T>was that it doesn't resolve typedefs, so that the typeTcan be a typedef type such asuint8_t. However, what if someone makes a typedef ofuint8_t, for instance,typedef uint8_t ubyte_t? Well in this case,PossiblySpecified<Uint8Type>won't matchubtyte_t. So clearly we need to resolve some typedefs. But if we try to write a query that usesexpr.getType().resolveTypedefs() instanceof Uint8Type, we'll have no results.Furthermore, we will have a lot of places in our queries where we want to treat
TandT&the same.So we really want an API that:
This PR introduces a candidate API and I welcome feedback on it.
Instead of
expr.getUnderlyingType() instanceof PointerType, you can writeexpr.getType() instanceof ResolvesTo<PointerType>::Exactly. This should raise some hairs "what do you mean by exactly?" That's becausegetUnderlyingType() instanceof PointerType()is usually wrong, or at least incomplete. Exactly means it will not match a cv-qualified pointer type. In this case, typically what we want to write isexpr.getType() instanceof ResolvesTo<PointerType>::IgnoringSpecifiers.Other options exist:
ResolvesTo<PointerType>::CvConstwill match a const pointer or const volatile pointer.ResolvesTo<PointerType>::Refmatches a reference to a pointer type.ResolvesTo<PointerType>::CvConstRefmatches a reference to a const pointer (not to be confused with a const reference to a pointer, which doesn't exist!).Added some additional classes
PointerTo<T>::Type,ReferenceOf<T>::Type, that work well with this API, such that you can writeResolvesTo<PointerTo<FooType>::Type>::IgnoringSpecifiers. AlsoRawConstType,ConstType,NonConstType.In addition to replacing the prior usages of
PossiblySpecified<T>, I also integrated this API into Jeongsoo's recent query, for comparison, and one other random query I found via grep that was usinggetUnderlyingType.Comments and feedback and bikeshedding welcome!
These are some of the improvements I'd like to see in it one day myself:
ResolvesTo<IntType>::Pointer.CvConstRef. This is complicated by CodeQL's handling of nested specified types (it doesn't!).RefOrNonRefkind of concepts (e.g., I want to match bothTandT&).Additional ideas?
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
INT36-CRULE-22-12RULE-22-13RULE-22-14RULE-9-5-1Release 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.