This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[C++ PATCH] classtype_has_nothrow_assign_or_copy_p is confusing


I found classtype_has_nothrow_assign_or_copy_p confusing, trying to figure out when it should return false and when true.

AFAICT what it's trying to tell you is if *all* the copy/move ctors or copy assignment ops are nothrow. Not, as its comment suggests that it has *at least one* nothrow variant.

If there are no assignment ops it returns false, but if there's at least one, but there is no copy-assignment op it'll return true. That seems wrong. (but perhaps that never happens, because the copy-op will be implicitly defined). It's certainly confusing.

Also the differing check on copy_fn_p's result is a difference that means nothing -- the -ve result means 'this is a broken copy/move ctor/assop', which IMHO puts it in the 'don't care' category.

This patch cleans it up to:

a) just consider well-formed fns (copy_fn_p returns > 0, but could just as easily become != 0)

b) only return true if it finds at least one function of interest -- and no functions of interest throw.

This caused no regressions, I'll commit in a few days unless someone notices a mistake in the above logic.

nathan
--
Nathan Sidwell
2017-07-03  Nathan Sidwell  <nathan@acm.org>

	* semantics.c (classtype_has_nothrow_assign_or_copy_p): Clarify
	semantics, simplify implementation.

Index: semantics.c
===================================================================
--- semantics.c	(revision 249925)
+++ semantics.c	(working copy)
@@ -9072,19 +9072,16 @@ finish_decltype_type (tree expr, bool id
 }
 
 /* Called from trait_expr_value to evaluate either __has_nothrow_assign or 
-   __has_nothrow_copy, depending on assign_p.  */
+   __has_nothrow_copy, depending on assign_p.  Returns true iff all
+   the copy {ctor,assign} fns are nothrow.  */
 
 static bool
 classtype_has_nothrow_assign_or_copy_p (tree type, bool assign_p)
 {
-  tree fns;
+  tree fns = NULL_TREE;
 
   if (assign_p)
-    {
-      fns = lookup_fnfields_slot (type, cp_assignment_operator_id (NOP_EXPR));
-      if (!fns)
-	return false;
-    } 
+    fns = lookup_fnfields_slot (type, cp_assignment_operator_id (NOP_EXPR));
   else if (TYPE_HAS_COPY_CTOR (type))
     {
       /* If construction of the copy constructor was postponed, create
@@ -9095,27 +9092,22 @@ classtype_has_nothrow_assign_or_copy_p (
 	lazily_declare_fn (sfk_move_constructor, type);
       fns = CLASSTYPE_CONSTRUCTORS (type);
     }
-  else
-    return false;
 
+  bool saw_copy = false;
   for (ovl_iterator iter (fns); iter; ++iter)
     {
       tree fn = *iter;
- 
-      if (assign_p)
+
+      if (copy_fn_p (fn) > 0)
 	{
-	  if (copy_fn_p (fn) == 0)
-	    continue;
+	  saw_copy = true;
+	  maybe_instantiate_noexcept (fn);
+	  if (!TYPE_NOTHROW_P (TREE_TYPE (fn)))
+	    return false;
 	}
-      else if (copy_fn_p (fn) <= 0)
-	continue;
-
-      maybe_instantiate_noexcept (fn);
-      if (!TYPE_NOTHROW_P (TREE_TYPE (fn)))
-	return false;
     }
 
-  return true;
+  return saw_copy;
 }
 
 /* Actually evaluates the trait.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]