public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of  derived classes
@ 2007-10-30  9:23 Volker Reichelt
  2007-11-12  2:49 ` Mark Mitchell
  0 siblings, 1 reply; 6+ messages in thread
From: Volker Reichelt @ 2007-10-30  9:23 UTC (permalink / raw)
  To: gcc-patches

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" }
+}
===================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions  of  derived classes
  2007-10-30  9:23 [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of derived classes Volker Reichelt
@ 2007-11-12  2:49 ` Mark Mitchell
  2007-11-27  0:14   ` Familie Reichelt
  2007-11-30 17:00   ` Ollie Wild
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Mitchell @ 2007-11-12  2:49 UTC (permalink / raw)
  To: Volker Reichelt; +Cc: gcc-patches

Volker Reichelt wrote:

> :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.

How do we get a METHOD_TYPE down into comptypes?  Did we start with a
pointer-to-member function type, and then pull the METHOD_TYPE out of that?

Your patch is probably OK, but I'm always unhappy to see METHOD_TYPEs
moving around the front end because usually these come from
pointers-to-members, and it's much easier to see that we're following
the standard if we operate on those types directly by, for example,
using TYPE_PTRMEM_CLASS_TYPE.  Would you kind walking me through how we
get here to the comptypes change?  I'm not entirely convinced we even
need METHOD_TYPEs at all -- but that, of course, is a big change, not to
be done now.

Also, I'd be happier with the build_binary_op code if we did it more
obviously following the standard.  In particular, call common_type
(passing in the two pointer-to-member types), convert both arguments to
that type, and then compare them using the existing code.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of   derived classes
  2007-11-12  2:49 ` Mark Mitchell
@ 2007-11-27  0:14   ` Familie Reichelt
  2007-11-30 17:00   ` Ollie Wild
  1 sibling, 0 replies; 6+ messages in thread
From: Familie Reichelt @ 2007-11-27  0:14 UTC (permalink / raw)
  To: Mark Mitchell; +Cc: gcc-patches

On 11 Nov, Mark Mitchell wrote:

Sorry for the long delay!

> Volker Reichelt wrote:
> 
>> :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.
> 
> How do we get a METHOD_TYPE down into comptypes?  Did we start with a
> pointer-to-member function type, and then pull the METHOD_TYPE out of that?
> 
> Your patch is probably OK, but I'm always unhappy to see METHOD_TYPEs
> moving around the front end because usually these come from
> pointers-to-members, and it's much easier to see that we're following
> the standard if we operate on those types directly by, for example,
> using TYPE_PTRMEM_CLASS_TYPE.  Would you kind walking me through how we
> get here to the comptypes change?  I'm not entirely convinced we even
> need METHOD_TYPEs at all -- but that, of course, is a big change, not to
> be done now.

In order to compare the pointer-to-member-functions we have to compare
the classes to which the member functions belong and the member functions.
The problem is that the first arguments of the functions will differ if
we compare member functions of different classes, while the rest of the
arguments must be identical. I'd be happy to call a function that performs
the checks for me but could find such a thing in the C++ frontend.

Therefore I messed with comptypes to implement the comparison myself:
The METHOD_TYPE case compares the return type, then the first argument
of the functions (allowing derived classes) and finally the remaining
arguments. Do you have a better suggestion?

> Also, I'd be happier with the build_binary_op code if we did it more
> obviously following the standard.

Me, too.

> In particular, call common_type (passing in the two pointer-to-member
> types), convert both arguments to that type, and then compare them
> using the existing code.

Following your suggestion, this seems to do the trick:

  if (!same_type_p (type0, type1))
    {
      type0 = common_type (type0, type1);
      op0 = convert_ptrmem (type0, op0, /*allow_inverse_p=*/true,
                            /*c_cast_p=*/false);
      op1 = convert_ptrmem (type0, op1, /*allow_inverse_p=*/true,
                            /*c_cast_p=*/false);
    }


Btw, here's an updated version of the patch with the simplified code
in build_binary_op.

Regards,
Volker


2007-11-26  Volker Reichelt  <v.reichelt@netcologne.de>

	PR c++/8171
	* typeck.c (structural_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.  Convert pointers to the common base class, 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 @@ structural_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;
@@ -3405,6 +3417,15 @@ build_binary_op (enum tree_code code, tr
 	  tree delta0;
 	  tree delta1;
 
+	  if (!same_type_p (type0, type1))
+	    {
+	      type0 = common_type (type0, type1);
+	      op0 = convert_ptrmem (type0, op0, /*allow_inverse_p=*/true,
+	      			    /*c_cast_p=*/false);
+	      op1 = convert_ptrmem (type0, op1, /*allow_inverse_p=*/true,
+				    /*c_cast_p=*/false);
+	    }
+
 	  if (TREE_SIDE_EFFECTS (op0))
 	    op0 = save_expr (op0);
 	  if (TREE_SIDE_EFFECTS (op1))
===================================================================

2007-11-26  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" }
+}
===================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of derived classes
  2007-11-12  2:49 ` Mark Mitchell
  2007-11-27  0:14   ` Familie Reichelt
@ 2007-11-30 17:00   ` Ollie Wild
  2007-11-30 23:03     ` Mark Mitchell
  1 sibling, 1 reply; 6+ messages in thread
From: Ollie Wild @ 2007-11-30 17:00 UTC (permalink / raw)
  To: Mark Mitchell; +Cc: Volker Reichelt, gcc-patches

On Nov 11, 2007 5:33 PM, Mark Mitchell <mark@codesourcery.com> wrote:
>
> How do we get a METHOD_TYPE down into comptypes?  Did we start with a
> pointer-to-member function type, and then pull the METHOD_TYPE out of that?

I was digging through bugzilla and noticed that I've also submitted a
patch that fixes the same problem (See
http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01544.html.).

My patch is quite a bit simpler than this one, and it passes both of
this patch's tests (except the error generated on line 18 of
ptrmemfun2.C reads as "error: comparison between distinct
pointer-to-member types 'void (B::*)()' and 'void (C::*)()' lacks a
cast").  I'll let Mark decide which fix he thinks is preferable.

Sorry for stepping on toes here.

Ollie

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions  of derived classes
  2007-11-30 17:00   ` Ollie Wild
