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