[PATCH, v3] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek <jakub@redhat.com>]

On Wed, Sep 29, 2021 at 03:38:45PM -0400, Jason Merrill wrote:
> It ought to be possible to defer check_final_overrider, but it sounds
> awkward.
> 
> Or maybe_instantiate_noexcept could use the non-defining path in
> build_comparison_op.
> 
> Maybe we want a maybe_synthesize_method that actually builds the function if
> the type is complete, or takes the non-defining path if not.

So something like this?
spaceship-synth8.C (apparently added by me, so how valid/invalid it is is
unclear) now doesn't ICE anymore, but without the change I've put there
is now rejected because std::strong_ordering::less etc. aren't found.
Previously when we were synthesizing it we did that before the FIELD_DECLs
for bases have been added, so those weren't compared, but now they actually
are compared.
After fixing the incomplete std::strong_ordering spaceship-synth8.C is now
accepted, but I'm afraid I'm getting lost in this - clang++ rejects that
testcase instead complaining that D has <=> operator, but has it pure virtual.
And, when maybe_instantiate_noexcept tries to find out if the defaulted
method would be implicitly deleted or not, when it does so before the class
is complete, seems it can check whether there are errors when comparing the
direct members of the class, but not errors about bases...

2021-09-30  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102490
	* cp-tree.h (build_comparison_op): Declare.
	* method.c (comp_info): Remove defining member.
	(comp_info::comp_info): Remove complain argument, don't initialize
	defining.
	(build_comparison_op): No longer static.  Add defining argument.
	Adjust comp_info construction.  Use defining instead of info.defining.
	Assert that if defining, ctype is a complete type.
	(synthesize_method, maybe_explain_implicit_delete,
	explain_implicit_non_constexpr): Adjust build_comparison_op callers.
	* class.c (check_bases_and_members): Don't call defaulted_late_check
	for sfk_comparison.
	(finish_struct_1): Call it here instead after class has been completed.
	* pt.c (maybe_instantiate_noexcept): For sfk_comparison of still
	incomplete classes, call build_comparison_op in non-defining mode
	instead of calling synthesize_method.

	* g++.dg/cpp2a/spaceship-synth8.C (std::strong_ordering): Provide more
	complete definition.
	(std::strong_ordering::less, std::strong_ordering::equal,
	std::strong_ordering::greater): Define.
	* g++.dg/cpp2a/spaceship-eq11.C: New test.
	* g++.dg/cpp2a/spaceship-eq12.C: New test.

--- gcc/cp/cp-tree.h.jj	2021-09-18 09:44:31.728743713 +0200
+++ gcc/cp/cp-tree.h	2021-09-30 18:39:10.416847290 +0200
@@ -7012,6 +7012,7 @@ extern bool maybe_explain_implicit_delet
 extern void explain_implicit_non_constexpr	(tree);
 extern bool deduce_inheriting_ctor		(tree);
 extern bool decl_remember_implicit_trigger_p	(tree);
+extern void build_comparison_op			(tree, bool, tsubst_flags_t);
 extern void synthesize_method			(tree);
 extern tree lazily_declare_fn			(special_function_kind,
 						 tree);
