Skip to content

C++: Allow forwarding to nested types forwards MaD models - #23016

Merged
MathiasVP merged 3 commits into
github:mainfrom
MathiasVP:replace-ql-models-with-mad-for-assoc-cleaned-up-step-1
Nov 4, 2026
Merged

MathiasVP merged 3 commits into
github:mainfrom
MathiasVP:replace-ql-models-with-mad-for-assoc-cleaned-up-step-1

Conversation

@MathiasVP

@MathiasVP MathiasVP commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Consider a type such as:

template<typename T>
struct Container {
  template<typename... Args>
  void insert(Args...&& args);
};

We can model the fact that insert creates a T object and inserts it into a value of type Container by using a forwards model like:

- addsTo:
      pack: codeql/cpp-all
      extensible: forwardsModel
    data:
      - ["", "Container<T>", True, "insert<Args>", "(Args)", "", "0", "T", "Argument[-1].Element", "manual"]

However, the value to construct can be more complex to identify than just the type name T. For example, std::map constructs a value of type std::pair<const Key, T>. And there's currently no way to identify such a complex type in the interpretation of the forwards rows (nor do I think we should add support for identifying such complex types!)

Instead, this PR makes it possible to simplify use the name of the nested type in the forwards model. For example, if we have:

template<typename T>
struct Container {
  using value_type = Wrapper<T>;

  template<typename... Args>
  void insert(Args...&& args);
};

then we can model the fact that insert creates a Wrapper<T> object and inserts it into a value of type Container by using a forwards model like:

- addsTo:
      pack: codeql/cpp-all
      extensible: forwardsModel
    data:
      - ["", "Container<T>", True, "insert<Args>", "(Args)", "", "0", "value_type", "Argument[-1].Element", "manual"]

Since we already had a suitable testcase for this I simply added a typedef and reused the existing testcase.

We still don't have any non-test forwards models. So I don't think this PR needs a DCA run.

@MathiasVP
MathiasVP requested a review from a team as a code owner November 4, 2026 13:47
Copilot AI auto review requested due to automatic review settings November 4, 2026 13:47
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Nov 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Stale expected-result files will fail fixture comparisons, and unresolved inherited value_type aliases can cause forwarding flows to be missed.

2 open issues
What changed in this PR

This PR extends C++ model-as-data forwarding models to identify constructed types through nested aliases, avoiding the need to express complex template types directly in model rows.

Changes:

  • Resolve named typedef members on the forwarding function’s declaring type.
  • Update the container forwarding model to use value_type.
  • Add scalar and pointer cases to the existing data-flow fixture.
File Description
cpp/​ql/​test/​library-tests/​dataflow/​external-models/​test.cpp Adds value_type and scalar/pointer forwarding cases.
cpp/​ql/​test/​library-tests/​dataflow/​external-models/​flow.ext.yml Switches the container model from T to value_type.
cpp/​ql/​lib/​semmle/​code/​cpp/​dataflow/​ExternalFlow.qll Adds nested typedef resolution for constructed types.

Review effort: Balanced (auto)

@github-actions github-actions Bot added the C++ label Nov 4, 2026

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One little nit.


template<typename T>
struct Container {
using value_type = T;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Indentation looks a bit funny here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I agree. I'll fix in a subsequent PR

@MathiasVP
MathiasVP merged commit 5780a0d into github:main Nov 4, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants