8000 C++: Add support for getting literals in using declarations by IdrissRio · Pull Request #19603 · github/codeql · GitHub
[go: up one dir, main page]

Skip to content

C++: Add support for getting literals in using declarations #19603

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

Merged
merged 3 commits into from
Jun 3, 2025

Conversation

IdrissRio
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the C++ label May 28, 2025
@IdrissRio IdrissRio added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label May 28, 2025
@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from ff24105 to 20edf34 Compare May 28, 2025 06:18
@IdrissRio IdrissRio changed the title C++: Add support for retrieving referenced literals in using declarations C++: Add support for getting literals in using declarations May 28, 2025
@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from 20edf34 to b322892 Compare May 28, 2025 12:32
@IdrissRio IdrissRio marked this pull request as ready for review May 28, 2025 15:47
@Copilot Copilot AI review requested due to automatic review settings May 28, 2025 15:47
@IdrissRio IdrissRio requested a review from a team as a code owner May 28, 2025 15:47
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds the ability to retrieve the specific member referenced by a using declaration in a templated base class and updates toString() to fall back to this new API.

  • Introduces getReferencedMember() in UsingDeclarationEntry.
  • Modifies toString() to use getDeclaration() first or getReferencedMember() as a fallback.
  • Documents the feature in a new change-notes entry.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cpp/ql/lib/semmle/code/cpp/Namespace.qll Added getReferencedMember() and updated toString()
cpp/ql/lib/change-notes/2025-05-28-using-template.md Added feature note for getReferencedMember
Comments suppressed due to low confidence (2)

cpp/ql/lib/semmle/code/cpp/Namespace.qll:191

  • The use of or between two assignment statements here is invalid syntax and won't produce the intended fallback behavior. Consider computing both descriptions first or use a conditional expression to assign a single result value.
result = "using " + this.getDeclaration().getDescription() or

cpp/ql/lib/semmle/code/cpp/Namespace.qll:192

  • getReferencedMember() returns an Element, not a string. You need to call its .getDescription() (or equivalent) to produce a textual representation.
result = "using " + this.getReferencedMember()

@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch 2 times, most recently from 237e296 to d6fbb31 Compare June 2, 2025 11:41
Comment on lines 183 to 186
* class A : public T {
* using T::foo; // `foo` is the member referenced by this using declaration
* };
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it actually relevant that T is a base class of A, or would the following also yield a result:

template <typename T>
class A {
   using T::foo;
};

@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from d6fbb31 to 79df7b9 Compare June 3, 2025 10:59
@IdrissRio IdrissRio requested a review from a team as a code owner June 3, 2025 10:59
@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jun 3, 2025
@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from 79df7b9 to 770708f Compare June 3, 2025 11:15
@github-actions github-actions bot removed the Rust Pull requests that update Rust code label Jun 3, 2025
@IdrissRio IdrissRio removed the request for review from a team June 3, 2025 11:16
@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from 770708f to a0dbe0e Compare June 3, 2025 12:19
Copy link
Contributor
@jketema jketema left a comment

Choose a reason for hiding this comment

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

Some small tweaks of the change note and QLDoc, otherwise LGTM.

Comment on lines 178 to 179
* Returns the member referenced by this using declaration when it refers
* to a member of a template type parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to make it a bit more uniform with the getDeclaration QLDoc, and using the word "depends", which is what the standard uses:

Suggested change
* Returns the member referenced by this using declaration when it refers
* to a member of a template type parameter.
* Gets the member that is referenced by this using declaration, where the member depends on a
* type template parameter.

* ```
* template <typename T>
* class A {
* using T::foo; // `foo` is the member referenced by this using declaration
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just omit the comment here, as you effectively explain this below.

Suggested change
* using T::foo; // `foo` is the member referenced by this using declaration
* using T::m;

Comment on lines 188 to 189
* `getReferencedMember()` returns the member `foo` declared within the
* template type parameter `T`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `getReferencedMember()` returns the member `foo` declared within the
* template type parameter `T`.
* Here, `getReferencedMember()` yields the member `m` of `T`. Observe that, as `T` is not instantiated, `m` represented by a `Literal` and not a `Declaration`.

---
category: feature
---
* Added `getReferencedMember` to extract the member referenced by a `using` declaration in a templated base class.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Added `getReferencedMember` to extract the member referenced by a `using` declaration in a templated base class.
* Added a predicate `getReferencedMember` to `UsingDeclarationEntry`, which yields a member depending on a type template parameter.

@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from a0dbe0e to 39469a2 Compare June 3, 2025 13:42
@IdrissRio IdrissRio force-pushed the idrissrio/comments-using branch from 39469a2 to 10fb806 Compare June 3, 2025 14:04
Copy link
Contributor
@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM

@IdrissRio IdrissRio merged commit 8fe2699 into main Jun 3, 2025
13 of 15 checks passed
@IdrissRio IdrissRio deleted the idrissrio/comments-using branch June 3, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0