Skip to content
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.

Sign up for GitHub

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

Jump to bottom

C++: Support C11 _Generic expressions #17138

Merged
merged 3 commits into from
Sep 3, 2024
Merged

C++: Support C11 _Generic expressions #17138

merged 3 commits into from
Sep 3, 2024

Conversation

jketema
Copy link
Contributor

@jketema jketema commented Aug 2, 2024

No description provided.

@github-actions github-actions bot added the C++ label Aug 2, 2024
@jketema jketema added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Aug 2, 2024
@jketema jketema force-pushed the generic branch 4 times, most recently from f56660a to eefc3cf Compare August 29, 2024 15:33
@jketema jketema marked this pull request as ready for review August 30, 2024 13:45
@jketema jketema requested a review from a team as a code owner August 30, 2024 13:45
jketema
jketema commented Aug 30, 2024
View reviewed changes
cpp/ql/test/library-tests/ir/ir/aliased_ir.expected Outdated
# 16| r16_2(glval<char[4]>) = StringConstant[int] :
# 16| r16_2(glval<char[4]>) = Constant[int] :
Copy link
Contributor Author

@jketema jketema Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is due to the the getType on the Value associated with the Generic being of ArrayType (only the underlying type is). This is inconsequential though.

Similar for the almost identical change below.

@jketema jketema requested a review from geoffw0 September 3, 2024 07:57
geoffw0
geoffw0 previously approved these changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

QL, tests and upgrade/downgrade scripts LGTM.

I trust the upgrade/downgrade scripts have been tested, and that the CI and DCA run looks good on the other PR?

It might be nice to have a test demonstrating data flow through generic expressions, but I won't hold this up for it.

cpp/ql/test/library-tests/ir/ir/PrintAST.expected Show resolved Hide resolved
cpp/ql/lib/semmle/code/cpp/exprs/Expr.qll Outdated Show resolved Hide resolved
@jketema
Copy link
Contributor Author

jketema commented Sep 3, 2024

I trust the upgrade/downgrade scripts have been tested, and that the CI and DCA run looks good on the other PR?

Correct.

It might be nice to have a test demonstrating data flow through generic expressions, but I won't hold this up for it.

The IR tests effectively demonstrate that the IR before and after are identical, so I don't think this adds much.

@jketema jketema requested a review from geoffw0 September 3, 2024 15:19
@jketema jketema merged commit 62766f6 into github:main Sep 3, 2024
11 of 16 checks passed
@jketema jketema deleted the generic branch September 3, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoffw0 geoffw0 geoffw0 approved these changes

Assignees
No one assigned
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jketema @geoffw0