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.

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

C++: Test C++20 implicit array sizes. #16956

Merged
merged 3 commits into from
Jul 11, 2024
Merged

Conversation

calumgrant
Copy link
Contributor

Adds tests for C++ 20 implicit new array sizes. Also reviewed the QL libraries and did not find any use of NewArrayExpr.getExtent() that would benefit from using getArraySize() instead.

Implement NewArrayExpr.getArraySize()
@calumgrant calumgrant requested a review from a team as a code owner July 11, 2024 12:05
@github-actions github-actions bot added the C++ label Jul 11, 2024
@jketema
Copy link
Contributor

jketema commented Jul 11, 2024

Do we use getAllocatedType().(ArrayType).getArraySize() directly in places where we should actually be using the predicate introduced here?

@calumgrant
Copy link
Contributor Author

Do we use getAllocatedType().(ArrayType).getArraySize() directly in places where we should actually be using the predicate introduced here?

I can't find any.

@@ -0,0 +1,4 @@
---
Copy link
Contributor

@jketema jketema Jul 11, 2024

Choose a reason for hiding this comment

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

The change should go in cpp/ql/lib/change-notes (it's a library change, not a query change). That should also fix the error you're getting now.

@jketema
Copy link
Contributor

jketema commented Jul 11, 2024

Do we use getAllocatedType().(ArrayType).getArraySize() directly in places where we should actually be using the predicate introduced here?

I can't find any.

I'm finding cpp/leap-year/unsafe-array-for-days-of-the-year and cpp/bad-strncpy-size. However, it's better to create an issue for follow-up work then to possibly change those queries as part of this PR.

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.

@calumgrant calumgrant merged commit 24914ef into main Jul 11, 2024
15 checks passed
@calumgrant calumgrant deleted the calumgrant/cpp20-array-sizes branch July 11, 2024 16:44
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.

2 participants