Handle -Wsign-conversion in conversion_warning.

It seemed strange to me to warn about sign conversion in
unsafe_conversion_p, when other warnings are in conversion_warning, and the
latter function is the only place that asks the former function to warn.
This change is also necessary for my -Warith-conversion patch.

	* c-common.c (unsafe_conversion_p): Don't warn, return UNSAFE_SIGN.
	* c-warn.c (conversion_warning): Warn about UNSAFE_SIGN.

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index fdddb98..fbbc924 100644 (file)
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-21  Jason Merrill  <jason@redhat.com>
+
+       * c-common.c (unsafe_conversion_p): Don't warn, return UNSAFE_SIGN.
+       * c-warn.c (conversion_warning): Warn about UNSAFE_SIGN.
+
 2020-01-20  Nathan Sidwell  <nathan@acm.org>
 
        PR preprocessor/80005

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 82c0894..4a7f3a5 100644 (file)
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -1312,9 +1312,9 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
        * EXPR is not a constant of integer type which cannot be
          exactly converted to real type.
 
-   Function allows conversions between types of different signedness and
-   can return SAFE_CONVERSION (zero) in that case.  Function can produce
-   signedness warnings if PRODUCE_WARNS is true.
+   Function allows conversions between types of different signedness if
+   CHECK_SIGN is false and can return SAFE_CONVERSION (zero) in that
+   case.  Function can return UNSAFE_SIGN if CHECK_SIGN is true.
 
    RESULT, when non-null is the result of the conversion.  When constant
    it is included in the text of diagnostics.
@@ -1325,14 +1325,11 @@ int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
 
 enum conversion_safety
 unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
-                    bool produce_warns)
+                    bool check_sign)
 {
   enum conversion_safety give_warning = SAFE_CONVERSION; /* is 0 or false */
   tree expr_type = TREE_TYPE (expr);
 
-  bool cstresult = (result
-                   && TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant);
-
   loc = expansion_point_location_if_in_system_header (loc);
 
   expr = fold_for_warn (expr);
@@ -1360,32 +1357,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
          if (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)
              && tree_int_cst_sgn (expr) < 0)
            {
-             if (produce_warns)
-               {
-                 if (cstresult)
-                   warning_at (loc, OPT_Wsign_conversion,
-                               "unsigned conversion from %qT to %qT "
-                               "changes value from %qE to %qE",
-                               expr_type, type, expr, result);
-                 else
-                   warning_at (loc, OPT_Wsign_conversion,
-                               "unsigned conversion from %qT to %qT "
-                               "changes the value of %qE",
-                               expr_type, type, expr);
-               }
+             if (check_sign)
+               give_warning = UNSAFE_SIGN;
            }
          else if (!TYPE_UNSIGNED (type) && TYPE_UNSIGNED (expr_type))
            {
-             if (cstresult)
-               warning_at (loc, OPT_Wsign_conversion,
-                           "signed conversion from %qT to %qT changes "
-                           "value from %qE to %qE",
-                           expr_type, type, expr, result);
-             else
-               warning_at (loc, OPT_Wsign_conversion,
-                           "signed conversion from %qT to %qT changes "
-                           "the value of %qE",
-                           expr_type, type, expr);
+             if (check_sign)
+               give_warning = UNSAFE_SIGN;
            }
          else
            give_warning = UNSAFE_OTHER;
@@ -1425,7 +1403,7 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
           is a constant, it's type is not used in text of generated warnings
           (otherwise they could sound misleading).  */
        return unsafe_conversion_p (loc, type, TREE_REALPART (expr), result,
-                                   produce_warns);
+                                   check_sign);
       /* Conversion from complex constant with non-zero imaginary part.  */
       else
        {
@@ -1433,21 +1411,11 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
             Perform checks for both real and imaginary parts.  */
          if (TREE_CODE (type) == COMPLEX_TYPE)
            {
-             /* Unfortunately, produce_warns must be false in two subsequent
-                calls of unsafe_conversion_p, because otherwise we could
-                produce strange "double" warnings, if both real and imaginary
-                parts have conversion problems related to signedness.
-
-                For example:
-                int32_t _Complex a = 0x80000000 + 0x80000000i;
-
-                Possible solution: add a separate function for checking
-                constants and combine result of two calls appropriately.  */
              enum conversion_safety re_safety =
-                 unsafe_conversion_p (loc, type, TREE_REALPART (expr),
-                                      result, false);
+               unsafe_conversion_p (loc, type, TREE_REALPART (expr),
+                                    result, check_sign);
              enum conversion_safety im_safety =
-               unsafe_conversion_p (loc, type, imag_part, result, false);
+               unsafe_conversion_p (loc, type, imag_part, result, check_sign);
 
              /* Merge the results into appropriate single warning.  */
 
@@ -1530,15 +1498,13 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
          /* When they are the same width but different signedness,
             then the value may change.  */
          else if (((TYPE_PRECISION (type) == TYPE_PRECISION (expr_type)
-                   && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type))
-                  /* Even when converted to a bigger type, if the type is
-                     unsigned but expr is signed, then negative values
-                     will be changed.  */
+                    && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type))
+                   /* Even when converted to a bigger type, if the type is
+                      unsigned but expr is signed, then negative values
+                      will be changed.  */
                    || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)))
