-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Description
Hello everybody,
I had some issue while implementing rules for integer usage in clang-tidy and
would like to ask for some clarification/discussion. The rules in discussion
are ES.100-107 in general but ES.100 and ES.102 in particular.
General
- How is the usage of literals (that are by default signed) handled? Its very
common to not use literals corresponding to the actual wanted integer type.
Should that be allowed as an exception? - What about comparison operators? Here again literals should be treated
differently then variable comparions. Especially checking forx > 0
might
be common. Ifx
is unsigned that can be safely replace withx != 0
but is
that reasonable?
Mixed arithmetic
ES.100 forbids the usage of mixed arithmetic and that is ok.
The integer promotion rules for char
and short
do undermine the rule
somewhat. (http://en.cppreference.com/w/cpp/language/implicit_conversion)
Note: I am no language lawer. Please correct missunderstandings and clarify :)
Adding short
will promote that variable to an int
or unsigned int
that
does the actual calculation. If one then mixes integer lengths and signedness
in calculations confusion on what happens might occur.
unsigned char UC1 = 42;
unsigned char UC2 = 42;
unsigned int UI = 42;
// Implicitly mixed arithmetic.
unsigned int Result = UI * (UC1 + UC2);
Trusting in clangs AST (UC1 + UC2)
results in type int
leading to mixed
arithmetic.
How to avoid the issue
- Allow this case because the arithmetic is not mixed as written.
- The Guidelines could forbid arithmetic with
char
andshort
, especially
because the calculations occur withint
and the result is then truncated on
assignment(correct?). This would relate togsl::narrow_cast
.
short
andchar
are then reserved as a representation for a discrete
number of values (ASCII, other coding) but not supposed to be used for
calculations. Increments and decrements might get an exception. - The Guidelines could forbid to mix integer lengths in arithmetic too. That
would not solve the issue with mixing and should be another rule.
It still might be reasonable.
Use signed types for arithmetic
The enforcement is pretty much checking for mixed arithmetic, because pure
unsigned is allowed as exception. Here comes the question about the usage of
literals.
Cases that could be reasonably allowed as exceptions:
unsigned int i = 0;
i += 1; // Mixed as written
Suggestion
The guidelines should allow the use of positive signed literals for mixed
arithmetic. Furthermore initializing unsigned types should be allowed with
positive signed literals, too.
I would be happy to get some clarification on these issues. They can be split
into multiple issues in the tracker too.
If changes to the guidelines are necessary I am willing to make a PR to these
sections.
The codereview for the current state is here: https://reviews.llvm.org/D40854
Thank you very much :)