[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for working on this!

Tried on a pet project <https://github.com/darktable-org/rawspeed>,
Two hits:

  $ ninja
  [16/376] Checking validity of cameras.xml
  /home/lebedevri/rawspeed/data/cameras.xml validates
  [100/376] Building CXX object src/CMakeFiles/rawspeed.dir/librawspeed/decoders/NefDecoder.cpp.o
  /home/lebedevri/rawspeed/build-Clang-SANITIZE/../src/librawspeed/decoders/NefDecoder.cpp:700:3: warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
    double g[6], bnd[2]={0,0}, r;
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  [127/376] Building CXX object fuzz/librawspeed/decompressors/HuffmanTable/CMakeFiles/HuffmanTableFuzzer-LUTVsLookup-BitPumpJPEG-NoFullDecode.dir/Dual.cpp.o
  /home/lebedevri/rawspeed/build-Clang-SANITIZE/../fuzz/librawspeed/decompressors/HuffmanTable/Dual.cpp:99:7: warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
        int decoded0, decoded1;
        ^~~~~~~~~~~~~~~~~~~~~~~

`clang-tidy`-applied fix-its:

  diff --git a/fuzz/librawspeed/decompressors/HuffmanTable/Dual.cpp b/fuzz/librawspeed/decompressors/HuffmanTable/Dual.cpp
  index b1f86a2a..af5b9164 100644
  --- a/fuzz/librawspeed/decompressors/HuffmanTable/Dual.cpp
  +++ b/fuzz/librawspeed/decompressors/HuffmanTable/Dual.cpp
  @@ -96,7 +96,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) {
       rawspeed::PUMP bits1(bs1);
   
       while (true) {
  -      int decoded0, decoded1;
  +      int decoded0;
  +      int decoded1;
   
         try {
           decoded0 = ht0.decode<decltype(bits0), FULLDECODE>(bits0);
  diff --git a/src/librawspeed/decoders/NefDecoder.cpp b/src/librawspeed/decoders/NefDecoder.cpp
  index c74d07a1..e0f841f8 100644
  --- a/src/librawspeed/decoders/NefDecoder.cpp
  +++ b/src/librawspeed/decoders/NefDecoder.cpp
  @@ -697,7 +697,9 @@ std::vector<ushort16> NefDecoder::gammaCurve(double pwr, double ts, int mode,
     std::vector<ushort16> curve(65536);
   
     int i;
  -  double g[6], bnd[2]={0,0}, r;
  +  double g[6];
  +  double bnd[2]={0,0};
  +  double r;
     g[0] = pwr;
     g[1] = ts;
     g[2] = g[3] = g[4] = 0;

The fixes are correct.

There are no **obvious** false-positives on that codebase, e.g. sonarcloud <https://sonarcloud.io/project/issues?id=rawspeed%3Adevelop&resolved=false&rules=cpp%3ASingleDeclarationPerStatement> finds one extra issue with struct fields, but no new issues that should have been caught by this check in it's current form.

So while i have not participated in the //code// review, this looks good to me.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949