C++: Use the new prototype-related extensionals in MaD#21899
Open
MathiasVP wants to merge 5 commits into
Open
Conversation
MathiasVP
force-pushed
the
use-new-prototype-extensionals
branch
from
357fd96 to
3002476
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the C++ MaD (Models as Data) support to leverage the newer template “prototype/origin” extensionals introduced in #21870, while retaining a fallback path for older databases whose upgrade leaves those extensionals empty.
Changes:
- Add “new vs old” implementations for resolving fully-templated class/function entities, selecting based on whether the new extensionals are populated.
- Adjust signature/type-name expansion to use the fully-templated declaring class resolution.
- Update the MaD signatures test expectations to reflect the new resolution behavior.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll | Switches MaD template-resolution logic to prefer new getOriginalTemplate-based extensionals with a compatibility fallback; minor QLDoc touch-ups. |
| cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected | Updates expected signature/type-name output to match the new template-resolution behavior. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
…o their fully templated versions.
MathiasVP
force-pushed
the
use-new-prototype-extensionals
branch
from
67d7281 to
2d58150
Compare
jketema
reviewed
May 29, 2026
Contributor
jketema
left a comment
jketema
left a comment
There was a problem hiding this comment.
Some comments and questions.
| /** Gets the fully templated version of `f`. */ | ||
| Function getFullyTemplatedFunction(Function f) { | ||
| // The `Function::getOriginalTemplate` predicate was introduced in CodeQL | ||
| // version 2.25.6 and the upgrade script leaves the |
Contributor
There was a problem hiding this comment.
Suggested change
| // version 2.25.6 and the upgrade script leaves the | |
| // version 2.25.7 and the upgrade script leaves the |
| /** Gets the fully templated version of `c`. */ | ||
| private Class getFullyTemplatedClass(Class c) { | ||
| // The `Class::getOriginalTemplate` predicate was introduced in CodeQL | ||
| // version 2.25.6 and the upgrade script leaves the |
Contributor
There was a problem hiding this comment.
Suggested change
| // version 2.25.6 and the upgrade script leaves the | |
| // version 2.25.7 and the upgrade script leaves the |
Comment on lines
+356
to
+367
| private Function getFullyTemplatedFunctionNew(Function f) { | ||
| not f.isFromUninstantiatedTemplate(_) and | ||
| exists(Function mid | | ||
| f.isConstructedFrom(mid) | ||
| or | ||
| not f.isConstructedFrom(_) and f = mid | ||
| | | ||
| result = getOriginalFunctionTemplate(mid) | ||
| or | ||
| not mid instanceof TemplateFunction and mid = result | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Does this properly handle the case where the template we are instantiating has multiple template parameters and we instantiate those not all at once but in multiple steps?
Comment on lines
+293
to
+304
| private Class getFullyTemplatedClassNew(Class c) { | ||
| not c.isFromUninstantiatedTemplate(_) and | ||
| exists(Class mid | | ||
| c.isConstructedFrom(mid) | ||
| or | ||
| not c.isConstructedFrom(_) and c = mid | ||
| | | ||
| result = getOriginalClassTemplate(mid) | ||
| or | ||
| not mid instanceof TemplateClass and mid = result | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Same question as for functions.
Contributor
Author
There was a problem hiding this comment.
Hmmm... I don't think I can construct an example for functions similar to what was added in e18448d. AFAIK, it's not possible to have an alias template for a function template.
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.
This PR makes use of the new extensionals that were added in #21870 to fix a couple of bugs in MaD.
I discussed the best way to handle "backwards compatibility" with @jketema, and we agreed to keep the old code in production for a couple of versions to allow new databases with populated
class_template_generated_fromandfunction_template_generated_fromextensionals to be produced.The DCA run is up-to-date (and boring as expected). The force-push was just me fixing up QLDoc.