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

clang-tidy: cppcoreguidelines-pro-bounds-constant-array-index #121353

Open
root-kidik opened this issue Dec 30, 2024 · 1 comment
Open

clang-tidy: cppcoreguidelines-pro-bounds-constant-array-index #121353

root-kidik opened this issue Dec 30, 2024 · 1 comment
Labels
clang-tidy false-positive Warning fires when it should not

Comments

@root-kidik
Copy link

root-kidik commented Dec 30, 2024

Description

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
crc = (crc >> kProtocolShiftCount) ^ kCrc32Tab[crc & constants::kPreamble];

Why do we need to disable this check here (don't use kCrc32Tab.at())?

  1. constants::kPreamble == 255
  2. crc == any number & constants::kPreamble <= 255 - always, the laws of mathematics have not been repealed
  3. kCrc32Tab.size() == 256, therefore the maximum index == 255
    Conclusion: therefore this expression will always be within the array boundaries

Proposal

Improve clang-tidy so that it understands such patterns and does not give a warning

@5chmidti
Copy link
Contributor

Flag any indexing expression on an expression or variable of array type (either static array or std::array) where the indexer is not a compile-time constant expression with a value between 0 and the upper bound of the array.

The check currently does nothing to infer something about the index, if it is not a compile-time constant.

E.g. (as per the example above):

void bar(std::array<int, 5> arr, int X) {
    arr[X&4]; // due to 'X', the index is not compile-time known -> FP warning
}

https://godbolt.org/z/9rbvo9bj7

We could relatively easily check for such a pattern, and compare the constant in the & with the array size.


The question is what other kinds of patterns should be ignored. Something like a < N ? a : b where b < N could be one, or a parent if statement with a < N as the condition, and arr[a] in the body,... This quickly becomes a dataflow problem, and I think we should only ignore the a & C case for now.

@5chmidti 5chmidti added the false-positive Warning fires when it should not label Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang-tidy false-positive Warning fires when it should not
Projects
None yet
Development

No branches or pull requests

2 participants