On Thu, Feb 04, 2021 at 10:59:21AM -0500, Jason Merrill wrote:
> On 2/3/21 7:03 PM, Marek Polacek wrote:
> > Since most of volatile is deprecated in C++20, we are required to warn
> > for compound assignments to volatile variables and so on.  But here we
> > have
> > 
> >    volatile int x, y, z;
> >    (b ? x : y) = 1;
> > 
> > and we shouldn't warn, because simple assignments like x = 24; should
> > not provoke the warning when they are a discarded-value expression.
> > 
> > We warn here because when ?: is used as an lvalue, we transform it in
> > cp_build_modify_expr/COND_EXPR from (a ? b : c) = rhs to
> > 
> >    (a ? (b = rhs) : (c = rhs))
> > 
> > and build_conditional_expr then calls mark_lvalue_use for the new
> > artificial assignments
> 
> Hmm, that seems wrong; the ?: expression itself does not use lvalue operands
> any more than ',' does.  I notice that removing those mark_lvalue_use calls
> doesn't regress Wunused-var-10.c, which was added with them in r160289.

The mark_lvalue_use calls didn't strike me as wrong because [expr.cond]/7
says that lvalue-to-rvalue conversion is performed on the second and third
operands.  With those mark_lvalue_use calls removed, we'd not issue the
warning for

  (b ? (x = 2) : y) = 1;
  (b ? x : (y = 5)) = 1;

which I think we want (and clang++ warns here too).

> > --- a/gcc/cp/expr.c
> > +++ b/gcc/cp/expr.c
> > @@ -228,7 +228,8 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
> >           && !cp_unevaluated_operand
> >           && (TREE_THIS_VOLATILE (lhs)
> >               || CP_TYPE_VOLATILE_P (TREE_TYPE (lhs)))
> > -         && !TREE_THIS_VOLATILE (expr))
> > +         && !TREE_THIS_VOLATILE (expr)
> > +         && warn_volatile)
> 
> Or you could check the return value of the warning?

Works too.  Happy to change that.

Marek

Reply via email to