[PATCH] D44559: [Sema] Wrong width of result of mul operation

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44559#1041002, @lebedev.ri wrote:

> In https://reviews.llvm.org/D44559#1041001, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D44559#1040928, @lebedev.ri wrote:
> >
> > > In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote:
> > >
> > > > I think we're correct not to warn here and that GCC/ICC are being noisy.  The existence of a temporary promotion to a wider type doesn't justify warning on arithmetic between two operands that are the same size as the ultimate result.  It is totally fair for users to think of this operation as being "closed" on the original type.
> > >
> > >
> > > Could you please clarify, are you saying that PR35409 <https://bugs.llvm.org/show_bug.cgi?id=35409> is not a bug, and clang should continue to not warn in those cases?
> >
> >
> > Correct.
> >
> > > If we would have "conversion sanitizer" <https://bugs.llvm.org/show_bug.cgi?id=21530>, detection of such problems would be easy, but without it, right now it is rather hard to detect such issues...
> >
> > What issue?  That arithmetic can overflow?
>
>
> But this isn't about overflow. Arithmetic overflow issues are nicely detectable with current UBSan (+`-fsanitize=integer`, for unsigned).
>  This is about the **lack** of detectable overflow - implicit cast to `int`, multiplication of `int`'s (which means no overflow actually happened),
>  and then implicit integer demotion (which changes the "integer's value", for which there is no sanitizer, yet).


The situation is effectively overflow in the original type of the operands.  That overflow does not have UB, and its formal moment is slightly different because it's tied to the conversion rather than the actual arithmetic, but, in the end, the observable effect is overflow.  From a language-design perspective, C would not be able to get away with implicitly promoting operands to a wider type if the result of the arithmetic after truncation weren't consistent with just doing the arithmetic in the narrower type.

> There is currently no way to detect this, at least in clang.
>  And it's not trivial to detect/debug such problems, which is exactly why a warning would be nice.

I think a "sanitizer" that tried to report when a integer value was used after undergoing an implicit conversion that changed its arithmetic value — but not when the value first passed through an explicit cast — would be a really interesting feature.

This patch, however, is contrary to the design of -Wconversion, which does attempt to avoid warning in cases where the user might reasonably see an operation as "really" being performed in its operand type.  If you want to argue that we should change the design of -Wconversion, that is a broader conversation that you should take to cfe-dev.


Repository:
  rC Clang

https://reviews.llvm.org/D44559