Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

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

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR38510
Status NEW
Importance P enhancement
Reported by Urmish Shah (s.urmish@gmail.com)
Reported on 2018-08-09 19:58:43 -0700
Last modified on 2019-09-11 10:29:12 -0700
Version unspecified
Hardware PC Linux
CC alexfh@google.com, development@jonas-toth.eu, djasper@google.com, eugene.zelenko@gmail.com, klimek@google.com, matthias.gehre@ohb.de, quolyk@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
#include <gsl/gsl_util>
#include <iostream>
#include <string>

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 <iostream>
#include <string>

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
Quuxplusone commented 6 years ago

Hi Urmish,

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

Best Jonas

Quuxplusone commented 5 years ago

Any updates on this? Does this require huge changes?

Quuxplusone 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 :/

Quuxplusone commented 5 years ago

https://reviews.llvm.org/D56661

Quuxplusone commented 5 years ago

Is this fixed? Please close the ticket. Thanks!

Quuxplusone commented 5 years ago

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

Quuxplusone commented 5 years ago

Can someone please review this and merge it?