[PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of derived classes

[Volker Reichelt <v.reichelt@netcologne.de>]

Hi,

the C++ frontend rejects comparisons (== or !=) between
pointer-to-member-functions where the involved classes are not
identical, but derived from each other like in this code snippet:

  struct A
  {
      void foo() {}
  };

  struct B : A {};

  int bar()
  {
      void (A::*pa)() = &A::foo;
      void (B::*pb)() = &B::foo;

      return pa == pb;
  }

The following patch fixes that by changing the case METHOD_TYPE in
in comptypes (from cp/typeck.c) to honor the flag STRICT when comparing
the first arguments of the functions. That's where the pointer to the
classes are stored. Actually the patch strips away the pointers (by
calling TREE_TYPE) in order to compare the underlying classes directly.

The function build_binary_op (also from cp/typeck.c) makes use of
this modified comparison for METHOD_TYPE by calling comptypes with
the parameter STRICT = COMPARE_BASE | COMPARE_DERIVED and not with
parameter STRICT = 0 (like the macro same_type_p does).
Instead of calling comptypes with the pointer-to-member-functions,
the pointers are stripped away before the call, so that the
member-functions are compared directly. If necessary, the suitable
delta offset when comparing member-functions of derived classes
is taken into account

The patch also adds to testcases: The first not only checks that
these comparisons are now accepted, but also checks that their
outcome is correct. The class hierarchy is chosen such the the delta
offset of the classes is non-zero. The second testcase checks that
comparisons where the underlying classes are not derived from each
other are rejected.

Bootstrapped and regtested on i686-pc-linux-gnu.
Ok for mainline?

Regards,
Volker

:ADDPATCH C++:


2007-10-29  Volker Reichelt  <v.reichelt@netcologne.de>

	PR c++/8171
	* typeck.c (comptypes) <METHOD_TYPE>: Do not fall-through to
	FUNCTION_TYPE, but use a slightly modified version: Compare the
	class-pointers according to the parameter STRICT.
	(build_binary_op) <EQ_EXPR, NE_EXPR>: Allow derived classes when
	comparing ptr-to-member-functions.  Strip pointer before calling
	comptypes.  Add delta offset if necessary.

===================================================================
--- gcc/gcc/cp/typeck.c	2006-03-14 18:36:29 +0100
+++ gcc/gcc/cp/typeck.c	2006-04-21 01:45:33 +0200
@@ -1028,6 +1028,16 @@ comptypes (tree t1, tree t2, int strict)
       break;
 
     case METHOD_TYPE:
+      if (!same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
+	return false;
+      if (!comptypes (TREE_TYPE (TREE_VALUE (TYPE_ARG_TYPES (t1))),
+		      TREE_TYPE (TREE_VALUE (TYPE_ARG_TYPES (t2))), strict))
+	return false;
+      if (!compparms (TREE_CHAIN (TYPE_ARG_TYPES (t1)),
+		      TREE_CHAIN (TYPE_ARG_TYPES (t2))))
+	return false;
+      break;
+
     case FUNCTION_TYPE:
       if (!same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
 	return false;
@@ -3393,7 +3403,9 @@ build_binary_op (enum tree_code code,
       else if (TYPE_PTRMEMFUNC_P (type1) && null_ptr_cst_p (op0))
 	return cp_build_binary_op (code, op1, op0);
       else if (TYPE_PTRMEMFUNC_P (type0) && TYPE_PTRMEMFUNC_P (type1)
-	       && same_type_p (type0, type1))
+	       && comptypes (TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type0)),
+			     TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type1)),
+			     COMPARE_BASE | COMPARE_DERIVED))
 	{
 	  /* E will be the final comparison.  */
 	  tree e;
@@ -3422,13 +3434,25 @@ build_binary_op (enum tree_code code, tr
 	      /* We generate:
 
 		 (op0.pfn == op1.pfn
-		  && ((op0.delta == op1.delta)
-     		       || (!op0.pfn && op0.delta & 1 == 0 
+		  && ((op0.delta + n == op1.delta)
+		       || (!op0.pfn && op0.delta & 1 == 0
 			   && op1.delta & 1 == 0))
 
-	         The reason for the `!op0.pfn' bit is that a NULL
-	         pointer-to-member is any member with a zero PFN and
-	         LSB of the DELTA field is 0.  */
+		 N is the DELTA offset between the involved classes.
+		 The reason for the `!op0.pfn' bit is that a NULL
+		 pointer-to-member is any member with a zero PFN and
+		 LSB of the DELTA field is 0.  */
+
+	      e1 = get_delta_difference (TYPE_PTRMEMFUNC_OBJECT_TYPE (type0),
+					 TYPE_PTRMEMFUNC_OBJECT_TYPE (type1),
+					 /*force=*/true, /*c_cast_p=*/0);
+	      if (!integer_zerop (e1))
+		{
+		  /* Adjust offset.  */
+		  e1 = cp_build_binary_op (LSHIFT_EXPR, e1, integer_one_node);
+		  /* Compute op0.delta + n.  */
+		  delta0 = cp_build_binary_op (PLUS_EXPR, delta0, e1);
+		}
 
 	      e1 = cp_build_binary_op (BIT_AND_EXPR,
 				       delta0, 
@@ -3451,12 +3475,19 @@ build_binary_op (enum tree_code code, tr
 	    {
 	      /* We generate:
 
-	         (op0.pfn == op1.pfn
-	         && (!op0.pfn || op0.delta == op1.delta))
+		 (op0.pfn == op1.pfn
+		  && (!op0.pfn || op0.delta + n == op1.delta))
 
-	         The reason for the `!op0.pfn' bit is that a NULL
-	         pointer-to-member is any member with a zero PFN; the
-	         DELTA field is unspecified.  */
+		 N is the DELTA offset between the involved classes.
+		 The reason for the `!op0.pfn' bit is that a NULL
+		 pointer-to-member is any member with a zero PFN; the
+		 DELTA field is unspecified.  */
+
+	      e1 = get_delta_difference (TYPE_PTRMEMFUNC_OBJECT_TYPE (type0),
+					 TYPE_PTRMEMFUNC_OBJECT_TYPE (type1),
+					 /*force=*/true, /*c_cast_p=*/0);
+	      if (!integer_zerop (e1))
+		delta0 = cp_build_binary_op (PLUS_EXPR, delta0, e1);
  
     	      e1 = cp_build_binary_op (EQ_EXPR, delta0, delta1);
 	      e2 = cp_build_binary_op (EQ_EXPR,
===================================================================

2007-10-29  Volker Reichelt  <v.reichelt@netcologne.de>

	PR c++/8171
	* g++.dg/conversion/ptrmemfun1.C: New test.
	* g++.dg/conversion/ptrmemfun2.C: New test.

===================================================================
--- gcc/gcc/testsuite/g++.dg/conversion/ptrmemfun1.C	2005-08-29 00:25:44 +0200
+++ gcc/gcc/testsuite/g++.dg/conversion/ptrmemfun1.C	2006-04-21 17:17:12 +0200
@@ -0,0 +1,43 @@
+// PR c++/8171
+// Origin: <zyzstar@ibl.sk>
+// { dg-do run }
+
+extern "C" void abort();
+
+struct A
+{
+  int i;
+  void foo() {}
+  void bar() {}
+  void baz() {}
+};
+
+struct B
+{
+  int j;
+};
+
+struct C : B, A
+{
+  void baz() {}
+};
+
+int main()
+{
+  void (A::*pa)() = &A::foo;
+  void (C::*pb)() = &C::foo;
+  void (C::*pc)() = &C::bar;
+  void (A::*pd)() = &A::baz;
+  void (C::*pe)() = &C::baz;
+
+  if (pa != pb || pb != pa)
+    abort();
+  if (pa == pc || pc == pa)
+    abort();
+  if (pb == pc || pc == pb)
+    abort();
+  if (pd == pe || pe == pd)
+    abort();
+
+  return 0;
+}
===================================================================
--- gcc/gcc/testsuite/g++.dg/conversion/ptrmemfun2.C	2005-08-29 00:25:44 +0200
+++ gcc/gcc/testsuite/g++.dg/conversion/ptrmemfun2.C	2006-04-21 18:22:20 +0200
@@ -0,0 +1,19 @@
+// PR c++/8171
+// { dg-do compile }
+
+struct A
+{
+  void foo() {}
+};
+
+struct B : A {};
+
+struct C : A {};
+
+int main()
+{
+  void (B::*pb)() = &B::foo;
+  void (C::*pc)() = &C::foo;
+
+  return pb == pc;  // { dg-error "invalid operands" }
+}
===================================================================