[Bug c/104711] Unnecessary -Wshift-negative-value warning

["jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104711

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org,
                   |                            |jsm28 at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
>From what I can see, -fsanitize=undefined -fwrapv already treats shifts the
C++20  https://wg21.link/p0907 way, where only the last operand is checked for
being out of bounds (negative or greater or equal to bitsize).
N2731 still contains the old (C11 etc.) shift wording.
Then we have this -Wshift-negative-value warning which doesn't seem to care
about details, complains about left shifts of negative constant always, doesn't
care about -fwrapv and is enabled in -Wextra for C99 and later and C++11 and
later.
I'd say we should not warn if TYPE_OVERFLOW_WRAPS (type) aka -fwrapv and we
shouldn't enable the warning in -Wextra for C++20 or later.

Note, what -fsanitize=undefined actually instruments is beyond the out of
bounds y is for signed x << y:
C99-C2x ((unsigned) x >> (uprecm1 - y)) != 0 then UB
C++11-C++17 x < 0 || ((unsigned) x >> (uprecm1 - y)) > 1 then UB

So, I think we want something like:
--- gcc/doc/invoke.texi.jj      2022-02-25 10:46:53.085181500 +0100
+++ gcc/doc/invoke.texi 2022-03-01 09:59:15.040855224 +0100
@@ -5809,7 +5809,7 @@ name is still supported, but the newer n
 -Wredundant-move @r{(only for C++)}  @gol
 -Wtype-limits  @gol
 -Wuninitialized  @gol
--Wshift-negative-value @r{(in C++03 and in C99 and newer)}  @gol
+-Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}  @gol
 -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}
@gol
 -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or}
@option{-Wall}@r{)}}

@@ -6839,7 +6839,7 @@ of the type.  This warning is enabled by
 @opindex Wshift-negative-value
 @opindex Wno-shift-negative-value
 Warn if left shifting a negative value.  This warning is enabled by
-@option{-Wextra} in C99 and C++11 modes (and newer).
+@option{-Wextra} in C99 (and newer) and C++11 to C++17 modes.

 @item -Wno-shift-overflow
 @itemx -Wshift-overflow=@var{n}
--- gcc/c-family/c-warn.cc.jj   2022-01-18 11:58:58.922991486 +0100
+++ gcc/c-family/c-warn.cc      2022-03-01 10:02:41.634971050 +0100
@@ -2605,7 +2605,7 @@ maybe_warn_shift_overflow (location_t lo
   unsigned int prec0 = TYPE_PRECISION (type0);

   /* Left-hand operand must be signed.  */
-  if (TYPE_UNSIGNED (type0) || cxx_dialect >= cxx20)
+  if (TYPE_OVERFLOW_WRAPS (type0) || cxx_dialect >= cxx20)
     return false;

   unsigned int min_prec = (wi::min_precision (wi::to_wide (op0), SIGNED)
--- gcc/c-family/c-ubsan.cc.jj  2022-02-09 15:15:59.288840032 +0100
+++ gcc/c-family/c-ubsan.cc     2022-03-01 09:55:51.779693845 +0100
@@ -173,7 +173,7 @@ ubsan_instrument_shift (location_t loc,
       || cxx_dialect >= cxx20)
     ;

-  /* For signed x << y, in C99/C11, the following:
+  /* For signed x << y, in C99 and later, the following:
      (unsigned) x >> (uprecm1 - y)
      if non-zero, is undefined.  */
   else if (code == LSHIFT_EXPR && flag_isoc99 && cxx_dialect < cxx11)
@@ -186,7 +186,7 @@ ubsan_instrument_shift (location_t loc,
                        build_int_cst (TREE_TYPE (tt), 0));
     }

-  /* For signed x << y, in C++11 and later, the following:
+  /* For signed x << y, in C++11 to C++17, the following:
      x < 0 || ((unsigned) x >> (uprecm1 - y))
      if > 1, is undefined.  */
   else if (code == LSHIFT_EXPR && cxx_dialect >= cxx11)
--- gcc/c-family/c-opts.cc.jj   2022-01-18 11:58:58.884992028 +0100
+++ gcc/c-family/c-opts.cc      2022-03-01 09:57:34.880253831 +0100
@@ -934,10 +934,12 @@ c_common_post_options (const char **pfil
   if (warn_shift_overflow == -1)
     warn_shift_overflow = cxx_dialect >= cxx11 || flag_isoc99;

-  /* -Wshift-negative-value is enabled by -Wextra in C99 and C++11 modes.  */
+  /* -Wshift-negative-value is enabled by -Wextra in C99 and C++11 to C++17
+     modes.  */
   if (warn_shift_negative_value == -1)
     warn_shift_negative_value = (extra_warnings
-                                && (cxx_dialect >= cxx11 || flag_isoc99));
+                                && (cxx_dialect >= cxx11 || flag_isoc99)
+                                && cxx_dialect < cxx20);

   /* -Wregister is enabled by default in C++17.  */
   SET_OPTION_IF_UNSET (&global_options, &global_options_set, warn_register,
--- gcc/c/c-typeck.cc.jj        2022-02-11 00:19:22.135067293 +0100
+++ gcc/c/c-typeck.cc   2022-03-01 10:04:20.925584897 +0100
@@ -12213,7 +12213,8 @@ build_binary_op (location_t location, en
        {
          doing_shift = true;
          if (TREE_CODE (op0) == INTEGER_CST
-             && tree_int_cst_sgn (op0) < 0)
+             && tree_int_cst_sgn (op0) < 0
+             && !TYPE_OVERFLOW_WRAPS (type0))
            {
              /* Don't reject a left shift of a negative value in a context
                 where a constant expression is needed in C90.  */
--- gcc/cp/typeck.cc.jj 2022-02-18 12:38:06.065393230 +0100
+++ gcc/cp/typeck.cc    2022-03-01 10:04:57.726071137 +0100
@@ -5382,6 +5382,7 @@ cp_build_binary_op (const op_location_t
          doing_shift = true;
          if (TREE_CODE (const_op0) == INTEGER_CST
              && tree_int_cst_sgn (const_op0) < 0
+             && !TYPE_OVERFLOW_WRAPS (type0)
              && (complain & tf_warning)
              && c_inhibit_evaluation_warnings == 0)
            warning_at (location, OPT_Wshift_negative_value,
plus testsuite adjustments.