Re: [PATCH] c++: non-dependent assignment checking [PR63198, PR18474]

2023-09-18 Thread Jason Merrill via Gcc-patches 
ded as far as bootstrap + regtest is concerned.
FWIW this line originally was added in
https://gcc.gnu.org/pipermail/gcc-patches/2004-July/144621.html
to suppress -Wparentheses warnings for non-templated compound assignment
operator expressions.

Here's a patch that also gets rid of that suppress_warning call:


OK.


-- >8 --

Subject: [PATCH] c++: non-dependent assignment checking [PR63198, PR18474]

PR c++/63198
PR c++/18474

gcc/cp/ChangeLog:

* semantics.cc (maybe_convert_cond): Look through implicit
INDIRECT_REF when deciding whether to issue a -Wparentheses
warning, and consider templated assignment expressions as well.
(finish_parenthesized_expr): Look through implicit INDIRECT_REF
when suppressing -Wparentheses warning.
* typeck.cc (build_x_modify_expr): Check simple assignments
ahead time too, not just compound assignments.  Give the second
operand of MODOP_EXPR a non-null type so that it's not considered
always instantiation-dependent.  Don't call suppress_warning.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/static_assert15.C: Expect diagnostic for
non-constant static_assert condition.
* g++.dg/expr/unary2.C: Remove xfails.
* g++.dg/template/init7.C: Make initializer type-dependent to
preserve intent of test.
* g++.dg/template/recurse3.C: Likewise for the erroneous
statement.
* g++.dg/template/non-dependent26.C: New test.
* g++.dg/warn/Wparentheses-32.C: New test.

libstdc++/ChangeLog:

* testsuite/26_numerics/random/discard_block_engine/cons/seed_seq2.cc:
Make data member seed_seq::called mutable.
* 
testsuite/26_numerics/random/independent_bits_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/linear_congruential_engine/cons/seed_seq2.cc
Likewise.
* 
testsuite/26_numerics/random/mersenne_twister_engine/cons/seed_seq2.cc:
Likewise.
* testsuite/26_numerics/random/shuffle_order_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/subtract_with_carry_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/ext/random/simd_fast_mersenne_twister_engine/cons/seed_seq2.cc
Likewise.
---
  gcc/cp/semantics.cc   | 19 ++---
  gcc/cp/typeck.cc  | 41 ---
  gcc/testsuite/g++.dg/cpp0x/static_assert15.C  |  2 +-
  gcc/testsuite/g++.dg/expr/unary2.C|  8 ++--
  gcc/testsuite/g++.dg/template/init7.C |  2 +-
  .../g++.dg/template/non-dependent26.C | 25 +++
  gcc/testsuite/g++.dg/template/recurse3.C  |  8 ++--
  gcc/testsuite/g++.dg/warn/Wparentheses-32.C   | 28 +
  .../discard_block_engine/cons/seed_seq2.cc|  2 +-
  .../independent_bits_engine/cons/seed_seq2.cc |  2 +-
  .../cons/seed_seq2.cc |  2 +-
  .../mersenne_twister_engine/cons/seed_seq2.cc |  2 +-
  .../shuffle_order_engine/cons/seed_seq2.cc|  2 +-
  .../cons/seed_seq2.cc |  2 +-
  .../cons/seed_seq2.cc |  2 +-
  15 files changed, 100 insertions(+), 47 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/template/non-dependent26.C
  create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-32.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 0f7f4e87ae4..4109ac33654 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -881,13 +881,17 @@ maybe_convert_cond (tree cond)
/* Do the conversion.  */
cond = convert_from_reference (cond);
  