@ 2007-11-30 23:03     ` Mark Mitchell
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Mitchell @ 2007-11-30 23:03 UTC (permalink / raw)
  To: Ollie Wild; +Cc: Volker Reichelt, gcc-patches

Ollie Wild wrote:

> My patch is quite a bit simpler than this one, and it passes both of
> this patch's tests (except the error generated on line 18 of
> ptrmemfun2.C reads as "error: comparison between distinct
> pointer-to-member types 'void (B::*)()' and 'void (C::*)()' lacks a
> cast").  I'll let Mark decide which fix he thinks is preferable.

Let's go with Ollie's patch.  It is indeed simpler.

Ollie, why did you use

+	  if (TREE_TYPE (op0) != type)

rather than:

  if (!same_type_p (TREE_TYPE (op0)), type)

?  Virtually all uses of pointer equality for types are bugs, and I
think in this case same_type_p is better, but I want to know if you had
a particular reason.

The patch is OK with that change (in both places you use it), or perhaps
without the change, if you can persuade me. :-)

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of   derived classes
@ 2007-11-27  6:19 Volker Reichelt
  0 siblings, 0 replies; 6+ messages in thread
From: Volker Reichelt @ 2007-11-27  6:19 UTC (permalink / raw)
  To: gcc-patches

Since my mailer screwed up the last message, I'm resending the message
below. Apologies, if you receive the message twice.

On 11 Nov, Mark Mitchell wrote:

Sorry for the long delay!

> Volker Reichelt wrote:
> 
>> :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.
> 
> How do we get a METHOD_TYPE down into comptypes?  Did we start with a
> pointer-to-member function type, and then pull the METHOD_TYPE out of that?
> 
> Your patch is probably OK, but I'm always unhappy to see METHOD_TYPEs
> moving around the front end because usually these come from
> pointers-to-members, and it's much easier to see that we're following
> the standard if we operate on those types directly by, for example,
> using TYPE_PTRMEM_CLASS_TYPE.  Would you kind walking me through how we
> get here to the comptypes change?  I'm not entirely convinced we even
> need METHOD_TYPEs at all -- but that, of course, is a big change, not to
> be done now.

In order to compare the pointer-to-member-functions we have to compare
the classes to which the member functions belong and the member functions.
The problem is that the first arguments of the functions will differ if
we compare member functions of different classes, while the rest of the
arguments must be identical. I'd be happy to call a function that performs
the checks for me but could find such a thing in the C++ frontend.

Therefore I messed with comptypes to implement the comparison myself:
The METHOD_TYPE case compares the return type, then the first argument
of the functions (allowing derived classes) and finally the remaining
arguments. Do you have a better suggestion?

> Also, I'd be happier with the build_binary_op code if we did it more
> obviously following the standard.

Me, too.

> In particular, call common_type (passing in the two pointer-to-member
> types), convert both arguments to that type, and then compare them
> using the existing code.

Following your suggestion, this seems to do the trick:

  if (!same_type_p (type0, type1))
    {
      type0 = common_type (type0, type1);
      op0 = convert_ptrmem (type0, op0, /*allow_inverse_p=*/true,
                            /*c_cast_p=*/false);
      op1 = convert_ptrmem (type0, op1, /*allow_inverse_p=*/true,
                            /*c_cast_p=*/false);
    }


Btw, here's an updated version of the patch with the simplified code
in build_binary_op.

Regards,
Volker


2007-11-26  Volker Reichelt  <v.reichelt@netcologne.de>

	PR c++/8171
	* typeck.c (structural_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.  Convert pointers to the common base class, 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 @@ structural_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;
@@ -3405,6 +3417,15 @@ build_binary_op (enum tree_code code, tr
 	  tree delta0;
 	  tree delta1;
 
+	  if (!same_type_p (type0, type1))
+	    {
+	      type0 = common_type (type0, type1);
+	      op0 = convert_ptrmem (type0, op0, /*allow_inverse_p=*/true,
+	      			    /*c_cast_p=*/false);
+	      op1 = convert_ptrmem (type0, op1, /*allow_inverse_p=*/true,
+				    /*c_cast_p=*/false);
+	    }
+
 	  if (TREE_SIDE_EFFECTS (op0))
 	    op0 = save_expr (op0);
 	  if (TREE_SIDE_EFFECTS (op1))
===================================================================

2007-11-26  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" }
+}
===================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-11-30 20:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-30  9:23 [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of derived classes Volker Reichelt
2007-11-12  2:49 ` Mark Mitchell
2007-11-27  0:14   ` Familie Reichelt
2007-11-30 17:00   ` Ollie Wild
2007-11-30 23:03     ` Mark Mitchell
2007-11-27  6:19 Volker Reichelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).