--- gcc/cp/method.c.jj	2021-09-30 09:22:46.323918164 +0200
+++ gcc/cp/method.c	2021-09-30 18:51:14.510744549 +0200
@@ -1288,21 +1288,16 @@ struct comp_info
 {
   tree fndecl;
   location_t loc;
-  bool defining;
   bool first_time;
   bool constexp;
   bool was_constexp;
   bool noex;
 
-  comp_info (tree fndecl, tsubst_flags_t &complain)
+  comp_info (tree fndecl)
     : fndecl (fndecl)
   {
     loc = DECL_SOURCE_LOCATION (fndecl);
 
-    /* We only have tf_error set when we're called from
-       explain_invalid_constexpr_fn or maybe_explain_implicit_delete.  */
-    defining = !(complain & tf_error);
-
     first_time = DECL_MAYBE_DELETED (fndecl);
     DECL_MAYBE_DELETED (fndecl) = false;
 
@@ -1364,12 +1359,12 @@ struct comp_info
    to use synthesize_method at the earliest opportunity and bail out if the
    function ends up being deleted.  */
 
-static void
-build_comparison_op (tree fndecl, tsubst_flags_t complain)
+void
+build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
 {
-  comp_info info (fndecl, complain);
+  comp_info info (fndecl);
 
-  if (!info.defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
+  if (!defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
     return;
 
   int flags = LOOKUP_NORMAL;
@@ -1384,6 +1379,7 @@ build_comparison_op (tree fndecl, tsubst
     lhs = convert_from_reference (lhs);
   rhs = convert_from_reference (rhs);
   tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+  gcc_assert (!defining || COMPLETE_TYPE_P (ctype));
 
   iloc_sentinel ils (info.loc);
 
@@ -1399,7 +1395,7 @@ build_comparison_op (tree fndecl, tsubst
     }
 
   tree compound_stmt = NULL_TREE;
-  if (info.defining)
+  if (defining)
     compound_stmt = begin_compound_stmt (0);
   else
     ++cp_unevaluated_operand;
@@ -1474,8 +1470,8 @@ build_comparison_op (tree fndecl, tsubst
 		break;
 	      tree idx;
 	      /* [1] array, no loop needed, just add [0] ARRAY_REF.
-		 Similarly if !info.defining.  */
-	      if (integer_zerop (maxval) || !info.defining)
+		 Similarly if !defining.  */
+	      if (integer_zerop (maxval) || !defining)
 		idx = size_zero_node;
 	      /* Some other array, will need runtime loop.  */
 	      else
@@ -1584,7 +1580,7 @@ build_comparison_op (tree fndecl, tsubst
 	  tree comp = comps[i];
 	  tree eq, retval = NULL_TREE, if_ = NULL_TREE;
 	  tree loop_indexes = NULL_TREE;
-	  if (info.defining)
+	  if (defining)
 	    {
 	      if (TREE_CODE (comp) == TREE_LIST)
 		{
@@ -1632,7 +1628,7 @@ build_comparison_op (tree fndecl, tsubst
 		comp = build_static_cast (input_location, rettype, comp,
 					  complain);
 	      info.check (comp);
-	      if (info.defining)
+	      if (defining)
 		{
 		  tree var = create_temporary_var (rettype);
 		  pushdecl (var);
@@ -1645,7 +1641,7 @@ build_comparison_op (tree fndecl, tsubst
 	    }
 	  tree ceq = contextual_conv_bool (eq, complain);
 	  info.check (ceq);
-	  if (info.defining)
+	  if (defining)
 	    {
 	      finish_if_stmt_cond (ceq, if_);
 	      finish_then_clause (if_);
@@ -1658,7 +1654,7 @@ build_comparison_op (tree fndecl, tsubst
 		finish_for_stmt (TREE_VALUE (loop_index));
 	    }
 	}
-      if (info.defining)
+      if (defining)
 	{
 	  tree val;
 	  if (code == EQ_EXPR)
@@ -1679,7 +1675,7 @@ build_comparison_op (tree fndecl, tsubst
 				NULL_TREE, NULL, complain);
       comp = contextual_conv_bool (comp, complain);
       info.check (comp);
-      if (info.defining)
+      if (defining)
 	{
 	  tree neg = build1 (TRUTH_NOT_EXPR, boolean_type_node, comp);
 	  finish_return_stmt (neg);
@@ -1692,12 +1688,12 @@ build_comparison_op (tree fndecl, tsubst
       tree comp2 = build_new_op (info.loc, code, flags, comp, integer_zero_node,
 				 NULL_TREE, NULL, complain);
       info.check (comp2);
-      if (info.defining)
+      if (defining)
 	finish_return_stmt (comp2);
     }
 
  out:
-  if (info.defining)
+  if (defining)
     finish_compound_stmt (compound_stmt);
   else
     --cp_unevaluated_operand;
@@ -1776,7 +1772,7 @@ synthesize_method (tree fndecl)
   else if (sfk == sfk_comparison)
     {
       /* Pass tf_none so the function is just deleted if there's a problem.  */
-      build_comparison_op (fndecl, tf_none);
+      build_comparison_op (fndecl, true, tf_none);
       need_body = false;
     }
 
@@ -2747,7 +2743,7 @@ maybe_explain_implicit_delete (tree decl
 	  inform (DECL_SOURCE_LOCATION (decl),
 		  "%q#D is implicitly deleted because the default "
 		  "definition would be ill-formed:", decl);
-	  build_comparison_op (decl, tf_warning_or_error);
+	  build_comparison_op (decl, false, tf_warning_or_error);
 	}
       else if (!informed)
 	{
@@ -2808,7 +2804,7 @@ explain_implicit_non_constexpr (tree dec
   if (sfk == sfk_comparison)
     {
       DECL_DECLARED_CONSTEXPR_P (decl) = true;
-      build_comparison_op (decl, tf_warning_or_error);
+      build_comparison_op (decl, false, tf_warning_or_error);
       DECL_DECLARED_CONSTEXPR_P (decl) = false;
     }
   else
--- gcc/cp/class.c.jj	2021-09-30 09:22:46.299918499 +0200
+++ gcc/cp/class.c	2021-09-30 18:40:52.330425343 +0200
@@ -6133,7 +6133,8 @@ check_bases_and_members (tree t)
 		 no longer ill-formed, it is defined as deleted instead.  */
 	      DECL_DELETED_FN (fn) = true;
 	  }
-	defaulted_late_check (fn);
+	if (special_function_p (fn) != sfk_comparison)
+	  defaulted_late_check (fn);
       }
 
   if (LAMBDA_TYPE_P (t))
@@ -7467,7 +7468,14 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthesize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison)
+	  defaulted_late_check (x);
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
--- gcc/cp/pt.c.jj	2021-09-29 10:07:41.815881043 +0200
+++ gcc/cp/pt.c	2021-09-30 18:48:09.091331517 +0200
@@ -25765,6 +25765,25 @@ maybe_instantiate_noexcept (tree fn, tsu
 	/* We're in start_preparsed_function, keep going.  */
 	return true;
 
+      if (special_function_p (fn) == sfk_comparison)
+	{
+	  tree lhs = DECL_ARGUMENTS (fn);
+	  if (is_this_parameter (lhs))
+	    lhs = cp_build_fold_indirect_ref (lhs);
+	  else
+	    lhs = convert_from_reference (lhs);
+	  tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+	  /* If the comparison type is still incomplete, don't synthesize the
+	     method, just see if it is not implicitly deleted.  */
+	  if (!COMPLETE_TYPE_P (ctype))
+	    {
+	      push_deferring_access_checks (dk_no_deferred);
+	      build_comparison_op (fn, false, tf_none);
+	      pop_deferring_access_checks ();
+	      return !DECL_MAYBE_DELETED (fn);
+	    }
+	}
+
       ++function_depth;
       synthesize_method (fn);
       --function_depth;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C.jj	2020-07-28 15:39:10.012756173 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C	2021-09-30 19:02:22.107430284 +0200
@@ -1,7 +1,18 @@
 // PR c++/94907
 // { dg-do compile { target c++20 } }
 
-namespace std { struct strong_ordering { }; }
+namespace std { struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
 
 struct E;
 struct D {
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-30 18:34:22.897858916 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-30 18:34:22.897858916 +0200
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-30 18:34:22.897858916 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-30 18:34:22.897858916 +0200
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"


	Jakub