-  if ((TREE_CODE (cond) == MODIFY_EXPR || is_assignment_op_expr_p (cond))

+  tree inner = REFERENCE_REF_P (cond) ? TREE_OPERAND (cond, 0) : cond;
+  if ((TREE_CODE (inner) == MODIFY_EXPR
+   || (TREE_CODE (inner) == MODOP_EXPR
+  && TREE_CODE (TREE_OPERAND (inner, 1)) == NOP_EXPR)
+   || is_assignment_op_expr_p (inner))
&& warn_parentheses
-  && !warning_suppressed_p (cond, OPT_Wparentheses)
-  && warning_at (cp_expr_loc_or_input_loc (cond),
+  && !warning_suppressed_p (inner, OPT_Wparentheses)
+  && warning_at (cp_expr_loc_or_input_loc (inner),
 OPT_Wparentheses, "suggest parentheses around "
   "assignment used as truth value"))
-suppress_warning (cond, OPT_Wparentheses);
+suppress_warning (inner, OPT_Wparentheses);
  
return condition_conversion (cond);

  }
@@ -2155,8 +2159,11 @@ cp_expr
  finish_parenthesized_expr (cp_expr expr)
  {
if (EXPR_P (expr))
-/* This inhibits warnings in c_common_truthvalue_conversion.  */
-suppress_warning (expr, OPT_Wparentheses);
+{
+  /* This inhibits warnings in c_common_truthvalue_conversion.  */
+  tree inner = REFERENCE_REF_P (expr) ? TREE_O

Re: [PATCH] c++: non-dependent assignment checking [PR63198, PR18474]

2023-09-18 Thread Patrick Palka via Gcc-patches 
_code modifycode,
> > rhs = build_non_dependent_expr (rhs);
> >   }
> >   -  if (modifycode != NOP_EXPR)
> > -{
> > -  tree op = build_nt (modifycode, NULL_TREE, NULL_TREE);
> > -  tree rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
> > +  tree rval;
> > +  if (modifycode == NOP_EXPR)
> > +rval = cp_build_modify_expr (loc, lhs, modifycode, rhs, complain);
> > +  else
> > +rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
> >  lhs, rhs, op, lookups, , complain);
> > -  if (rval)
> > -   {
> > if (rval == error_mark_node)
> > -   return rval;
> > +return error_mark_node;
> > +  if (modifycode != NOP_EXPR)
> >   suppress_warning (rval /* What warning? */);
> 
> Did you try disabling this to see if it's still needed?

Looks like it's not needed as far as bootstrap + regtest is concerned.
FWIW this line originally was added in
https://gcc.gnu.org/pipermail/gcc-patches/2004-July/144621.html
to suppress -Wparentheses warnings for non-templated compound assignment
operator expressions.

Here's a patch that also gets rid of that suppress_warning call:

-- >8 --

Subject: [PATCH] c++: non-dependent assignment checking [PR63198, PR18474]

PR c++/63198
PR c++/18474

gcc/cp/ChangeLog:

* semantics.cc (maybe_convert_cond): Look through implicit
INDIRECT_REF when deciding whether to issue a -Wparentheses
warning, and consider templated assignment expressions as well.
(finish_parenthesized_expr): Look through implicit INDIRECT_REF
when suppressing -Wparentheses warning.
* typeck.cc (build_x_modify_expr): Check simple assignments
ahead time too, not just compound assignments.  Give the second
operand of MODOP_EXPR a non-null type so that it's not considered
always instantiation-dependent.  Don't call suppress_warning.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/static_assert15.C: Expect diagnostic for
non-constant static_assert condition.
* g++.dg/expr/unary2.C: Remove xfails.
* g++.dg/template/init7.C: Make initializer type-dependent to
preserve intent of test.
* g++.dg/template/recurse3.C: Likewise for the erroneous
statement.
* g++.dg/template/non-dependent26.C: New test.
* g++.dg/warn/Wparentheses-32.C: New test.

libstdc++/ChangeLog:

* testsuite/26_numerics/random/discard_block_engine/cons/seed_seq2.cc:
Make data member seed_seq::called mutable.
* 
testsuite/26_numerics/random/independent_bits_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/linear_congruential_engine/cons/seed_seq2.cc
Likewise.
* 
testsuite/26_numerics/random/mersenne_twister_engine/cons/seed_seq2.cc:
Likewise.
* testsuite/26_numerics/random/shuffle_order_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/subtract_with_carry_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/ext/random/simd_fast_mersenne_twister_engine/cons/seed_seq2.cc
Likewise.
---
 gcc/cp/semantics.cc   | 19 ++---
 gcc/cp/typeck.cc  | 41 ---
 gcc/testsuite/g++.dg/cpp0x/static_assert15.C  |  2 +-
 gcc/testsuite/g++.dg/expr/unary2.C|  8 ++--
 gcc/testsuite/g++.dg/template/init7.C |  2 +-
 .../g++.dg/template/non-dependent26.C | 25 +++
 gcc/testsuite/g++.dg/template/recurse3.C  |  8 ++--
 gcc/testsuite/g++.dg/warn/Wparentheses-32.C   | 28 +
 .../discard_block_engine/cons/seed_seq2.cc|  2 +-
 .../independent_bits_engine/cons/seed_seq2.cc |  2 +-
 .../cons/seed_seq2.cc |  2 +-
 .../mersenne_twister_engine/cons/seed_seq2.cc |  2 +-
 .../shuffle_order_engine/cons/seed_seq2.cc|  2 +-
 .../cons/seed_seq2.cc |  2 +-
 .../cons/seed_seq2.cc |  2 +-
 15 files changed, 100 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/non-dependent26.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-32.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 0f7f4e87ae4..4109ac33654 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -881,13 +881,17 @@ maybe_convert_cond (tree cond)
   /* Do the conversion.  */
   cond = convert_from_reference (cond);
 
-  if ((TREE_CODE (cond) == MODIFY_EXPR || is_assignment_op_expr_p (cond))
+  tree inner = REFERENCE_REF_P (cond) ? TREE_OPERAND (cond, 0) : cond;
+  if ((TREE_CODE (inner) == MODIFY_EXPR
+   || (TREE_CODE (inner) == MODOP_EXPR
+  && TREE_CODE (TREE_OPERAND (inner, 1)) == NOP_EXPR)
+   || is_assignment_op_expr_p (inner))
   && warn_parentheses
-  &

Re: [PATCH] c++: non-dependent assignment checking [PR63198, PR18474]

2023-09-17 Thread Jason Merrill via Gcc-patches 

On 9/17/23 14:51, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Patch generatde with -w to avoid noisy whitespace changes.

-- >8 --

This patch makes us recognize and check non-dependent simple assigments
ahead of time, like we already do for compound assignments.  This means
the templated representation of such assignments will now usually have
an implicit INDIRECT_REF (due to the reference return type), which the
-Wparentheses code needs to handle.  As a drive-by improvement, this
patch also makes maybe_convert_cond issue -Wparentheses warnings ahead
of time.

This revealed some libstdc++ tests were attempting to modify a data
member from a uninstantiated const member function; naively fixed by
making the data member mutable.

PR c++/63198
PR c++/18474

gcc/cp/ChangeLog:

* semantics.cc (maybe_convert_cond): Look through implicit
INDIRECT_REF when deciding whether to issue a -Wparentheses
warning, and consider templated assignment expressions as well.
(finish_parenthesized_expr): Look through implicit INDIRECT_REF
when suppressing -Wparentheses warning.
* typeck.cc (build_x_modify_expr): Check simple assignments
ahead time too, not just compound assignments.  Give the second
operand of MODOP_EXPR a non-null type so that it's not considered
always instantiation-dependent.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/static_assert15.C: Expect diagnostic for
non-constant static_assert condition.
* g++.dg/expr/unary2.C: Remove xfails.
* g++.dg/template/init7.C: Make initializer type-dependent to
preserve intent of test.
* g++.dg/template/recurse3.C: Likewise for the erroneous
statement.
* g++.dg/template/non-dependent26.C: New test.
* g++.dg/warn/Wparentheses-32.C: New test.

libstdc++/ChangeLog:

* testsuite/26_numerics/random/discard_block_engine/cons/seed_seq2.cc:
Make seed_seq::called member mutable.
* 
testsuite/26_numerics/random/independent_bits_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/linear_congruential_engine/cons/seed_seq2.cc
Likewise.
* 
testsuite/26_numerics/random/mersenne_twister_engine/cons/seed_seq2.cc:
Likewise.
* testsuite/26_numerics/random/shuffle_order_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/subtract_with_carry_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/ext/random/simd_fast_mersenne_twister_engine/cons/seed_seq2.cc
Likewise.
---
  gcc/cp/semantics.cc   | 17 +++
  gcc/cp/typeck.cc  | 23 +++
  gcc/testsuite/g++.dg/cpp0x/static_assert15.C  |  2 +-
  gcc/testsuite/g++.dg/expr/unary2.C|  8 ++
  gcc/testsuite/g++.dg/template/init7.C |  2 +-
  .../g++.dg/template/non-dependent26.C | 25 +
  gcc/testsuite/g++.dg/template/recurse3.C  |  8 +++---
  gcc/testsuite/g++.dg/warn/Wparentheses-32.C   | 28 +++
  .../discard_block_engine/cons/seed_seq2.cc|  2 +-
  .../independent_bits_engine/cons/seed_seq2.cc |  2 +-
  .../cons/seed_seq2.cc |  2 +-
  .../mersenne_twister_engine/cons/seed_seq2.cc |  2 +-
  .../shuffle_order_engine/cons/seed_seq2.cc|  2 +-
  .../cons/seed_seq2.cc |  2 +-
  .../cons/seed_seq2.cc |  2 +-
  15 files changed, 91 insertions(+), 36 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/template/non-dependent26.C
  create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-32.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 0f7f4e87ae4..b57c1ac868b 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 459739d5866..74f5fced060 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -9739,15 +9739,15 @@ build_x_modify_expr (location_t loc, tree lhs, enum 
tree_code modifycode,
rhs = build_non_dependent_expr (rhs);
  }
  
-  if (modifycode != NOP_EXPR)

-{
-  tree op = build_nt (modifycode, NULL_TREE, NULL_TREE);
-  tree rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
+  tree rval;
+  if (modifycode == NOP_EXPR)
+rval = cp_build_modify_expr (loc, lhs, modifycode, rhs, complain);
+  else
+rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
 lhs, rhs, op, lookups, , complain);
-  if (rval)
-   {
if (rval == error_mark_node)
-   return rval;
+return error_mark_node;
+  if (modifycode != NOP_EXPR)
  suppress_warning (rval /* What warning? */);


Did you try disabling this to see if it's still needed?

Jason

[PATCH] c++: non-dependent assignment checking [PR63198, PR18474]

2023-09-17 Thread Patrick Palka via Gcc-patches 
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Patch generatde with -w to avoid noisy whitespace changes.

-- >8 --

This patch makes us recognize and check non-dependent simple assigments
ahead of time, like we already do for compound assignments.  This means
the templated representation of such assignments will now usually have
an implicit INDIRECT_REF (due to the reference return type), which the
-Wparentheses code needs to handle.  As a drive-by improvement, this
patch also makes maybe_convert_cond issue -Wparentheses warnings ahead
of time.

This revealed some libstdc++ tests were attempting to modify a data
member from a uninstantiated const member function; naively fixed by
making the data member mutable.

PR c++/63198
PR c++/18474

gcc/cp/ChangeLog:

* semantics.cc (maybe_convert_cond): Look through implicit
INDIRECT_REF when deciding whether to issue a -Wparentheses
warning, and consider templated assignment expressions as well.
(finish_parenthesized_expr): Look through implicit INDIRECT_REF
when suppressing -Wparentheses warning.
* typeck.cc (build_x_modify_expr): Check simple assignments
ahead time too, not just compound assignments.  Give the second
operand of MODOP_EXPR a non-null type so that it's not considered
always instantiation-dependent.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/static_assert15.C: Expect diagnostic for
non-constant static_assert condition.
* g++.dg/expr/unary2.C: Remove xfails.
* g++.dg/template/init7.C: Make initializer type-dependent to
preserve intent of test.
* g++.dg/template/recurse3.C: Likewise for the erroneous
statement.
* g++.dg/template/non-dependent26.C: New test.
* g++.dg/warn/Wparentheses-32.C: New test.

libstdc++/ChangeLog:

* testsuite/26_numerics/random/discard_block_engine/cons/seed_seq2.cc:
Make seed_seq::called member mutable.
* 
testsuite/26_numerics/random/independent_bits_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/linear_congruential_engine/cons/seed_seq2.cc
Likewise.
* 
testsuite/26_numerics/random/mersenne_twister_engine/cons/seed_seq2.cc:
Likewise.
* testsuite/26_numerics/random/shuffle_order_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/26_numerics/random/subtract_with_carry_engine/cons/seed_seq2.cc:
Likewise.
* 
testsuite/ext/random/simd_fast_mersenne_twister_engine/cons/seed_seq2.cc
Likewise.
---
 gcc/cp/semantics.cc   | 17 +++
 gcc/cp/typeck.cc  | 23 +++
 gcc/testsuite/g++.dg/cpp0x/static_assert15.C  |  2 +-
 gcc/testsuite/g++.dg/expr/unary2.C|  8 ++
 gcc/testsuite/g++.dg/template/init7.C |  2 +-
 .../g++.dg/template/non-dependent26.C | 25 +
 gcc/testsuite/g++.dg/template/recurse3.C  |  8 +++---
 gcc/testsuite/g++.dg/warn/Wparentheses-32.C   | 28 +++
 .../discard_block_engine/cons/seed_seq2.cc|  2 +-
 .../independent_bits_engine/cons/seed_seq2.cc |  2 +-
 .../cons/seed_seq2.cc |  2 +-
 .../mersenne_twister_engine/cons/seed_seq2.cc |  2 +-
 .../shuffle_order_engine/cons/seed_seq2.cc|  2 +-
 .../cons/seed_seq2.cc |  2 +-
 .../cons/seed_seq2.cc |  2 +-
 15 files changed, 91 insertions(+), 36 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/non-dependent26.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-32.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 0f7f4e87ae4..b57c1ac868b 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -881,13 +881,17 @@ maybe_convert_cond (tree cond)
   /* Do the conversion.  */
   cond = convert_from_reference (cond);
 
-  if ((TREE_CODE (cond) == MODIFY_EXPR || is_assignment_op_expr_p (cond))
+  tree inner = REFERENCE_REF_P (cond) ? TREE_OPERAND (cond, 0) : cond;
+  if ((TREE_CODE (inner) == MODIFY_EXPR
+   || (TREE_CODE (inner) == MODOP_EXPR
+  && TREE_CODE (TREE_OPERAND (inner, 1)) == NOP_EXPR)
+   || is_assignment_op_expr_p (inner))
   && warn_parentheses
-  && !warning_suppressed_p (cond, OPT_Wparentheses)
-  && warning_at (cp_expr_loc_or_input_loc (cond),
+  && !warning_suppressed_p (inner, OPT_Wparentheses)
+  && warning_at (cp_expr_loc_or_input_loc (inner),
 OPT_Wparentheses, "suggest parentheses around "
   "assignment used as truth value"))
-suppress_warning (cond, OPT_Wparentheses);
+suppress_warning (inner, OPT_Wparentheses);
 
   return condition_conversion (cond);
 }
@@ -2155,8 +2159,11 @@ cp_expr
 finish_parenthesized_expr (cp_expr expr)
 {
   if (EXPR_P (expr))
+{
+  tree inner = REFERENCE_REF_P