This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]: PR29066 ptrmemfunc_vbit_in_delta is broken


Mark Mitchell wrote:

Thanks for asking!  Please put it in g++.dg/expr.  Also, do you actually
need:

+// { dg-options "" }

Normally, you only need that for a test case that isn't ISO-conforming, and I would think it should certainly be possible to test for the problem you're fixing here in an ISO-conforming manner.

No I don't need that. I used existing test cases as a reference and I noticed a bunch of test cases had it. I assumed it was placeholder for something.


Actually, I'd imagine I'm driving you crazy; I keep coming up with new
nit-picks. :-)  Here are the last ones, I promise:

Nah, I'd prefer to get it right.

+ op0 = cp_build_binary_op (TRUTH_ANDIF_EXPR, e1, e2);
+ op1 = cp_convert (TREE_TYPE (op0), integer_one_node); + }
+ else + {
+ op0 = build_ptrmemfunc_access_xpr (op0, pfn_identifier);

I don't think you compiled this code; this won't compile, since the function is spelled "build_ptrmemfunc_access_expr".

I think what happened here is that after I generated the diff, I opened the diff to double check the changes and somehow I clobbered the 'e'. I will be more careful in the future. For the record, I have been building and testing as I go.


What I think you need is:

 op0.pfn == op1.pfn
 && (/* They have the same delta or ...
     (op0.delta == op1.delta)
     /* ... both are NULL pointers.  */
     || (!op0.pfn && op0.delta & 1 == 0 && op1.delta & 1 == 0))

If you agree, you should update the comment above, explaining the code
that you are generating for this code.

I've made the change to generate that code. I see in the ptrmemfunc to ptrmemfunc comparison part that cp_build_binary_op is used for delta comparison, but build2 is used for the pfn comparison. Why the difference? Please let me know if there is any places in the new code where I could be using build2 instead of cp_build_binary_op.


Regards,

Ryan Mansfield

2006-11-14 Ryan Mansfield <rmansfield@qnx.com>

        PR c++/29066
        * typeck.c (build_binary_op):  Fix pointer to member function
        comparison for ptrmemfunc_vbit_in_delta targets.

2006-11-14 Ryan Mansfield <rmansfield@qnx.com>

        PR c++/29066
        * g++.dg/expr/pr29066.c: New.
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 118481)
+++ gcc/cp/typeck.c	(working copy)
@@ -3266,8 +3266,28 @@
 	}
       else if (TYPE_PTRMEMFUNC_P (type0) && null_ptr_cst_p (op1))
 	{
-	  op0 = build_ptrmemfunc_access_expr (op0, pfn_identifier);
-	  op1 = cp_convert (TREE_TYPE (op0), integer_zero_node);
+	  if (TARGET_PTRMEMFUNC_VBIT_LOCATION
+	      == ptrmemfunc_vbit_in_delta)
+	    {
+	      tree pfn0 = pfn_from_ptrmemfunc (op0);
+	      tree delta0 = build_ptrmemfunc_access_expr (op0,
+			 	 			  delta_identifier);
+	      tree e1 = cp_build_binary_op (EQ_EXPR,
+	  			            pfn0,	
+				      	    fold_convert (TREE_TYPE (pfn0),
+							  integer_zero_node));
+	      tree e2 = cp_build_binary_op (BIT_AND_EXPR, 
+					    delta0,
+				            integer_one_node);
+	      e2 = cp_build_binary_op (EQ_EXPR, e2, integer_zero_node);
+	      op0 = cp_build_binary_op (TRUTH_ANDIF_EXPR, e1, e2);
+	      op1 = cp_convert (TREE_TYPE (op0), integer_one_node); 
+	    }
+     	  else 
+	    {
+	      op0 = build_ptrmemfunc_access_expr (op0, pfn_identifier);
+	      op1 = cp_convert (TREE_TYPE (op0), integer_zero_node); 
+	    }
 	  result_type = TREE_TYPE (op0);
 	}
       else if (TYPE_PTRMEMFUNC_P (type1) && null_ptr_cst_p (op0))
