Skip to content

C++: Add table that identifies C++ structured bindings #7928

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 6 commits into from
Feb 22, 2022

Conversation

jketema
Copy link
Contributor

@jketema jketema commented Feb 10, 2022

This introduces a new table is_structured_binding that identifies all variables that are declared as part of structured binding declarations. The naming follows the C++17 standard, which calls something like auto [x, y] = ... a structured binding declaration and the x and y in that declaration structured bindings. Not sure how clear this is, so I'm open to other name suggestions.

The table is exposed by adding a predicate to the Variable class. Not sure if this is the preferred approach, or whether subclassing Variable would be better.

@github-actions github-actions bot added the C++ label Feb 10, 2022
@jketema jketema force-pushed the structured-bindings-db-scheme branch 4 times, most recently from 424d631 to a4343ce Compare February 16, 2022 11:28
@jketema jketema force-pushed the structured-bindings-db-scheme branch from a4343ce to d4832b4 Compare February 17, 2022 10:59
@jketema jketema marked this pull request as ready for review February 21, 2022 09:01
@jketema jketema requested a review from a team as a code owner February 21, 2022 09:01
@MathiasVP
Copy link
Contributor

I think this PR also deserves a change note.

Accodring to the documentation in `Element.qll`, `underlyingElement` is
supposed to be used here and not `unresolveElement`.
@jketema
Copy link
Contributor Author

jketema commented Feb 21, 2022

I think this PR also deserves a change note.

Added a change note.

Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM! (I'll wait with merge until the internal PR has been approved)

@MathiasVP MathiasVP added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Feb 21, 2022
@jketema jketema removed the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Feb 22, 2022
@jketema jketema merged commit aecc17c into github:main Feb 22, 2022
@jketema jketema deleted the structured-bindings-db-scheme branch February 22, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants