C++ PATCH for c++/34158 (template placement delete)

C++ PATCH for c++/34158 (template placement delete)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

Our handling of delete completely failed to consider templates, so we 
would sometimes crash when we saw one (36406) and didn't use them when 
appropriate (34158).  This patch fixes placement delete matching to use 
instantiate_type rather than reinvent that wheel, and simplifies some 
other logic.

Tested x86_64-pc-linux-gnu, applied to trunk.

[-- Attachment #2: 34158.patch --]
[-- Type: text/x-patch, Size: 10234 bytes --]

2009-11-10  Jason Merrill  <jason@redhat.com>

	PR c++/34158
	PR c++/36406
	* call.c (non_placement_deallocation_fn_p): Split out...
	(build_op_delete_call): ...from here.  Use instantiate_type
	for placement delete.  Simplify logic.
	* pt.c (primary_template_instantiation_p): Non-static.
	* cp-tree.h: Declare it.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1aebaac..efee604 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4503,6 +4503,33 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3,
   return NULL_TREE;
 }
 
+/* Returns true iff T, an element of an OVERLOAD chain, is a usual
+   deallocation function (3.7.4.2 [basic.stc.dynamic.deallocation]).  */
+
+static bool
+non_placement_deallocation_fn_p (tree t)
+{
+  /* A template instance is never a usual deallocation function,
+     regardless of its signature.  */
+  if (TREE_CODE (t) == TEMPLATE_DECL
+      || primary_template_instantiation_p (t))
+    return false;
+
+  /* If a class T has a member deallocation function named operator delete
+     with exactly one parameter, then that function is a usual
+     (non-placement) deallocation function. If class T does not declare
+     such an operator delete but does declare a member deallocation
+     function named operator delete with exactly two parameters, the second
+     of which has type std::size_t (18.2), then this function is a usual
+     deallocation function.  */
+  t = FUNCTION_ARG_CHAIN (t);
+  if (t == void_list_node
+      || (t && same_type_p (TREE_VALUE (t), size_type_node)
+	  && TREE_CHAIN (t) == void_list_node))
+    return true;
+  return false;
+}
+
 /* Build a call to operator delete.  This has to be handled very specially,
    because the restrictions on what signatures match are different from all
    other call instances.  For a normal delete, only a delete taking (void *)
@@ -4528,8 +4555,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 		      tree alloc_fn)
 {
   tree fn = NULL_TREE;
-  tree fns, fnname, argtypes, type;
-  int pass;
+  tree fns, fnname, type, t;
 
   if (addr == error_mark_node)
     return error_mark_node;
@@ -4564,78 +4590,70 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 
   if (placement)
     {
-      /* Get the parameter types for the allocation function that is
-	 being called.  */
-      gcc_assert (alloc_fn != NULL_TREE);
-      argtypes = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (alloc_fn)));
-    }
-  else
-    {
-      /* First try it without the size argument.  */
-      argtypes = void_list_node;
-    }
+      /* "A declaration of a placement deallocation function matches the
+	 declaration of a placement allocation function if it has the same
+	 number of parameters and, after parameter transformations (8.3.5),
+	 all parameter types except the first are identical."
 
-  /* We make two tries at finding a matching `operator delete'.  On
-     the first pass, we look for a one-operator (or placement)
-     operator delete.  If we're not doing placement delete, then on
-     the second pass we look for a two-argument delete.  */
-  for (pass = 0; pass < (placement ? 1 : 2); ++pass)
-    {
-      /* Go through the `operator delete' functions looking for one
-	 with a matching type.  */
-      for (fn = BASELINK_P (fns) ? BASELINK_FUNCTIONS (fns) : fns;
-	   fn;
-	   fn = OVL_NEXT (fn))
-	{
-	  tree t;
+	 So we build up the function type we want and ask instantiate_type
+	 to get it for us.  */
+      t = FUNCTION_ARG_CHAIN (alloc_fn);
+      t = tree_cons (NULL_TREE, ptr_type_node, t);
+      t = build_function_type (void_type_node, t);
 
-	  /* The first argument must be "void *".  */
-	  t = TYPE_ARG_TYPES (TREE_TYPE (OVL_CURRENT (fn)));
-	  if (!same_type_p (TREE_VALUE (t), ptr_type_node))
-	    continue;
-	  t = TREE_CHAIN (t);
-	  /* On the first pass, check the rest of the arguments.  */
-	  if (pass == 0)
-	    {
-	      tree a = argtypes;
-	      while (a && t)
-		{
-		  if (!same_type_p (TREE_VALUE (a), TREE_VALUE (t)))
-		    break;
-		  a = TREE_CHAIN (a);
-		  t = TREE_CHAIN (t);
-		}
-	      if (!a && !t)
-		break;
-	    }
-	  /* On the second pass, look for a function with exactly two
-	     arguments: "void *" and "size_t".  */
-	  else if (pass == 1
-		   /* For "operator delete(void *, ...)" there will be
-		      no second argument, but we will not get an exact
-		      match above.  */
-		   && t
-		   && same_type_p (TREE_VALUE (t), size_type_node)
-		   && TREE_CHAIN (t) == void_list_node)
-	    break;
-	}
+      fn = instantiate_type (t, fns, tf_none);
+      if (fn == error_mark_node)
+	return NULL_TREE;
 
-      /* If we found a match, we're done.  */
-      if (fn)
-	break;
+      if (BASELINK_P (fn))
+	fn = BASELINK_FUNCTIONS (fn);
+
+      /* "If the lookup finds the two-parameter form of a usual deallocation
+	 function (3.7.4.2) and that function, considered as a placement
+	 deallocation function, would have been selected as a match for the
+	 allocation function, the program is ill-formed."  */
+      if (non_placement_deallocation_fn_p (fn))
+	{
+	  error ("non-placement deallocation function %q+D", fn);
+	  error ("selected for placement delete");
+	}
     }