@@ -3290,26 +3310,61 @@
 	  if (TREE_SIDE_EFFECTS (op1))
 	    op1 = save_expr (op1);
 
-	  /* We generate:
-
-	     (op0.pfn == op1.pfn
-	      && (!op0.pfn || op0.delta == 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.  */
 	  pfn0 = pfn_from_ptrmemfunc (op0);
 	  pfn1 = pfn_from_ptrmemfunc (op1);
 	  delta0 = build_ptrmemfunc_access_expr (op0,
 						 delta_identifier);
 	  delta1 = build_ptrmemfunc_access_expr (op1,
 						 delta_identifier);
-	  e1 = cp_build_binary_op (EQ_EXPR, delta0, delta1);
-	  e2 = cp_build_binary_op (EQ_EXPR,
-				   pfn0,
-				   cp_convert (TREE_TYPE (pfn0),
-					       integer_zero_node));
-	  e1 = cp_build_binary_op (TRUTH_ORIF_EXPR, e1, e2);
+	  if (TARGET_PTRMEMFUNC_VBIT_LOCATION
+	      == ptrmemfunc_vbit_in_delta)
+	    {
+	      /* We generate:
+
+		 (op0.pfn == op1.pfn
+		  && ((op0.delta == 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.  */
+
+	      e1 = cp_build_binary_op (BIT_AND_EXPR,
+				       delta0, 
+				       integer_one_node);
+	      e1 = cp_build_binary_op (EQ_EXPR, e1, integer_zero_node);
+	      e2 = cp_build_binary_op (BIT_AND_EXPR,
+				       delta1,
+				       integer_one_node);
+	      e2 = cp_build_binary_op (EQ_EXPR, e2, integer_zero_node);
+	      e1 = cp_build_binary_op (TRUTH_ANDIF_EXPR, e2, e1);
+	      e2 = cp_build_binary_op (EQ_EXPR,
+				       pfn0,
+				       fold_convert (TREE_TYPE (pfn0),
+						     integer_zero_node));
+	      e2 = cp_build_binary_op (TRUTH_ANDIF_EXPR, e2, e1);
+	      e1 = cp_build_binary_op (EQ_EXPR, delta0, delta1);
+	      e1 = cp_build_binary_op (TRUTH_ORIF_EXPR, e1, e2);
+	    }
+	  else
+	    {
+	      /* We generate:
+
+	         (op0.pfn == op1.pfn
+	         && (!op0.pfn || op0.delta == 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.  */
+ 
+    	      e1 = cp_build_binary_op (EQ_EXPR, delta0, delta1);
+	      e2 = cp_build_binary_op (EQ_EXPR,
+		      		       pfn0,
+			   	       fold_convert (TREE_TYPE (pfn0),
+						   integer_zero_node));
+	      e1 = cp_build_binary_op (TRUTH_ORIF_EXPR, e1, e2);
+	    }
 	  e2 = build2 (EQ_EXPR, boolean_type_node, pfn0, pfn1);
 	  e = cp_build_binary_op (TRUTH_ANDIF_EXPR, e2, e1);
 	  if (code == EQ_EXPR)
Index: gcc/testsuite/g++.dg/expr/pr29066.C
===================================================================
--- gcc/testsuite/g++.dg/expr/pr29066.C	(revision 0)
+++ gcc/testsuite/g++.dg/expr/pr29066.C	(revision 0)
@@ -0,0 +1,42 @@
+// PR c++/29066
+// Test pointer to member function comparison
+// { dg-do run }
+
+extern "C" void abort (void);
+
+struct X
+{
+  virtual void a(void)=0;
+};
+
+struct Z : public X
+{
+  void a(void) {};
+};
+
+
+void f(X *obj)
+{
+  void (X::*xp)(void) = 0;
+  void (X::*xp2)(void) = 0;
+
+  xp = &X::a;
+
+  if (xp == xp2)
+    {
+      abort(); 
+    } 
+
+  if (xp == 0)
+    {
+      abort();
+    }
+}
+
+int main(int argc, char* argv[])
+{
+  Z myobj;
+
+  f(&myobj);
+  return 0;
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]