diff mbox series

c++: Fix bogus -Wvolatile warning in C++20 [PR98947]

Message ID 20210204000319.299127-1-polacek@redhat.com
State New
Headers show
Series c++: Fix bogus -Wvolatile warning in C++20 [PR98947] | expand

Commit Message

Marek Polacek Feb. 4, 2021, 12:03 a.m. UTC
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, which then evokes the warning.  So use
a warning sentinel.  But since we should still warn for

  (b ? x : y) += 1; // compound assignment
  (b ? (x = 2) : y) = 1; // lvalue-to-rvalue conv on x = 2

I've tweaked mark_use to only set TREE_THIS_VOLATILE when the warning
is enabled.

I'd argue this is a regression because GCC 9 doesn't warn.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

gcc/cp/ChangeLog:

	PR c++/98947
	* expr.c (mark_use) <case MODIFY_EXPR>: Only set TREE_THIS_VOLATILE
	if warn_volatile.
	* typeck.c (cp_build_modify_expr) <case COND_EXPR>: Add a warning
	sentinel for -Wvolatile around build_conditional_expr.

gcc/testsuite/ChangeLog:

	PR c++/98947
	* g++.dg/cpp2a/volatile5.C: New test.
---
 gcc/cp/expr.c                          |  3 ++-
 gcc/cp/typeck.c                        |  4 ++++
 gcc/testsuite/g++.dg/cpp2a/volatile5.C | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/volatile5.C


base-commit: 34215a7a3a359d700a520f1d5bdaec835f0b5180

Comments

Jason Merrill Feb. 4, 2021, 3:59 p.m. UTC | #1
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.

, which then evokes the warning.  So use
> a warning sentinel.  But since we should still warn for
> 
>    (b ? x : y) += 1; // compound assignment
>    (b ? (x = 2) : y) = 1; // lvalue-to-rvalue conv on x = 2
> 
> I've tweaked mark_use to only set TREE_THIS_VOLATILE when the warning
> is enabled.
> 
> I'd argue this is a regression because GCC 9 doesn't warn.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/98947
> 	* expr.c (mark_use) <case MODIFY_EXPR>: Only set TREE_THIS_VOLATILE
> 	if warn_volatile.
> 	* typeck.c (cp_build_modify_expr) <case COND_EXPR>: Add a warning
> 	sentinel for -Wvolatile around build_conditional_expr.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/98947
> 	* g++.dg/cpp2a/volatile5.C: New test.
> ---
>   gcc/cp/expr.c                          |  3 ++-
>   gcc/cp/typeck.c                        |  4 ++++
>   gcc/testsuite/g++.dg/cpp2a/volatile5.C | 15 +++++++++++++++
>   3 files changed, 21 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/volatile5.C
> 
> diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
> index 480e740f08c..d697978ce19 100644
> --- 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?

