Bootstrap and regtesting in progress on x86_64-pc-linux-gnu, does this
look OK for trunk if successful?

-- >8 --

Since r14-4977-g0f2e2080685e75 the -Wparentheses warning now undesirably
warns on the idiom

Wparentheses-34.C:9:14: warning: suggest parentheses around assignment used as 
truth value [-Wparentheses]
    9 |   b = v[i] = true;
      |              ^~~~

where v has type std::vector<bool>.  That commit intended to only extend
the existing diagnostics so that they happen in a template context as
well, but the refactoring of is_assignment_op_expr_p caused us for this
particular -Wparentheses warning (from convert_for_assignment) to now
consider user-defined operator= instead of just built-in operator=.  And
since std::vector<bool> is really a bitset, whose operator[] returns a
class type with such a user-defined operator= (taking bool), we now warn.

But arguably "boolish" class types should be treated like ordinary bool
as far as the warning is concerned.  To that end this patch relaxes the
warning for such types, specifically when the (class) type can be
(implicitly) converted to a and assigned from a bool.  This should cover
at least implementations of std::vector<bool>::reference.

gcc/cp/ChangeLog:

        * cp-tree.h (maybe_warn_unparenthesized_assignment): Add
        'nested_p' bool parameter.
        * semantics.cc (is_assignment_op_expr_p): Add 'rhs' bool
        parameter and set it accordingly.
        (class_type_is_boolish_cache): Define.
        (class_type_is_boolish): Define.
        (maybe_warn_unparenthesized_assignment): Add 'nested_p'
        bool parameter.  Relax the warning for nested assignments
        to boolean-like class types.
        (maybe_convert_cond): Pass nested_p=false to
        maybe_warn_unparenthesized_assignment.
        * typeck.cc (convert_for_assignment): Pass nested_p=true to
        maybe_warn_unparenthesized_assignment.

gcc/testsuite/ChangeLog:

        * g++.dg/warn/Wparentheses-34.C: New test.
---
 gcc/cp/cp-tree.h                            |   2 +-
 gcc/cp/semantics.cc                         | 106 ++++++++++++++++++--
 gcc/cp/typeck.cc                            |   2 +-
 gcc/testsuite/g++.dg/warn/Wparentheses-34.C |  31 ++++++
 4 files changed, 129 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-34.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1979572c365..97065cccf3d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7928,7 +7928,7 @@ extern tree lambda_regenerating_args              (tree);
 extern tree most_general_lambda                        (tree);
 extern tree finish_omp_target                  (location_t, tree, tree, bool);
 extern void finish_omp_target_clauses          (location_t, tree, tree *);
-extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t);
+extern void maybe_warn_unparenthesized_assignment (tree, bool, tsubst_flags_t);
 extern tree cp_check_pragma_unroll             (location_t, tree);
 
 /* in tree.cc */
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 64839b1ac87..92acd560fa4 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -839,10 +839,11 @@ finish_goto_stmt (tree destination)
   return add_stmt (build_stmt (input_location, GOTO_EXPR, destination));
 }
 
-/* Returns true if T corresponds to an assignment operator expression.  */
+/* Returns true if T corresponds to an assignment operator expression,
+   and sets *LHS to its left-hand-side operand if so.  */
 
 static bool
-is_assignment_op_expr_p (tree t)
+is_assignment_op_expr_p (tree t, tree *lhs)
 {
   if (t == NULL_TREE)
     return false;
@@ -850,7 +851,10 @@ is_assignment_op_expr_p (tree t)
   if (TREE_CODE (t) == MODIFY_EXPR
       || (TREE_CODE (t) == MODOP_EXPR
          && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR))
-    return true;
+    {
+      *lhs = TREE_OPERAND (t, 0);
+      return true;
+    }
 
   tree call = extract_call_expr (t);
   if (call == NULL_TREE
@@ -859,26 +863,107 @@ is_assignment_op_expr_p (tree t)
     return false;
 
   tree fndecl = cp_get_callee_fndecl_nofold (call);
-  return fndecl != NULL_TREE
-    && DECL_ASSIGNMENT_OPERATOR_P (fndecl)
-    && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR);
+  if (fndecl != NULL_TREE
+      && DECL_ASSIGNMENT_OPERATOR_P (fndecl)
+      && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR))
+    {
+      *lhs = get_nth_callarg (call, 0);
+      return true;
+    }
+
+  return false;
 }
 
