From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30644 invoked by alias); 26 Nov 2007 21:40:31 -0000 Received: (qmail 30595 invoked by uid 22791); 26 Nov 2007 21:40:30 -0000 X-Spam-Check-By: sourceware.org Received: from fmmailgate01.web.de (HELO fmmailgate01.web.de) (217.72.192.221) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 26 Nov 2007 21:40:23 +0000 Received: from smtp06.web.de (fmsmtp06.dlan.cinetic.de [172.20.5.172]) by fmmailgate01.web.de (Postfix) with ESMTP id 960C5B5B07DD; Mon, 26 Nov 2007 22:40:20 +0100 (CET) Received: from [87.78.134.28] (helo=xdsl-87-78-134-28.netcologne.de) by smtp06.web.de with esmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.108 #208) id 1Iwlgi-0002y1-00; Mon, 26 Nov 2007 22:40:20 +0100 Date: Tue, 27 Nov 2007 00:14:00 -0000 From: Familie Reichelt Subject: Re: [PATCH] Fix PR c++/8171: Comparing pointer-to-member-functions of derived classes To: Mark Mitchell cc: gcc-patches@gcc.gnu.org Message-ID: References: <4737AD61.9020702@codesourcery.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; CHARSET=us-ascii Content-Disposition: INLINE X-Sender: wir.reichelts@web.de Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2007-11/txt/msg01422.txt.bz2 On 11 Nov, Mark Mitchell wrote: Sorry for the long delay! > Volker Reichelt wrote: > >> :ADDPATCH C++: >> >> >> 2007-10-29 Volker Reichelt >> >> PR c++/8171 >> * typeck.c (comptypes) : 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) : 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 PR c++/8171 * typeck.c (structural_comptypes) : 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) : 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 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: +// { 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" } +} ===================================================================