rjmccall added a comment.
In https://reviews.llvm.org/D44559#1049049, @avt77 wrote:
> > If that operation overflows, so be it — we're not going to warn about the
> > potential for overflow every time the user adds two ints, and by the same
> > token, it doesn't make any sense to warn about every time the user adds two
> > shorts just because the language made this otherwise-unimportant technical
> > decision to do the arithmetic in a wider type.
>
> There is one comment only: this patch changes the 'mul' operation only and
> initially the change was done for X86 only because it clearly says about
> "doubling" of mul result width.
Yes, I understand that addition was not changed by the patch. I do not think
that using different logic for multiplication is appropriate just because
multiplication produces bigger values. It is reasonable to multiply two 8-bit
numbers and store the result in an 8-bit variable without a warning because the
net effect is just doing a "closed" multiplication on an 8-bit type, which is a
reasonable operation to want to do. GCC is too noisy here.
There is nothing x86-specific about the supposed problem here, and using
target-specific logic was always clearly inappropriate.
> And another comment: currently we have warning here:
>
> int c = a * b;
> T d1 = c; // expected-warning{{implicit conversion loses integer precision:
> 'int' to 'char'}}
>
>
> but we don't have it here:
>
> T d2 = a * b; // when T is char
>
> It's a potential problem from my point of view.
All of these warnings are single-line analyses, which is unlikely to change
absent massive architectural improvements to clang. It has nothing to do with
there being a multiplication; we would give you that exact same warning if you
wrote `int c = a;`.
In https://reviews.llvm.org/D44559#1049057, @lebedev.ri wrote:
> In https://reviews.llvm.org/D44559#1049049, @avt77 wrote:
>
> > ...
> > But I don't mind to close the review - and the corresponding bug of course.
>
>
> I disagree with closing the bug.
> It's a real issue that is not detected/diagnosed by anything in clang
> (diagnostic, sanitizer).
>
> It seems we are in a deadlock here :)
I don't have any special authority here besides having designed and implemented
the current diagnostic. You are welcome to start a thread on cfe-dev to gather
support for changing the design of -Wconversion to always warn about the
potential for overflow in sub-int multiplication. I will argue against you
there, like I have here, but if there's consensus that we should warn about
this, I'll abide by the community's judgment.
https://reviews.llvm.org/D44559
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
