isocpp / isocpp/CppCoreGuidelines

Question relating the usage of Integer arithmetic

Open
#1,120 2 comments 0 reactions 1 assignee View on GitHub

@BjarneStroustrup is already working on this.

Since Jan 22, 2018.

Dominant language
CSS
Stars
45.3k
Forks
5.6k
PR merge metrics
No merged PRs in 30d

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 for x > 0 might
    be common. If x is unsigned that can be safely replace with x != 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 and short, especially
    because the calculations occur with int and the result is then truncated on
    assignment(correct?). This would relate to gsl::narrow_cast.
    short and char 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 :)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.