[libcxx-commits] [PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

NagaChaitanya Vellanki via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue May 23 18:28:15 PDT 2023


chaitanyav added a comment.

I used the patch to compile LLVM, apache/arrow, apache/trafficserver, folly, tensorstore, protobuf. I did not see any cases with pointer arithmetic in these repos.  I see there is some value for the patch in terms of readability (will be helpful to someone who is new to a codebase). I have uploaded the make logs for the projects. I will attach the LLVM log in a bit since am building it from scratch. It's taking a while.
F27618552: apache_arrow_cpp_out.txt <https://reviews.llvm.org/F27618552>

F27618556: apache_trafficserver_out.txt <https://reviews.llvm.org/F27618556>

F27618559: protobuf_out.txt <https://reviews.llvm.org/F27618559>

F27618563: folly_out.txt <https://reviews.llvm.org/F27618563>

F27618565: tensorstore_out.txt <https://reviews.llvm.org/F27618565>

Some examples

  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3721:49: warning: operator '?:' has lower precedence than '%'; '%' will be evaluated first [-Wparentheses]
    unsigned NumLeftovers = OrigNumElts % NumElts ? 1 : 0;
                            ~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3721:49: note: place parentheses around the '%' expression to silence this warning
    unsigned NumLeftovers = OrigNumElts % NumElts ? 1 : 0;
                                                  ^
                            (                    )
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3721:49: note: place parentheses around the '?:' expression to evaluate it first
    unsigned NumLeftovers = OrigNumElts % NumElts ? 1 : 0;
                                                  ^
                                          (              )
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3775:49: warning: operator '?:' has lower precedence than '%'; '%' will be evaluated first [-Wparentheses]
    unsigned NumLeftovers = OrigNumElts % NumElts ? 1 : 0;
                            ~~~~~~~~~~~~~~~~~~~~~ ^



  /home/nvellanki/scratch/tensorstore/build/_deps/zlib-src/crc32.c:572:19: warning: operator '?:' has lower precedence than '&'; '&' will be evaluated first [-Wbitwise-conditional-parentheses]
          b = b & 1 ? (b >> 1) ^ POLY : b >> 1;
              ~~~~~ ^
  /home/nvellanki/scratch/tensorstore/build/_deps/zlib-src/crc32.c:572:19: note: place parentheses around the '&' expression to silence this warning
          b = b & 1 ? (b >> 1) ^ POLY : b >> 1;
                    ^
              (    )
  /home/nvellanki/scratch/tensorstore/build/_deps/zlib-src/crc32.c:572:19: note: place parentheses around the '?:' expression to evaluate it first
          b = b & 1 ? (b >> 1) ^ POLY : b >> 1;
                    ^
                  (                           )



  /home/nvellanki/scratch/arrow/cpp/src/arrow/vendored/datetime/date.h:3941:61: warning: operator '?:' has lower precedence than '%'; '%' will be evaluated first [-Wparentheses]
      static CONSTDATA std::uint64_t value = h * h * (exp % 2 ? 10 : 1);
                                                      ~~~~~~~ ^
  /home/nvellanki/scratch/arrow/cpp/src/arrow/vendored/datetime/date.h:3941:61: note: place parentheses around the '%' expression to silence this warning
      static CONSTDATA std::uint64_t value = h * h * (exp % 2 ? 10 : 1);
                                                              ^
                                                      (      )
  /home/nvellanki/scratch/arrow/cpp/src/arrow/vendored/datetime/date.h:3941:61: note: place parentheses around the '?:' expression to evaluate it first
      static CONSTDATA std::uint64_t value = h * h * (exp % 2 ? 10 : 1);
                                                              ^
                                                            (         )



  /home/nvellanki/scratch/trafficserver/include/tscore/SimpleTokenizer.h:145:47: warning: operator '?:' has lower precedence than '&'; '&' will be evaluated first [-Wbitwise-conditional-parentheses]
      _data   = (_mode & OVERWRITE_INPUT_STRING ? const_cast<char *>(s) : ats_strdup(s));
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/trafficserver/include/tscore/SimpleTokenizer.h:145:47: note: place parentheses around the '&' expression to silence this warning
      _data   = (_mode & OVERWRITE_INPUT_STRING ? const_cast<char *>(s) : ats_strdup(s));
                                                ^
                 (                             )
  /home/nvellanki/scratch/trafficserver/include/tscore/SimpleTokenizer.h:145:47: note: place parentheses around the '?:' expression to evaluate it first
      _data   = (_mode & OVERWRITE_INPUT_STRING ? const_cast<char *>(s) : ats_strdup(s));
                         ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~



  /home/nvellanki/scratch/protobuf/src/google/protobuf/map.h:773:47: warning: operator '?:' has lower precedence than '&'; '&' will be evaluated first [-Wbitwise-conditional-parentheses]
                        bits & kUseDestructFunc ? DestroyNode<Node> : nullptr};
                        ~~~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/protobuf/src/google/protobuf/map.h:773:47: note: place parentheses around the '&' expression to silence this warning
                        bits & kUseDestructFunc ? DestroyNode<Node> : nullptr};
                                                ^
                        (                      )
  /home/nvellanki/scratch/protobuf/src/google/protobuf/map.h:773:47: note: place parentheses around the '?:' expression to evaluate it first
                        bits & kUseDestructFunc ? DestroyNode<Node> : nullptr};
                                                ^



  In file included from /home/nvellanki/scratch/folly/folly/lang/Bits.h:63:
  /home/nvellanki/scratch/folly/folly/ConstexprMath.h:139:20: warning: operator '?:' has lower precedence than '%'; '%' will be evaluated first [-Wparentheses]
            (exp % 2 ? base : T(1));
             ~~~~~~~ ^
  /home/nvellanki/scratch/folly/folly/ConstexprMath.h:139:20: note: place parentheses around the '%' expression to silence this warning
            (exp % 2 ? base : T(1));
                     ^
             (      )
  /home/nvellanki/scratch/folly/folly/ConstexprMath.h:139:20: note: place parentheses around the '?:' expression to evaluate it first
            (exp % 2 ? base : T(1));
                     ^

  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:24:36: warning: operator '?:' has lower precedence than '&'; '&' will be evaluated first [-Wbitwise-conditional-parentheses]
    return Flags & MACRO_OFFSET_SIZE ? DWARF64 : DWARF32;
           ~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:24:36: note: place parentheses around the '&' expression to silence this warning
    return Flags & MACRO_OFFSET_SIZE ? DWARF64 : DWARF32;
                                     ^
           (                        )
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:24:36: note: place parentheses around the '?:' expression to evaluate it first
    return Flags & MACRO_OFFSET_SIZE ? DWARF64 : DWARF32;
                                     ^
                   (                                    )



  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3381:49: warning: operator '?:' has lower precedence than '&'; '&' will be evaluated first [-Wbitwise-conditional-parentheses]
              : Flags & MachineMemOperand::MOLoad ? "from" : "into";
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3381:49: note: place parentheses around the '&' expression to silence this warning
              : Flags & MachineMemOperand::MOLoad ? "from" : "into";
                                                  ^
                (                                )
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3381:49: note: place parentheses around the '?:' expression to evaluate it first
              : Flags & MachineMemOperand::MOLoad ? "from" : "into";
                                                  ^



                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2377:5: note: place parentheses around the '&' expression to silence this warning
      GENBOOLCOMMENT(", ", VRData, HasVMXInstruction);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2138:65: note: expanded from macro 'GENBOOLCOMMENT'
    CommentOS << (Prefix) << ((V) & (TracebackTable::Field##Mask) ? "+" : "-")   \
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2377:5: note: place parentheses around the '?:' expression to evaluate it first
      GENBOOLCOMMENT(", ", VRData, HasVMXInstruction);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /home/nvellanki/scratch/temp_llvm/llvm-project/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2138:65: note: expanded from macro 'GENBOOLCOMMENT'
    CommentOS << (Prefix) << ((V) & (TracebackTable::Field##Mask) ? "+" : "-")   \
                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147844/new/

https://reviews.llvm.org/D147844



More information about the libcxx-commits mailing list