On 7/11/25 2:17 PM, Jakub Jelinek wrote:
On Fri, Jul 11, 2025 at 12:26:54PM -0400, Jason Merrill wrote:
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?
That would be wrong.
The goal of the patch is to just add some extra exceptions which aren't
counted as uses.
If a MODIFY_EXPR is say
op0 = foo (op0);
then we shouldn't clear DECL_READ_P, the call does or could read the value
and use it for something, so warning that op0 is just set but not used would
be false positive.
But by the time we get to cp_fold, DECL_READ_P should have already been
set appropriately when we built the thing we're now folding. And
calling mark_exp_read on foo(op0) won't mark op0 anyway; it doesn't
recurse. I don't see any regressions on Wunused* after
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 9a98628d9e8..addbc29d104 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -3221,16 +3221,8 @@ cp_fold (tree x, fold_flags_t flags)
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;
- }
+ && !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;
@@ -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.
Well, I'm not sure it is actually an error. finish_unary_op_expr doesn't
use cp_fully_fold result as an operand of {PRE,POST}{INC,DEC}REMENT_EXPR
(that would be wrong, we don't want the operand to fold into non-lvalue), it
is called only to find out if overflow warning should be emitted.
But it only warns if the whole expression folds to a constant, which can
never happen for these tree codes. So folding the operand is useless.
Jason
