[cfe-users] -Wconversion does not produce warnings when it should

[David Blaikie via cfe-users]

On Mon, Nov 16, 2020 at 4:49 PM Sven Köhler via cfe-users
<cfe-users at lists.llvm.org> wrote:
>
> Am 16.11.20 um 21:49 schrieb David Blaikie via cfe-users:
> > On Mon, Nov 16, 2020 at 12:36 PM Sven Köhler via cfe-users
> > <cfe-users at lists.llvm.org> wrote:
> >>
> >> Can you elaborate what is happening here?
> >
> > I believe/would guess the goal is to reduce false positives (where a
> > false positive is defined as "warning on code where the code does what
> > the author intended").
> >
> > In the taste of testb4 this code is "as good as" the unsigned int
> > equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
> > - wraparound occurs in both cases, so the code perhaps isn't too
> > surprising/the semantics of widening/narrowing conversions don't
> > really impact the behavior.
>
> I do appreciate your explanation. But I do not appreciate the compiler
> guessing that wrap around is OK.
>
> The C standard could also be interpreted as "wrap around only occurs
> with types of rank equal to (unsigned) int and above" as all other types
> are promoted to int prior to any arithmetic.
> (My guess for the origin of this promotion would be that int is the
> natural word size of the underlying CPU.)
>
> With most modern architectures, int is 32 bit which is a reasonable size
> for wraparounds. Unfortunately, int doesn't always have a fixed size.
>
> > The uint16 version of the code and the
> > unsigned int version of the code would appear to the user to work in
> > the same way.
>
> I would appreciate that only if my understanding of the C language was
> that arithmetic on two uint16_t operands is performed with uint16_t
> precision. But that is simply false. It is performed with 32 bit
> precision (if int is 32 bit).
>
> > But, yeah, once the expressions are sufficiently complicated, as in
> > testa4, clang's attempt to silence benign cases of the warning gives
> > up and you get warnings. It's best-effort sort of stuff.
>
> I would vote for -Wconversion=strict or -Wimplicit-int-conversion=strict
> where we get a warning for every int conversion that loses precision.
>
> I would be very happy if there was a clang option to generate warnings
> in such cases.

In general, Clang warnings are designed to have very low false
positive rates so they're more likely to be enabled by users even on
existing codebases (eg: where the work to cleanup existing violations
of the warning is fruitful in terms of fixing bugs rather than only
suppressing a warning where it doesn't indicate any bug was present) -
so it may not be especially desirable to have such a strict form of
the warning in clang. Not to say "certainly not", but that it might be
a bit trickier to motivate/justify.

Might be that something more like a clang-tidy check could be more
suited to your needs.

- Dave