Bug 80793 - three signed conversion warnings for the same expression
Summary: three signed conversion warnings for the same expression
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2017-05-17 01:45 UTC by Martin Sebor
Modified: 2021-10-19 15:59 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-02-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2017-05-17 01:45:50 UTC
In a review of an implementation of a new -Wenum-conversion warning (https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00142.html) I brought up a concern over the proliferation of warning options targeting closely related and sometimes even overlapping problems.  This is an extreme example of one such problem.  The following test case, extracted from the one on line 44 of gcc.dg/Wsign-conversion.c, triggers three distinct yet closely related warnings.  Also worth noting are the different ways of what boils down to the same problem (but that's a separate issue, tracked in bug 80731).

$ cat t.c && gcc -O2 -S -Wall -Wextra -Wconversion t.c
int f (int i)
{
  unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;
  return c;
}
t.c: In function ‘f’:
t.c:3:46: warning: signed and unsigned type in conditional expression [-Wsign-compare]
   unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;
                                              ^
t.c:3:46: warning: negative integer implicitly converted to unsigned type [-Wsign-conversion]
t.c:3:21: warning: conversion to ‘unsigned char’ alters ‘unsigned int’ constant value [-Wconversion]
   unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;
                     ^


In contrast, Clang issues the following:

$ clang -S -Wall -Wextra -Wpedantic -Weverything t.c
t.c:3:41: warning: operand of ? changes signedness: 'int' to 'unsigned char'
      [-Wsign-conversion]
  unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;
                ~        ~~~~~~~~~~~~~~~^~~
t.c:1:5: warning: no previous prototype for function 'f' [-Wmissing-prototypes]
int f (int i)
    ^
2 warnings generated.
Comment 1 Andrew Pinski 2017-05-17 03:52:58 UTC
I think GCC is correct in warning about all three.
There are all different issues really with the single statement.

The first warning is most accurate warning for all 4 warnings that are produced (that includes the clang warning).

The second warning is accurate also because of a?b:c where b and c are int and unsigned are converted to int and the signed constant was a negative value.

The third and final gcc warning is correct because -__SCHAR_MAX__ - 1 converted to unsigned int and then to unsigned char will truncate the value.

The warning that clang produces seems even worse and points to the wrong problem.  Fixing one of GCC warnings might be the wrong thing to do.  And I don't even understand what clang warning is trying to say because the lhs of the ?: was of type unsigned int and not unsigned char originally.
Comment 2 Martin Sebor 2017-05-17 17:34:15 UTC
The warnings aren't incorrect, there are just too many of them for what boils down to essentially the same problem.  It's true that the operands of the conditional expression have mixed signedness.  It's also true (and obvious given the first message) that one of the operands has to be converted to the type of the other.  And finally, it's true that the subsequent conversion of one of the two converted operands results in changing its value (if that operand happens to be the the result of the ternary expression).  But issuing three warnings for these three closely related problems doesn't make it or the solution clearer.  In fact, I think it can do the opposite and confuse the user (especially the last one).

But my general point is not about these three warnings in particular but rather that while GCC makes an effort to avoid diagnosing the same problem or even the same expression multiple times (TREE_NO_WARNING), the increasing number of specialized warning options tends to make the interactions between them too subtle to achieve the right balance (or even decide what the right balance is).
Comment 3 Martin Sebor 2017-05-19 15:14:23 UTC
See also bug 60083 for an example where the duplicate conversion warnings are a source of user complaints.
Comment 4 Eric Gallager 2018-02-20 10:58:16 UTC
(In reply to Martin Sebor from comment #0)
> In a review of an implementation of a new -Wenum-conversion warning
> (https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00142.html) 

That came from bug 78736, adding it to the related bugs
Comment 5 Manuel López-Ibáñez 2018-02-21 21:42:55 UTC
(In reply to Martin Sebor from comment #0)

There are several issues conflated here.

> t.c: In function ‘f’:
> t.c:3:46: warning: signed and unsigned type in conditional expression
> [-Wsign-compare]
>    unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;

This warning should be moved to where -Wsign-conversion is handled to avoid emitting both.

The reason we cannot simply remove -Wsign-compare is because -Wsign-conversion is not enabled by neither -Wall nor -Wextra, while -Wsign-compare is.

The reason -Wsign-conversion is not enabled by neither -Wall nor -Wextra is because it has annoying false positives like PR40752. Clang doesn't have those.
                                              
> t.c:3:46: warning: negative integer implicitly converted to unsigned type
> [-Wsign-conversion]

This is same warning as Clang gives, just less informative than Clang's. Printing the types should be easy.

> t.c:3:21: warning: conversion to ‘unsigned char’ alters ‘unsigned int’
> constant value [-Wconversion]
>    unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;

This warning is a bug in my opinion. Probably the patch in PR40752 silences it.
Comment 6 Eric Gallager 2018-11-21 04:39:27 UTC
(In reply to Manuel López-Ibáñez from comment #5)
> (In reply to Martin Sebor from comment #0)
> 
> There are several issues conflated here.
> 
> > t.c: In function ‘f’:
> > t.c:3:46: warning: signed and unsigned type in conditional expression
> > [-Wsign-compare]
> >    unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;
> 
> This warning should be moved to where -Wsign-conversion is handled to avoid
> emitting both.
> 
> The reason we cannot simply remove -Wsign-compare is because
> -Wsign-conversion is not enabled by neither -Wall nor -Wextra, while
> -Wsign-compare is.

Only in C++ though, and this bug is for plain C

> 
> The reason -Wsign-conversion is not enabled by neither -Wall nor -Wextra is
> because it has annoying false positives like PR40752. Clang doesn't have
> those.
>                                               
> > t.c:3:46: warning: negative integer implicitly converted to unsigned type
> > [-Wsign-conversion]
> 
> This is same warning as Clang gives, just less informative than Clang's.
> Printing the types should be easy.
> 
> > t.c:3:21: warning: conversion to ‘unsigned char’ alters ‘unsigned int’
> > constant value [-Wconversion]
> >    unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U;
> 
> This warning is a bug in my opinion. Probably the patch in PR40752 silences
> it.