[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 cfe-commits
cfe-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;
^
( )
[1m/home/nvellanki/scratch/arrow/cpp/src/arrow/vendored/datetime/date.h:3941:61: [0m[0;1;35mwarning: [0m[1moperator '?:' has lower precedence than '%'; '%' will be evaluated first [-Wparentheses][0m
static CONSTDATA std::uint64_t value = h * h * (exp % 2 ? 10 : 1);
[0;1;32m ~~~~~~~ ^
[0m[1m/home/nvellanki/scratch/arrow/cpp/src/arrow/vendored/datetime/date.h:3941:61: [0m[0;1;30mnote: [0mplace parentheses around the '%' expression to silence this warning[0m
static CONSTDATA std::uint64_t value = h * h * (exp % 2 ? 10 : 1);
[0;1;32m ^
[0m[0;32m ( )
[0m[1m/home/nvellanki/scratch/arrow/cpp/src/arrow/vendored/datetime/date.h:3941:61: [0m[0;1;30mnote: [0mplace parentheses around the '?:' expression to evaluate it first[0m
static CONSTDATA std::uint64_t value = h * h * (exp % 2 ? 10 : 1);
[0;1;32m ^
[0m[0;32m ( )
/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 cfe-commits
mailing list