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
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.

> > @@ -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.

        Jakub

Reply via email to