+static GTY((deletable)) hash_map<tree, bool> *class_type_is_boolish_cache;
+
+/* Return true if the class type TYPE can be converted to and assigned
+   from a boolean.  */
+
+static bool
+class_type_is_boolish (tree type)
+{
+  type = TYPE_MAIN_VARIANT (type);
+  if (!COMPLETE_TYPE_P (type))
+    return false;
+
+  if (bool *r = hash_map_safe_get (class_type_is_boolish_cache, type))
+    return *r;
+
+  bool has_bool_conversion = false;
+  bool has_bool_assignment = false;
+
+  tree ops = lookup_conversions (type);
+  for (; ops; ops = TREE_CHAIN (ops))
+    {
+      tree op = TREE_VALUE (ops);
+      if (!DECL_NONCONVERTING_P (op)
+         && TREE_CODE (TREE_TYPE (DECL_NAME (op))) == BOOLEAN_TYPE)
+       {
+         has_bool_conversion = true;
+         break;
+       }
+    }
+
+  if (!has_bool_conversion)
+    {
+      hash_map_safe_put<true> (class_type_is_boolish_cache, type, false);
+      return false;
+    }
+
+  ops = lookup_fnfields (type, assign_op_identifier, /*protect=*/0, tf_none);
+  for (tree op : ovl_range (BASELINK_FUNCTIONS (ops)))
+    {
+      op = STRIP_TEMPLATE (op);
+      if (TREE_CODE (op) != FUNCTION_DECL)
+       continue;
+      tree parm = DECL_CHAIN (DECL_ARGUMENTS (op));
+      tree parm_type = non_reference (TREE_TYPE (parm));
+      if (TREE_CODE (parm_type) == BOOLEAN_TYPE)
+       {
+         has_bool_assignment = true;
+         break;
+       }
+    }
+
+  bool boolish = has_bool_conversion && has_bool_assignment;
+  hash_map_safe_put<true> (class_type_is_boolish_cache, type, boolish);
+  return boolish;
+}
+
+
 /* Maybe warn about an unparenthesized 'a = b' (appearing in a
-   boolean context where 'a == b' might have been intended).  */
+   boolean context where 'a == b' might have been intended).
+   NESTED_P is true if T appears as the RHS of another assignment.  */
 
 void
-maybe_warn_unparenthesized_assignment (tree t, tsubst_flags_t complain)
+maybe_warn_unparenthesized_assignment (tree t, bool nested_p,
+                                      tsubst_flags_t complain)
 {
   t = STRIP_REFERENCE_REF (t);
 
+  tree lhs;
   if ((complain & tf_warning)
       && warn_parentheses
-      && is_assignment_op_expr_p (t)
+      && is_assignment_op_expr_p (t, &lhs)
       /* A parenthesized expression would've had this warning
         suppressed by finish_parenthesized_expr.  */
       && !warning_suppressed_p (t, OPT_Wparentheses))
     {
+      /* In a = b = c, don't warn if b is a boolean-like class type.  */
+      if (nested_p)
+       {
+         /* The caller already checked this.  */
+         gcc_checking_assert (TREE_CODE (TREE_TYPE (t)) != BOOLEAN_TYPE);
+
+         if (TREE_CODE (lhs) == ADDR_EXPR)
+           lhs = TREE_OPERAND (lhs, 0);
+
+         tree lhs_type = non_reference (TREE_TYPE (lhs));
+         if (CLASS_TYPE_P (lhs_type)
+             && class_type_is_boolish (lhs_type))
+           return;
+       }
+
       warning_at (cp_expr_loc_or_input_loc (t), OPT_Wparentheses,
                  "suggest parentheses around assignment used as truth value");
       suppress_warning (t, OPT_Wparentheses);
@@ -903,7 +988,8 @@ maybe_convert_cond (tree cond)
   if (warn_sequence_point && !processing_template_decl)
     verify_sequence_points (cond);
 
-  maybe_warn_unparenthesized_assignment (cond, tf_warning_or_error);
+  maybe_warn_unparenthesized_assignment (cond, /*nested_p=*/false,
+                                        tf_warning_or_error);
 
   /* Do the conversion.  */
   cond = convert_from_reference (cond);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index a6e2f4ee7da..195114e716e 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10381,7 +10381,7 @@ convert_for_assignment (tree type, tree rhs,
      does not.  */
   if (TREE_CODE (type) == BOOLEAN_TYPE
       && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE)
-    maybe_warn_unparenthesized_assignment (rhs, complain);
+    maybe_warn_unparenthesized_assignment (rhs, /*nested_p=*/true, complain);
 
   if (complain & tf_warning)
     warn_for_address_of_packed_member (type, rhs);
diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-34.C 
b/gcc/testsuite/g++.dg/warn/Wparentheses-34.C
new file mode 100644
index 00000000000..2100c8a193d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wparentheses-34.C
@@ -0,0 +1,31 @@
+// Verify our -Wparentheses warning handles "boolish" class types
+// such as std::vector<bool>'s reference type the same as ordinary
+// bool.
+// { dg-additional-options "-Wparentheses" }
+
+#include <vector>
+
+void f(std::vector<bool> v, int i) {
+  bool b;
+  b = v[i] = true;
+  b = v[i] = v[i+1];
+
+  if (v[i] = 42) { }     // { dg-message "parentheses" }
+  if (v[i] = v[i+1]) { } // { dg-message "parentheses" }
+
+  if ((v[i] = 42)) { }
+  if ((v[i] = v[i+1])) { }
+}
+
+template<class>
+void ft(std::vector<bool> v, int i) {
+  bool b;
+  b = v[i] = true;
+  b = v[i] = v[i+1];
+
+  if (v[i] = 42) { }     // { dg-message "parentheses" }
+  if (v[i] = v[i+1]) { } // { dg-message "parentheses" }
+
+  if ((v[i] = 42)) { }
+  if ((v[i] = v[i+1])) { }
+}
-- 
2.43.0.121.g624eb90fa8

Reply via email to