+  else
+    /* "Any non-placement deallocation function matches a non-placement
+       allocation function. If the lookup finds a single matching
+       deallocation function, that function will be called; otherwise, no
+       deallocation function will be called."  */
+    for (t = BASELINK_P (fns) ? BASELINK_FUNCTIONS (fns) : fns;
+	 t; t = OVL_NEXT (t))
+      {
+	tree elt = OVL_CURRENT (t);
+	if (non_placement_deallocation_fn_p (elt))
+	  {
+	    fn = elt;
+	    /* "If a class T has a member deallocation function named
+	       operator delete with exactly one parameter, then that
+	       function is a usual (non-placement) deallocation
+	       function. If class T does not declare such an operator
+	       delete but does declare a member deallocation function named
+	       operator delete with exactly two parameters, the second of
+	       which has type std::size_t (18.2), then this function is a
+	       usual deallocation function."
+
+	       So (void*) beats (void*, size_t).  */
+	    if (FUNCTION_ARG_CHAIN (fn) == void_list_node)
+	      break;
+	  }
+      }
 
   /* If we have a matching function, call it.  */
   if (fn)
     {
-      /* Make sure we have the actual function, and not an
-	 OVERLOAD.  */
-      fn = OVL_CURRENT (fn);
+      gcc_assert (TREE_CODE (fn) == FUNCTION_DECL);
 
       /* If the FN is a member function, make sure that it is
 	 accessible.  */
-      if (DECL_CLASS_SCOPE_P (fn))
-	perform_or_defer_access_check (TYPE_BINFO (type), fn, fn);
+      if (BASELINK_P (fns))
+	perform_or_defer_access_check (BASELINK_BINFO (fns), fn, fn);
 
       /* Core issue 901: It's ok to new a type with deleted delete.  */
       if (DECL_DELETED_FN (fn) && alloc_fn)
@@ -4659,7 +4677,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 	  tree ret;
 	  VEC(tree,gc) *args = VEC_alloc (tree, gc, 2);
 	  VEC_quick_push (tree, args, addr);
-	  if (pass != 0)
+	  if (FUNCTION_ARG_CHAIN (fn) != void_list_node)
 	    VEC_quick_push (tree, args, size);
 	  ret = cp_build_function_call_vec (fn, &args, tf_warning_or_error);
 	  VEC_free (tree, gc, args);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 68be934..ca52bdf 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4853,6 +4853,7 @@ extern struct tinst_level *outermost_tinst_level(void);
 extern bool parameter_of_template_p		(tree, tree);
 extern void init_template_processing		(void);
 bool template_template_parameter_p		(const_tree);
+extern bool primary_template_instantiation_p    (const_tree);
 extern tree get_primary_template_innermost_parameters	(const_tree);
 extern tree get_template_innermost_arguments	(const_tree);
 extern tree get_template_argument_pack_elems	(const_tree);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3efeda8..0e688bf 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -191,7 +191,6 @@ static tree tsubst_decl (tree, tree, tsubst_flags_t);
 static void perform_typedefs_access_check (tree tmpl, tree targs);
 static void append_type_to_template_for_access_check_1 (tree, tree, tree);
 static hashval_t iterative_hash_template_arg (tree arg, hashval_t val);
-static bool primary_template_instantiation_p (const_tree);
 static tree listify (tree);
 static tree listify_autos (tree, tree);
 
@@ -2739,7 +2738,7 @@ make_ith_pack_parameter_name (tree name, int i)
 /* Return true if T is a primary function
    or class template instantiation.  */
 
-static bool
+bool
 primary_template_instantiation_p (const_tree t)
 {
   if (!t)
diff --git a/gcc/testsuite/g++.dg/init/placement4.C b/gcc/testsuite/g++.dg/init/placement4.C
new file mode 100644
index 0000000..9c61eca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/placement4.C
@@ -0,0 +1,32 @@
+// PR c++/34158
+
+typedef __SIZE_TYPE__ size_t;
+extern "C" void* malloc (size_t);
+extern "C" void free (void *);
+
+template <class T> class undef;
+
+struct A {
+  A() { throw 1; }
+};
+
+template<typename T> class Pool { };
+
+void *operator new(size_t size,Pool<int>& pool)
+{
+  return malloc(size);
+}
+
+template<typename T>
+void operator delete(void *p,Pool<T>& pool)
+{
+  undef<T> t;			// { dg-error "incomplete" }
+  free(p);
+}
+
+int main ()
+{
+  Pool<int> pool;
+  new (pool) A();		// { dg-message "instantiated" }
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/init/placement5.C b/gcc/testsuite/g++.dg/init/placement5.C
new file mode 100644
index 0000000..bb88239
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/placement5.C
@@ -0,0 +1,18 @@
+// 5.3.4/19: If the lookup finds the two-parameter form of a usual
+// deallocation function (3.7.4.2) and that function, considered as a
+// placement deallocation function, would have been selected as a match for
+// the allocation function, the program is ill-formed.
+
+typedef __SIZE_TYPE__ size_t;
+
+struct A
+{
+  A();
+  static void* operator new (size_t, size_t);
+  static void operator delete (void *, size_t); // { dg-error "non-placement" }
+};
+
+int main()
+{
+  A* ap = new (24) A;		// { dg-error "placement delete" }
+}

Re: C++ PATCH for c++/34158 (template placement delete)

[Jason Merrill]

I've now downgraded the error for a placement delete that is also a 
non-placement delete to a permerror.

Jason

C++ PATCH for c++/42115 (excessive placement delete errors)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

It occurred to me when responding to this PR that a reasonable 
interpretation of the standard would be that if there is an op delete 
(void *), the op delete (void *, size_t) isn't a usual deallocation 
function, so using it for placement delete is OK.

Tested x86_64-pc-linux-gnu, applied to trunk.


[-- Attachment #2: 42115.patch --]
[-- Type: text/x-patch, Size: 2367 bytes --]

commit 081898d3bd656946dc0ce3b3955e395dc02e4999
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Nov 19 22:32:34 2009 -0500

    	PR c++/42115
    	* call.c (build_op_delete_call): Don't complain about using
    	op delete (void *, size_t) for placement delete if there's an
    	op delete (void *).

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ca6bd0b..3b3ccb6 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4622,8 +4622,20 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 	 allocation function, the program is ill-formed."  */
       if (non_placement_deallocation_fn_p (fn))
 	{
+	  /* But if the class has an operator delete (void *), then that is
+	     the usual deallocation function, so we shouldn't complain
+	     about using the operator delete (void *, size_t).  */
+	  for (t = BASELINK_P (fns) ? BASELINK_FUNCTIONS (fns) : fns;
+	       t; t = OVL_NEXT (t))
+	    {
+	      tree elt = OVL_CURRENT (t);
+	      if (non_placement_deallocation_fn_p (elt)
+		  && FUNCTION_ARG_CHAIN (elt) == void_list_node)
+		goto ok;
+	    }
 	  permerror (0, "non-placement deallocation function %q+D", fn);
 	  permerror (input_location, "selected for placement delete");
+	ok:;
 	}
     }
   else
diff --git a/gcc/testsuite/g++.dg/init/placement5.C b/gcc/testsuite/g++.dg/init/placement5.C
index bb88239..1d540da 100644
--- a/gcc/testsuite/g++.dg/init/placement5.C
+++ b/gcc/testsuite/g++.dg/init/placement5.C
@@ -3,16 +3,30 @@
 // placement deallocation function, would have been selected as a match for
 // the allocation function, the program is ill-formed.
 
+// But we should only complain about using op delete (void *, size_t) for
+// placement delete if it would also be selected for normal delete, not if
+// there's also an op delete (void *).
+
 typedef __SIZE_TYPE__ size_t;
 
 struct A
 {
   A();
-  static void* operator new (size_t, size_t);
-  static void operator delete (void *, size_t); // { dg-error "non-placement" }
+  void* operator new (size_t, size_t);
+  void operator delete (void *, size_t); // { dg-error "non-placement" }
+};
+
+struct B
+{
+  B();
+  void * operator new (size_t);
+  void * operator new (size_t, size_t);
+  void operator delete (void *);
+  void operator delete (void *, size_t);
 };
 
 int main()
 {
   A* ap = new (24) A;		// { dg-error "placement delete" }
+  B* bp = new (24) B;
 }