>   	    {
>   	      warning_at (location_of (expr), OPT_Wvolatile,
>   			  "using value of simple assignment with %<volatile%>-"
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index a87d5e5f2ac..52c2344530d 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -8617,6 +8617,10 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
>   	tree op2 = TREE_OPERAND (lhs, 2);
>   	if (TREE_CODE (op2) != THROW_EXPR)
>   	  op2 = cp_build_modify_expr (loc, op2, modifycode, rhs, complain);
> +	/* build_conditional_expr calls mark_lvalue_use for op1/op2,
> +	   which are now assignments due to the above transformation,
> +	   generating bogus C++20 warnings.  */
> +	warning_sentinel w (warn_volatile);
>   	tree cond = build_conditional_expr (input_location,
>   					    TREE_OPERAND (lhs, 0), op1, op2,
>   					    complain);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/volatile5.C b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
> new file mode 100644
> index 00000000000..1f9d23845b4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
> @@ -0,0 +1,15 @@
> +// PR c++/98947
> +// { dg-do compile }
> +
> +volatile int x, y, z;
> +
> +void
> +f (bool b)
> +{
> +  (b ? x : y) = 1;
> +  (b ? x : y) += 1; // { dg-warning "compound assignment" "" { target c++20 } }
> +  z = (b ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +  ((z = 2) ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +  (b ? (x = 2) : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +  (b ? x : (y = 5)) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +}
> 
> base-commit: 34215a7a3a359d700a520f1d5bdaec835f0b5180
>
Marek Polacek Feb. 4, 2021, 6:11 p.m. UTC | #2
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
Jason Merrill Feb. 4, 2021, 8:47 p.m. UTC | #3
On 2/4/21 1:11 PM, Marek Polacek wrote:
> 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.

Only after we've decided (in /6) that the result is a prvalue.

> With those mark_lvalue_use calls removed, we'd not issue the
> warning for
> 
>    (b ? (x = 2) : y) = 1;
>    (b ? x : (y = 5)) = 1;

Why wouldn't we?  The assignment should call mark_lvalue_use for the 
LHS, which recursively applies it to the arms of the ?:.

Jason
Marek Polacek Feb. 4, 2021, 11:09 p.m. UTC | #4
On Thu, Feb 04, 2021 at 03:47:54PM -0500, Jason Merrill via Gcc-patches wrote:
> On 2/4/21 1:11 PM, Marek Polacek wrote:
> > 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.
> 
> Only after we've decided (in /6) that the result is a prvalue.

Oh, I see.  :~

> > With those mark_lvalue_use calls removed, we'd not issue the
> > warning for
> > 
> >    (b ? (x = 2) : y) = 1;
> >    (b ? x : (y = 5)) = 1;
> 
> Why wouldn't we?  The assignment should call mark_lvalue_use for the LHS,
> which recursively applies it to the arms of the ?:.

Aha: we call mark_lvalue_use_*nonread* and the warning was guarded by
read_p.  I don't know why I did it, I see no regressions if I just
remove the check.  And then I get the expected results.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

-- >8 --
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, which then evokes the warning.  The calls
to mark_lvalue_use were added in r160289 to suppress warnings in
Wunused-var-10.c, but looks like they're no longer needed.

To warn on

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

I've tweaked a check in mark_use/MODIFY_EXPR.

I'd argue this is a regression because GCC 9 doesn't warn.

gcc/cp/ChangeLog:

	PR c++/98947
	* call.c (build_conditional_expr_1): Don't call mark_lvalue_use
	on arg2/arg3.
	* expr.c (mark_use) <case MODIFY_EXPR>: Don't check read_p when
	issuing the -Wvolatile warning.  Only set TREE_THIS_VOLATILE if
	a warning was emitted.

gcc/testsuite/ChangeLog:

	PR c++/98947
	* g++.dg/cpp2a/volatile5.C: New test.
---
 gcc/cp/call.c                          |  2 --
 gcc/cp/expr.c                          | 14 +++++++-------
 gcc/testsuite/g++.dg/cpp2a/volatile5.C | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/volatile5.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c7e13f3a22b..4744c9768ec 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5559,8 +5559,6 @@ build_conditional_expr_1 (const op_location_t &loc,
       && same_type_p (arg2_type, arg3_type))
     {
       result_type = arg2_type;
-      arg2 = mark_lvalue_use (arg2);
-      arg3 = mark_lvalue_use (arg3);
       goto valid_operands;
     }
 
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 480e740f08c..d16d1896f2d 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -224,17 +224,17 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
 	     a volatile-qualified type is deprecated unless the assignment
 	     is either a discarded-value expression or appears in an
 	     unevaluated context."  */
-	  if (read_p
-	      && !cp_unevaluated_operand
+	  if (!cp_unevaluated_operand
 	      && (TREE_THIS_VOLATILE (lhs)
 		  || CP_TYPE_VOLATILE_P (TREE_TYPE (lhs)))
 	      && !TREE_THIS_VOLATILE (expr))
 	    {
-	      warning_at (location_of (expr), OPT_Wvolatile,
-			  "using value of simple assignment with %<volatile%>-"
-			  "qualified left operand is deprecated");
-	      /* Make sure not to warn about this assignment again.  */
-	      TREE_THIS_VOLATILE (expr) = true;
+	      if (warning_at (location_of (expr), OPT_Wvolatile,
+			      "using value of simple assignment with "
+			      "%<volatile%>-qualified left operand is "
+			      "deprecated"))
+		/* Make sure not to warn about this assignment again.  */
+		TREE_THIS_VOLATILE (expr) = true;
 	    }
 	  break;
 	}
diff --git a/gcc/testsuite/g++.dg/cpp2a/volatile5.C b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
new file mode 100644
index 00000000000..1f9d23845b4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
@@ -0,0 +1,15 @@
+// PR c++/98947
+// { dg-do compile }
+
+volatile int x, y, z;
+
+void
+f (bool b)
+{
+  (b ? x : y) = 1;
+  (b ? x : y) += 1; // { dg-warning "compound assignment" "" { target c++20 } }
+  z = (b ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+  ((z = 2) ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+  (b ? (x = 2) : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+  (b ? x : (y = 5)) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+}

base-commit: 4e7c24d97dd65083a770252ce942f43d408fe11d
Jason Merrill Feb. 5, 2021, 2:45 p.m. UTC | #5
On 2/4/21 6:09 PM, Marek Polacek wrote:
> On Thu, Feb 04, 2021 at 03:47:54PM -0500, Jason Merrill via Gcc-patches wrote:
>> On 2/4/21 1:11 PM, Marek Polacek wrote:
>>> 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.
>>
>> Only after we've decided (in /6) that the result is a prvalue.
> 
> Oh, I see.  :~
> 
>>> With those mark_lvalue_use calls removed, we'd not issue the
>>> warning for
>>>
>>>     (b ? (x = 2) : y) = 1;
>>>     (b ? x : (y = 5)) = 1;
>>
>> Why wouldn't we?  The assignment should call mark_lvalue_use for the LHS,
>> which recursively applies it to the arms of the ?:.
> 
> Aha: we call mark_lvalue_use_*nonread* and the warning was guarded by
> read_p.  I don't know why I did it, I see no regressions if I just
> remove the check.  And then I get the expected results.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

OK.

> -- >8 --
> 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, which then evokes the warning.  The calls
> to mark_lvalue_use were added in r160289 to suppress warnings in
> Wunused-var-10.c, but looks like they're no longer needed.
> 
> To warn on
> 
>      (b ? (x = 2) : y) = 1;
>      (b ? x : (y = 5)) = 1;
> 
> I've tweaked a check in mark_use/MODIFY_EXPR.
> 
> I'd argue this is a regression because GCC 9 doesn't warn.
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/98947
> 	* call.c (build_conditional_expr_1): Don't call mark_lvalue_use
> 	on arg2/arg3.
> 	* expr.c (mark_use) <case MODIFY_EXPR>: Don't check read_p when
> 	issuing the -Wvolatile warning.  Only set TREE_THIS_VOLATILE if
> 	a warning was emitted.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/98947
> 	* g++.dg/cpp2a/volatile5.C: New test.
> ---
>   gcc/cp/call.c                          |  2 --
>   gcc/cp/expr.c                          | 14 +++++++-------
>   gcc/testsuite/g++.dg/cpp2a/volatile5.C | 15 +++++++++++++++
>   3 files changed, 22 insertions(+), 9 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/volatile5.C
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index c7e13f3a22b..4744c9768ec 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -5559,8 +5559,6 @@ build_conditional_expr_1 (const op_location_t &loc,
>         && same_type_p (arg2_type, arg3_type))
>       {
>         result_type = arg2_type;
> -      arg2 = mark_lvalue_use (arg2);
> -      arg3 = mark_lvalue_use (arg3);
>         goto valid_operands;
>       }
>   
> diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
> index 480e740f08c..d16d1896f2d 100644
> --- a/gcc/cp/expr.c
> +++ b/gcc/cp/expr.c
> @@ -224,17 +224,17 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
>   	     a volatile-qualified type is deprecated unless the assignment
>   	     is either a discarded-value expression or appears in an
>   	     unevaluated context."  */
> -	  if (read_p
> -	      && !cp_unevaluated_operand
> +	  if (!cp_unevaluated_operand
>   	      && (TREE_THIS_VOLATILE (lhs)
>   		  || CP_TYPE_VOLATILE_P (TREE_TYPE (lhs)))
>   	      && !TREE_THIS_VOLATILE (expr))
>   	    {
> -	      warning_at (location_of (expr), OPT_Wvolatile,
> -			  "using value of simple assignment with %<volatile%>-"
> -			  "qualified left operand is deprecated");
> -	      /* Make sure not to warn about this assignment again.  */
> -	      TREE_THIS_VOLATILE (expr) = true;
> +	      if (warning_at (location_of (expr), OPT_Wvolatile,
> +			      "using value of simple assignment with "
> +			      "%<volatile%>-qualified left operand is "
> +			      "deprecated"))
> +		/* Make sure not to warn about this assignment again.  */
> +		TREE_THIS_VOLATILE (expr) = true;
>   	    }
>   	  break;
>   	}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/volatile5.C b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
> new file mode 100644
> index 00000000000..1f9d23845b4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
> @@ -0,0 +1,15 @@
> +// PR c++/98947
> +// { dg-do compile }
> +
> +volatile int x, y, z;
> +
> +void
> +f (bool b)
> +{
> +  (b ? x : y) = 1;
> +  (b ? x : y) += 1; // { dg-warning "compound assignment" "" { target c++20 } }
> +  z = (b ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +  ((z = 2) ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +  (b ? (x = 2) : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +  (b ? x : (y = 5)) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
> +}
> 
> base-commit: 4e7c24d97dd65083a770252ce942f43d408fe11d
>
diff mbox series

Patch

diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 480e740f08c..d697978ce19 100644
--- 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)
 	    {
 	      warning_at (location_of (expr), OPT_Wvolatile,
 			  "using value of simple assignment with %<volatile%>-"
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a87d5e5f2ac..52c2344530d 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -8617,6 +8617,10 @@  cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
 	tree op2 = TREE_OPERAND (lhs, 2);
 	if (TREE_CODE (op2) != THROW_EXPR)
 	  op2 = cp_build_modify_expr (loc, op2, modifycode, rhs, complain);
+	/* build_conditional_expr calls mark_lvalue_use for op1/op2,
+	   which are now assignments due to the above transformation,
+	   generating bogus C++20 warnings.  */
+	warning_sentinel w (warn_volatile);
 	tree cond = build_conditional_expr (input_location,
 					    TREE_OPERAND (lhs, 0), op1, op2,
 					    complain);
diff --git a/gcc/testsuite/g++.dg/cpp2a/volatile5.C b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
new file mode 100644
index 00000000000..1f9d23845b4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/volatile5.C
@@ -0,0 +1,15 @@ 
+// PR c++/98947
+// { dg-do compile }
+
+volatile int x, y, z;
+
+void
+f (bool b)
+{
+  (b ? x : y) = 1;
+  (b ? x : y) += 1; // { dg-warning "compound assignment" "" { target c++20 } }
+  z = (b ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+  ((z = 2) ? x : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+  (b ? (x = 2) : y) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+  (b ? x : (y = 5)) = 1; // { dg-warning "using value of simple assignment" "" { target c++20 } }
+}