-                  && produce_warns)
-           warning_at (loc, OPT_Wsign_conversion, "conversion to %qT from %qT "
-                       "may change the sign of the result",
-                       type, expr_type);
+                  && check_sign)
+           give_warning = UNSAFE_SIGN;
        }
 
       /* Warn for integer types converted to real types if and only if
@@ -1594,13 +1560,10 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, tree result,
              /* Check for different signedness, see case for real-domain
                 integers (above) for a more detailed comment.  */
              else if (((TYPE_PRECISION (to_type) == TYPE_PRECISION (from_type)
-                   && TYPE_UNSIGNED (to_type) != TYPE_UNSIGNED (from_type))
-                   || (TYPE_UNSIGNED (to_type) && !TYPE_UNSIGNED (from_type)))
-                   && produce_warns)
-               warning_at (loc, OPT_Wsign_conversion,
-                       "conversion to %qT from %qT "
-                       "may change the sign of the result",
-                       type, expr_type);
+                        && TYPE_UNSIGNED (to_type) != TYPE_UNSIGNED (from_type))
+                       || (TYPE_UNSIGNED (to_type) && !TYPE_UNSIGNED (from_type)))
+                      && check_sign)
+               give_warning = UNSAFE_SIGN;
            }
          else if (TREE_CODE (from_type) == INTEGER_TYPE
                   && TREE_CODE (to_type) == REAL_TYPE

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 59d4aaf..fb0d53b 100644 (file)
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -784,8 +784,7 @@ enum conversion_safety {
   SAFE_CONVERSION = 0,
   /* Another type of conversion with problems.  */
   UNSAFE_OTHER,
-  /* Conversion between signed and unsigned integers
-     which are all warned about immediately, so this is unused.  */
+  /* Conversion between signed and unsigned integers.  */
   UNSAFE_SIGN,
   /* Conversions that reduce the precision of reals including conversions
      from reals to integers.  */

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index bd74957..6dbc660 100644 (file)
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -1210,7 +1210,39 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
        else
          break;
 
-       if (TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant)
+       if (conversion_kind == UNSAFE_SIGN)
+         {
+           bool cstresult
+             = (result
+                && TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant);
+           if (TYPE_UNSIGNED (type))
+             {
+               if (cstresult)
+                 warning_at (loc, OPT_Wsign_conversion,
+                             "unsigned conversion from %qT to %qT "
+                             "changes value from %qE to %qE",
+                             expr_type, type, expr, result);
+               else
+                 warning_at (loc, OPT_Wsign_conversion,
+                             "unsigned conversion from %qT to %qT "
+                             "changes the value of %qE",
+                             expr_type, type, expr);
+             }
+           else
+             {
+               if (cstresult)
+                 warning_at (loc, OPT_Wsign_conversion,
+                             "signed conversion from %qT to %qT changes "
+                             "value from %qE to %qE",
+                             expr_type, type, expr, result);
+               else
+                 warning_at (loc, OPT_Wsign_conversion,
+                             "signed conversion from %qT to %qT changes "
+                             "the value of %qE",
+                             expr_type, type, expr);
+             }
+         }
+       else if (TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant)
          warning_at (loc, warnopt,
                      "conversion from %qT to %qT changes value from %qE to %qE",
                      expr_type, type, expr, result);
@@ -1234,23 +1266,29 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
 
     default: /* 'expr' is not a constant.  */
       conversion_kind = unsafe_conversion_p (loc, type, expr, result, true);
-      if (conversion_kind == UNSAFE_IMAGINARY)
-       warning_at (loc, OPT_Wconversion,
-                   "conversion from %qT to %qT discards imaginary component",
-                   expr_type, type);
-      else
-       {
-         int warnopt;
-         if (conversion_kind == UNSAFE_REAL)
-           warnopt = OPT_Wfloat_conversion;
-         else if (conversion_kind)
-           warnopt = OPT_Wconversion;
-         else
-           break;
+      {
+       int warnopt;
+       if (conversion_kind == UNSAFE_REAL)
+         warnopt = OPT_Wfloat_conversion;
+       else if (conversion_kind == UNSAFE_SIGN)
+         warnopt = OPT_Wsign_conversion;
+       else if (conversion_kind)
+         warnopt = OPT_Wconversion;
+       else
+         break;
+       if (conversion_kind == UNSAFE_SIGN)
+         warning_at (loc, warnopt, "conversion to %qT from %qT "
+                     "may change the sign of the result",
+                     type, expr_type);
+       else if (conversion_kind == UNSAFE_IMAGINARY)
+         warning_at (loc, warnopt,
+                     "conversion from %qT to %qT discards imaginary component",
+                     expr_type, type);
+       else
          warning_at (loc, warnopt,
                      "conversion from %qT to %qT may change value",
                      expr_type, type);
-       }
+      }
     }
 }