llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.58k forks source link

Clang-tidy generates incorrect array name while generating a fix for a cppcoreguidelines-pro-bounds-constant-array-index warning #37858

Open llvmbot opened 6 years ago

llvmbot commented 6 years ago
Bugzilla Link 38510
Version unspecified
OS Linux
Reporter LLVM Bugzilla Contributor
CC @JonasToth,@EugeneZelenko

Extended Description

include <gsl/gsl_util>

include

include

int main() {

int arr[1] = {1};

auto index = 0; std::cout << arr[index] << std::endl;

return 0; }


On running clang-tidy check, I get the following:

[archlinux@thinkpad fizzbuzz]$ clang-tidy clang_test_case.cpp -checks=-.,cppcoreguidelines-pro-bounds-constant-array-index -config="{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader, value: gsl/gsl_util}]}"
5 warnings generated. /home/archlinux/coding/c_projects/fizzbuzz/clang_test_case.cpp:10:16: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] std::cout << arr[index] << std::endl; ^ Suppressed 4 warnings (4 in non-user code). Use -header-filter=.
to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. [archlinux@thinkpad fizzbuzz]$


On trying to generate a fix using clang-tidy, I get an incorrect array name (a instead of arr being used in the piece of code above)

[archlinux@thinkpad fizzbuzz]$ clang-tidy clang_test_case.cpp -checks=-.,cppcoreguidelines-pro-bounds-constant-array-index -config="{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader, value: gsl/gsl_util}]}" --fix 5 warnings generated. /home/archlinux/coding/c_projects/fizzbuzz/clang_test_case.cpp:10:16: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] std::cout << arr[index] << std::endl; ^ /home/archlinux/coding/c_projects/fizzbuzz/clang_test_case.cpp:10:16: note: FIX-IT applied suggested code changes /home/archlinux/coding/c_projects/fizzbuzz/clang_test_case.cpp:10:17: note: FIX-IT applied suggested code changes std::cout << arr[index] << std::endl; ^ /home/archlinux/coding/c_projects/fizzbuzz/clang_test_case.cpp:10:25: note: FIX-IT applied suggested code changes std::cout << arr[index] << std::endl; ^ clang-tidy applied 3 of 3 suggested fixes. Suppressed 4 warnings (4 in non-user code). Use -header-filter=. to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. [archlinux@thinkpad fizzbuzz]$


And the 'fix'ed code looks like:

include <gsl/gsl_util>

include

include

int main() {

int arr[1] = {1};

auto index = 0; std::cout << gsl::at(a, index) << std::endl;

return 0; }

Is this a legitimate bug?

[archlinux@thinkpad fizzbuzz]$ clang-tidy --version LLVM (http://llvm.org/): LLVM version 6.0.1 Optimized build. Default target: x86_64-pc-linux-gnu Host CPU: sandybridge

llvmbot commented 5 years ago

Can someone please review this and merge it?

EugeneZelenko commented 5 years ago

Review is still open, so patch was not propagated to code base.

llvmbot commented 5 years ago

Is this fixed? Please close the ticket. Thanks!

llvmbot commented 5 years ago

https://reviews.llvm.org/D56661

JonasToth commented 5 years ago

Not from my side. I am not sure what it takes code-wise, but it requires developer time which is spare :)

If you want and are a bit familiar with C++ you can take a look your self! I am happy to help with specific questions, otherwise it might take a bit until someone has time to look at it :/

llvmbot commented 5 years ago

Any updates on this? Does this require huge changes?

JonasToth commented 6 years ago

Hi Urmish,

thank you for reporting this bug. I can reproduce it.

Best Jonas

sean-mcmanus commented 2 years ago

I hit this bug with clang-tidy 14.0.0.

surmish commented 9 months ago

https://reviews.llvm.org/D56661

Did this patch make it to the release?

Endilll commented 9 months ago

https://reviews.llvm.org/D56661

Did this patch make it to the release?

It's still Needs Review, so it hasn't even landed on main branch to be included in the upcoming release.