[PR c++/84426] ICE after conflicting member decl

[Nathan Sidwell <nathan at acm dot org>]

The crash was happening because add_method wasn't telling it's caller 
something went wrong.  We then ended up with a vfunc and a regular field 
on the TYPE_MEMBERS list, but only the field in the 
CLASSTYPE_MEMBER_VEC.  So we knew there was a virtual func, but couldn't 
find it.

Fixed by having add_method return false.  But that leads to the 
possibility of having NULL slots during class definition time as 
get_member_slot would always create the slot.   I should have know that 
might bite somewhere else.  So fixed by breaking get_member_slot into a 
finder and an adder.

nathan
--
Nathan Sidwell

2018-02-27  Nathan Sidwell  <nathan@acm.org>

	PR c++/84426
	* name-lookup.h (get_member_slot): Rename ...
	(find_member_slot): ... here.
	(add_member_slot): New.
	* name-lookup.c (member_vec_linear_search): No need to check for
	NULL slot.
	(get_member_slot): Rename ...
	(find_member_slot): ... here.  Don't add slot for incomplete class.
	(add_member_slot): New.
	* class.c (add_method): Adjust get_member_slot rename.  Bail out
	if push_class_level_binding fails.  Create slot and grok
	properties once we're committed to insertion.

	PR c++/84426
	* g++.dg/lookup/pr84426.C: New.

Index: cp/class.c
===================================================================
--- cp/class.c	(revision 258041)
+++ cp/class.c	(working copy)
@@ -993,14 +993,11 @@ add_method (tree type, tree method, bool
   if (method == error_mark_node)
     return false;
 
-  /* Maintain TYPE_HAS_USER_CONSTRUCTOR, etc.  */
-  grok_special_member_properties (method);
-
-  tree *slot = get_member_slot (type, DECL_NAME (method));
-  tree current_fns = *slot;
-
   gcc_assert (!DECL_EXTERN_C_P (method));
 
+  tree *slot = find_member_slot (type, DECL_NAME (method));
+  tree current_fns = slot ? *slot : NULL_TREE;
+
   /* Check to see if we've already got this method.  */
   for (ovl_iterator iter (current_fns); iter; ++iter)
     {
@@ -1146,8 +1143,15 @@ add_method (tree type, tree method, bool
 
   current_fns = ovl_insert (method, current_fns, via_using);
 
-  if (!DECL_CONV_FN_P (method) && !COMPLETE_TYPE_P (type))
-    push_class_level_binding (DECL_NAME (method), current_fns);
+  if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
+      && !push_class_level_binding (DECL_NAME (method), current_fns))
+    return false;
+
+  if (!slot)
+    slot = add_member_slot (type, DECL_NAME (method));
+
+  /* Maintain TYPE_HAS_USER_CONSTRUCTOR, etc.  */
+  grok_special_member_properties (method);
 
   *slot = current_fns;
 
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 258041)
+++ cp/name-lookup.c	(working copy)
@@ -1146,17 +1146,9 @@ static tree
 member_vec_linear_search (vec<tree, va_gc> *member_vec, tree name)
 {
   for (int ix = member_vec->length (); ix--;)
-    /* We can get a NULL binding during insertion of a new method
-       name, because the identifier_binding machinery performs a
-       lookup.  If we find such a NULL slot, that's the thing we were
-       looking for, so we might as well bail out immediately.  */
     if (tree binding = (*member_vec)[ix])
-      {
-	if (OVL_NAME (binding) == name)
-	  return binding;
-      }
-    else
-      break;
+      if (OVL_NAME (binding) == name)
+	return binding;
 
   return NULL_TREE;
 }
@@ -1334,15 +1326,15 @@ get_class_binding (tree klass, tree name
 }
 
 /* Find the slot containing overloads called 'NAME'.  If there is no
-   such slot, create an empty one.  KLASS might be complete at this
-   point, in which case we need to preserve ordering.  Deals with
-   conv_op marker handling.  */
+   such slot and the class is complete, create an empty one, at the
+   correct point in the sorted member vector.  Otherwise return NULL.
+   Deals with conv_op marker handling.  */
 
 tree *
-get_member_slot (tree klass, tree name)
+find_member_slot (tree klass, tree name)
 {
   bool complete_p = COMPLETE_TYPE_P (klass);
-  
+
   vec<tree, va_gc> *member_vec = CLASSTYPE_MEMBER_VEC (klass);
   if (!member_vec)
     {
@@ -1389,24 +1381,34 @@ get_member_slot (tree klass, tree name)
 	break;
     }
 
-  /* No slot found.  Create one at IX.  We know in this case that our
-     caller will succeed in adding the function.  */
+  /* No slot found, add one if the class is complete.  */
   if (complete_p)
     {
-      /* Do exact allocation when complete, as we don't expect to add
-	 many.  */
+      /* Do exact allocation, as we don't expect to add many.  */
+      gcc_assert (name != conv_op_identifier);
       vec_safe_reserve_exact (member_vec, 1);
+      CLASSTYPE_MEMBER_VEC (klass) = member_vec;
       member_vec->quick_insert (ix, NULL_TREE);
+      return &(*member_vec)[ix];
     }
-  else
-    {
-      gcc_checking_assert (ix == length);
-      vec_safe_push (member_vec, NULL_TREE);
-    }
+
+  return NULL;
+}
+
+/* KLASS is an incomplete class to which we're adding a method NAME.
+   Add a slot and deal with conv_op marker handling.  */
+
+tree *
+add_member_slot (tree klass, tree name)
+{
+  gcc_assert (!COMPLETE_TYPE_P (klass));
+
+  vec<tree, va_gc> *member_vec = CLASSTYPE_MEMBER_VEC (klass);
+  vec_safe_push (member_vec, NULL_TREE);
   CLASSTYPE_MEMBER_VEC (klass) = member_vec;
 
-  tree *slot = &(*member_vec)[ix];
-  if (name == conv_op_identifier)
+  tree *slot = &member_vec->last ();
+  if (IDENTIFIER_CONV_OP_P (name))
     {
       /* Install the marker prefix.  */
       *slot = ovl_make (conv_op_marker, NULL_TREE);
Index: cp/name-lookup.h
===================================================================
--- cp/name-lookup.h	(revision 258041)
+++ cp/name-lookup.h	(working copy)
@@ -310,7 +310,8 @@ extern tree lookup_arg_dependent (tree,
 extern tree search_anon_aggr (tree, tree);
 extern tree get_class_binding_direct (tree, tree, int type_or_fns = -1);
 extern tree get_class_binding (tree, tree, int type_or_fns = -1);
-extern tree *get_member_slot (tree klass, tree name);
+extern tree *find_member_slot (tree klass, tree name);
+extern tree *add_member_slot (tree klass, tree name);
 extern void resort_type_member_vec (void *, void *,
 				    gt_pointer_operator, void *);
 extern void set_class_bindings (tree, unsigned extra = 0);
Index: testsuite/g++.dg/lookup/pr84426.C
===================================================================
--- testsuite/g++.dg/lookup/pr84426.C	(revision 0)
+++ testsuite/g++.dg/lookup/pr84426.C	(working copy)
@@ -0,0 +1,9 @@
+// PR c++/84426 ICE after error
+
+struct A
+{
+  int foo; // { dg-message "previous" }
+  virtual void foo(); // { dg-error "conflict" }
+};
+
+struct B : A {}; // ICED here