C++ Logo

std-proposals

Advanced search

Re: [std-proposals] Integer overflow arithmetic

From: Jan Schultke <janschultke_at_[hidden]>
Date: Thu, 15 Feb 2024 23:10:14 +0100
I agree that C++ should have widening/overflow functions, and at least
LEWG-I seems to think of it favorably as well. The devil lies entirely
in the details though.

Firstly, I strongly oppose returning std::tuple and I suspect the
committee will see it the same way. The style of returnings pairs or
tuples has been phased out, and I don't think it's going to make a
comeback with this paper. It degrades code quality too much not to
have meaningful names and makes the potential for misuse too great.

> template<class T>
> [[nodiscard]] constexpr std::tuple<T, T> mul_wide(T, T) noexcept;

How is the user supposed to know which T is meant to store the high
bits, and which one the low bits? The potential for mistakes is too
great. It doesn't help that you subsequently do std::get<0> to access
one of the members. Or alternatively, you use structured bindings,
however, both of the following declarations are valid:

> auto [lo, hi] = std::mul_wide(...);
> auto [hi, lo] = std::mul_wide(...);

std::tuple is suboptimal for performance-critical applications because
it has bad ABI in libstdc++. Even trivially copyable types such as
std::tuple<int, int> are not passed by register.

The use of the term "overflow" for "would_cast_overflow" does not make
sense, at least not in C++. Type conversions in C++ never overflow,
but they may truncate non-zero bits, or be lossy. This should probably
be "is_cast_lossless" or something along those lines.

Furthermore, [[nodiscard]] does not belong in the wording. It's
debatable whether the standard should mandate [[nodiscard]] on
functions at all. This is not one of the compelling cases like
std::vector::empty where [[nodiscard]] avoids ambiguity, so I doubt
this would pass through the committee. The proposal for functions in
the <bit> header similarly had [[nodiscard]] attributes, which were
removed during later revisions.

The wording also uses CTAD in the return type (std::tuple with no
template arguments) which is probably a mistake.

The wording needs work in general; in particular, the Returns
paragraphs are under-specified. Terms like "the low bits" aren't
well-defined within the C++ standard; they're domain-specific. At the
very least, you need to say how many bits there are. For example, you
could word it as "the least significant N bits". "flag" also doesn't
have meaning within the standard. The standard typically just says
"true if the result is not representable ...".

> Constraints: R and T are signed or unsigned integer types ([basic.fundamental]).

This can be simplified to just saying they are integer types.

You can also improve the wording of is_div_defined. You could just use
a single expression involving the conditional operator, which is
better than mixing prose and multiple boolean expressions. However, I
would simply phrase it as:

> Returns: true if dividend / divisor is defined.

You can omit the "false otherwise"; to my knowledge, it's not
consistently used in the standard and not seen as necessary. I could
be wrong though and the standard is just inconsistent.


Some editorial notes:

> "application specific"

This should be hyphenated.

> Papers such as [P0543] have already been accepted and is on track for C++26.

This is mixing singular and plural.

> In addition there's no cpu instruction that would give the right result in the degenerate case, a library implementer

CPU should be capitalized and the comma in here should be a semicolon or period.

Received on 2024-02-15 22:10:27