On 7/11/25 9:09 AM, Jakub Jelinek wrote:
On Thu, Jul 10, 2025 at 04:35:49PM -0400, Jason Merrill wrote:
--- gcc/cp/cp-gimplify.cc.jj    2025-04-12 21:41:42.660924514 +0200
+++ gcc/cp/cp-gimplify.cc       2025-04-23 21:33:19.050931604 +0200
@@ -3200,7 +3200,23 @@ cp_fold (tree x, fold_flags_t flags)
         loc = EXPR_LOCATION (x);
         op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops, flags);
+      bool clear_decl_read;
+      clear_decl_read = false;
+      if (code == MODIFY_EXPR
+         && (VAR_P (op0) || TREE_CODE (op0) == PARM_DECL)
+         && !DECL_READ_P (op0)
+         && (VAR_P (op0) ? warn_unused_but_set_variable
+                         : warn_unused_but_set_parameter) > 2
+         && BINARY_CLASS_P (TREE_OPERAND (x, 1))
+         && TREE_OPERAND (TREE_OPERAND (x, 1), 0) == op0)
+       {
+         mark_exp_read (TREE_OPERAND (TREE_OPERAND (x, 1), 1));
+         if (!DECL_READ_P (op0))
+           clear_decl_read = true;
+       }
         op1 = cp_fold_rvalue (TREE_OPERAND (x, 1), flags);
+      if (clear_decl_read)
+       DECL_READ_P (op0) = 0;

Why does this need to happen in cp_fold?  Weren't the flags set properly at
build time?

Without the cp-gimplify.cc (cp_fold) hunk there are tons of FAILs, with

Then could we do something simpler at this point, just preserving the pre-folding state of DECL_READ_P (op0) without looking at the RHS?

GXX_TESTSUITE_STDS=17 make check-g++ RUNTESTFLAGS="dg.exp='Wunused-var* 
Wunused-parm* name-independent-decl1.C unused-9.c memchr-3.c'"
(just 17 so that the same FAILs don't keep repeated for different std
versions):
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 14)
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 15)
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 16)
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 17)
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 18)
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 19)
FAIL: g++.dg/warn/Wunused-parm-12.C  -std=gnu++17  (test for warnings, line 20)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 14)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 15)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 16)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 17)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 18)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 19)
FAIL: g++.dg/warn/Wunused-parm-13.C  -std=gnu++17  (test for warnings, line 20)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 13)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 14)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 15)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 16)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 17)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 18)
FAIL: c-c++-common/Wunused-parm-1.c  -std=gnu++17  (test for warnings, line 19)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 13)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 14)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 15)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 16)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 17)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 18)
FAIL: c-c++-common/Wunused-parm-2.c  -std=gnu++17  (test for warnings, line 19)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 13)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 14)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 15)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 16)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 17)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 18)
FAIL: c-c++-common/Wunused-parm-3.c  -std=gnu++17  (test for warnings, line 19)

cp_fold_rvalue -> cp_fold_maybe_rvalue -> mark_rvalue_use -> mark_use
->mark_exp_read then sets DECL_READ_P on op0, which we want to avoid in the
op0 @= expr
case in some of the levels.

@@ -211,8 +211,27 @@ mark_use (tree expr, bool rvalue_p, bool
            }
          return expr;
        }
-      gcc_fallthrough();
+      gcc_fallthrough ();
       CASE_CONVERT:
+      if (VOID_TYPE_P (TREE_TYPE (expr)))
+       switch (TREE_CODE (TREE_OPERAND (expr, 0)))
+         {
+         case PREINCREMENT_EXPR:
+         case PREDECREMENT_EXPR:
+         case POSTINCREMENT_EXPR:
+         case POSTDECREMENT_EXPR:

Why is this specific to these codes?  I would think we would want consistent
handling of (void) here and in mark_exp_read.

The second/third levels of the warnings (i.e. the new ones) want
++op0
--op0
op0++
op0--
not to be treated as uses of op0 (but op1 = ++op0 etc. counts as use).
Which is why it checks for these special cases.  The cast to void is there
to just ignore the pre/post inc/decrements alone when their value isn't
used.
The above mark_exp_read changes wouldn't be needed if
     case PREINCREMENT_EXPR:
     case PREDECREMENT_EXPR:
     case POSTINCREMENT_EXPR:
     case POSTDECREMENT_EXPR:
       mark_exp_read (TREE_OPERAND (exp, 0));
       break;
weren't added, but that is needed so that x = ++op0; etc. are treated as
use of op0, while ++op0; on its own is not.

Makes sense.

@@ -3740,7 +3740,28 @@ finish_unary_op_expr (location_t op_loc,
     tree expr_ovl = expr;
     if (!processing_template_decl)
-    expr_ovl = cp_fully_fold (expr_ovl);
+    switch (code)
+      {
+      case PREINCREMENT_EXPR:
+      case PREDECREMENT_EXPR:
+      case POSTINCREMENT_EXPR:
+      case POSTDECREMENT_EXPR:
+       tree stripped_expr;
+       stripped_expr = tree_strip_any_location_wrapper (expr);
+       if ((VAR_P (stripped_expr) || TREE_CODE (stripped_expr) == PARM_DECL)
+           && !DECL_READ_P (stripped_expr)
+           && (VAR_P (stripped_expr) ? warn_unused_but_set_variable
+                                     : warn_unused_but_set_parameter) > 1)
+         {
+           expr_ovl = cp_fully_fold (expr_ovl);

Again I wonder why cp_fold is setting DECL_READ_P.

See above.

Ah, the problem here is that cp_fully_fold assumes that we're using its operand as an rvalue, which is wrong for ++/--. And we aren't going to get a constant result anyway, so finish_unary_op_expr shouldn't call cp_fully_fold, it should return result early for lvalue codes.

Jason

Reply via email to