M5-3-1: Exclude unknown types #852
Conversation
This ensures we consistently exclude unknown results for unevaluated contexts in uninstantiated templates.
There was a problem hiding this comment.
PR Overview
This pull request adds a change note describing the exclusion of unknown results in unevaluated contexts for the M5-3-1 rule.
- Adds a new change note markdown file.
- Describes consistent exclusion of uninstantiated template contexts.
Changes
| File | Description |
|---|---|
| change_notes/2025-02-06-m5-3-1-exclude-unknown-type.md | Adds documentation about excluding unknown results in unevaluated contexts. |
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
| // within uninstantiated templates. It's necessary to check for this explicitly because | ||
| // not all unevaluated contexts are considered to be `isFromUninstantiatedTemplate(_)`, | ||
| // e.g. `noexcept` specifiers | ||
| not t instanceof UnknownType and |
There was a problem hiding this comment.
This is great, and we should do this...probably just about everywhere!
How about:
signature class ExamineType = Type;
module TypeOps<ExamineType T> {
final class TypeFinal = Type;
class Subtype extends TypeFinal {
T superType;
SubType() {
this instanceof UnknownType
or
this = superType
or
this.getUnspecifiedType() = superType
}
}
}
Additional work would have to be done (not right away) to properly support TypeOps<SpecifiedType>::SubType and TypeOps<TypeDefType>::SubType, and to apply the right subtyping recursively for types like PointerType, FunctionType, and class types with inheritance.
But generally, resolveTypedefs(), unspecifiedType(), stripTopLevelSpecifiers() are clunky operations on types that don't truly reflect our intention when we use them. Typically when we use them, we're asking, "is this compatible with." I know that subtyping per se is not quite the right term in C/C++, its worth thinking about what relationship(s) we typically ask about in queries.
Description
Fixes #851.
Ensure we consistently exclude unknown results for unevaluated contexts in uninstantiated templates.
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
M5-3-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.