From patchwork Thu Nov 11 08:20:44 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 70783 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 24645B7122 for ; Thu, 11 Nov 2010 19:21:00 +1100 (EST) Received: (qmail 10877 invoked by alias); 11 Nov 2010 08:20:56 -0000 Received: (qmail 10862 invoked by uid 22791); 11 Nov 2010 08:20:54 -0000 X-SWARE-Spam-Status: No, hits=-5.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, TW_FN, TW_NV, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 11 Nov 2010 08:20:48 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oAB8KkaC020915 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 11 Nov 2010 03:20:46 -0500 Received: from adjoa.redhat.com (ovpn-113-27.phx2.redhat.com [10.3.113.27]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id oAB8KirA016225; Thu, 11 Nov 2010 03:20:45 -0500 From: Dodji Seketeli To: Jason Merrill Subject: Fix PR c++/45383 CC: GCC Patches X-URL: http://www.seketeli.net/~dodji Date: Thu, 11 Nov 2010 09:20:44 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Hello, In the exemple gcc/testsuite/g++.dg/conversion/cond4.C of the second patch below, G++ fails to consider the conversion function template template operator T*() of struct null so it fails to compile the line if (ptr == null). This is due to my fix for PR c++/42260. In that bug the following code was producing and ICE: struct A { template operator T*(); //#1 }; int i = *A();// <- ICE here. The ICE was because in build_new_op [while looking at the expression *A()] add_builtin_candidates was considering the conversion function template #1 as a candidate for user conversion and was eventually wrongly choosing it. I thought I could fix the problem by avoiding considering function templates when looking for candidate conversion functions. This current bug seems to argue that systematically ruling out function templates from the list of candidates in add_builtin_candidate was probably too strong of a measure. Furthermore, a comment of add_builtin_candidates reads: Here we generate a superset of the possible candidates for this particular case. That is a subset of the full set the standard defines, plus some other cases which the standard disallows. add_builtin_candidate will filter out the invalid set. add_builtin_candidates seems to be designed to first generate a superset of the possible candidates and then let add_building_candidate (without 's' this time) rule out some invalid candidates. So I went to look at why add_building_candidate wasn't filtering out the conversion function template. I suppose that for a conversion function 'T& operator*(T*)', T should be a complete type [the comment of that case in the add_building_candidate even says that too]. But the code doesn't enforce that. If we were looking at a type dependent expression, I guess this whole overloading business would be deferred to after the dependent expression is tsubsted. Incidentally, requiring that T be complete in that context is also similar to what add_building_candidate does for the operator CV12 T& operator->*(CV1 C1*, CV2 T C2::*). So the first patch below is a revert of the initial patch for PR c++/42260. The revert fixes this PR c++/45383. The second patch makes add_building_candidate require that the result of applying the operator T& operator*(T*) be a complete type, and that fixes PR c++/42260. I have bootstrapped and regtested both patches together on x86-unknown-linux gnu against trunk and I am about to do it for 4.5 too if the approach is OK. What do you think? diff --git a/gcc/cp/call.c b/gcc/cp/call.c index eb7247d..bf9439f 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -1093,7 +1093,7 @@ convert_class_to_reference (tree reference_type, tree s, tree expr, int flags) if (!expr) return NULL; - conversions = lookup_conversions (s, /*lookup_template_convs_p=*/true); + conversions = lookup_conversions (s); if (!conversions) return NULL; @@ -2464,8 +2464,7 @@ add_builtin_candidates (struct z_candidate **candidates, enum tree_code code, if (i == 0 && code == MODIFY_EXPR && code2 == NOP_EXPR) return; - convs = lookup_conversions (argtypes[i], - /*lookup_template_convs_p=*/false); + convs = lookup_conversions (argtypes[i]); if (code == COND_EXPR) { @@ -3028,8 +3027,7 @@ build_user_type_conversion_1 (tree totype, tree expr, int flags) reference to it)... */ } else - conv_fns = lookup_conversions (fromtype, - /*lookup_template_convs_p=*/true); + conv_fns = lookup_conversions (fromtype); } candidates = 0; @@ -3585,7 +3583,7 @@ build_op_call (tree obj, VEC(tree,gc) **args, tsubst_flags_t complain) LOOKUP_NORMAL, &candidates); } - convs = lookup_conversions (type, /*lookup_template_convs_p=*/true); + convs = lookup_conversions (type); for (; convs; convs = TREE_CHAIN (convs)) { diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 241805c..2a52742 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5161,7 +5161,7 @@ extern int at_function_scope_p (void); extern bool at_class_scope_p (void); extern bool at_namespace_scope_p (void); extern tree context_for_name_lookup (tree); -extern tree lookup_conversions (tree, bool); +extern tree lookup_conversions (tree); extern tree binfo_from_vbase (tree); extern tree binfo_for_vbase (tree, tree); extern tree look_for_overrides_here (tree, tree); diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index d2d6f4a..17ba540 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -1476,9 +1476,7 @@ build_expr_type_conversion (int desires, tree expr, bool complain) if (!TYPE_HAS_CONVERSION (basetype)) return NULL_TREE; - for (conv = lookup_conversions (basetype, /*lookup_template_convs_p=*/true); - conv; - conv = TREE_CHAIN (conv)) + for (conv = lookup_conversions (basetype); conv; conv = TREE_CHAIN (conv)) { int win = 0; tree candidate; diff --git a/gcc/cp/search.c b/gcc/cp/search.c index 370ddf6..c02800c 100644 --- a/gcc/cp/search.c +++ b/gcc/cp/search.c @@ -2440,13 +2440,10 @@ lookup_conversions_r (tree binfo, functions in this node were selected. This function is effectively performing a set of member lookups as lookup_fnfield does, but using the type being converted to as the unique key, rather than the - field name. - If LOOKUP_TEMPLATE_CONVS_P is TRUE, the returned TREE_LIST contains - the non-hidden user-defined template conversion functions too. */ + field name. */ tree -lookup_conversions (tree type, - bool lookup_template_convs_p) +lookup_conversions (tree type) { tree convs, tpl_convs; tree list = NULL_TREE; @@ -2473,9 +2470,6 @@ lookup_conversions (tree type, } } - if (lookup_template_convs_p == false) - tpl_convs = NULL_TREE; - for (; tpl_convs; tpl_convs = TREE_CHAIN (tpl_convs)) { tree probe, next; diff --git a/gcc/testsuite/g++.dg/conversion/cast2.C b/gcc/testsuite/g++.dg/conversion/cast2.C deleted file mode 100644 index 3868d74..0000000 --- a/gcc/testsuite/g++.dg/conversion/cast2.C +++ /dev/null @@ -1,11 +0,0 @@ -// Contributed by Dodji Seketeli -// Origin: PR c++/42260 -// { dg-do compile } - -struct A -{ - template operator T*(); -}; - -int i = *A();// { dg-error "no match" } - -- 1.7.2.3 From 6833062fd785fdaf8147b19f29a131ec8705d421 Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Wed, 10 Nov 2010 16:02:12 +0100 Subject: [PATCH 2/2] Fix PR c++/42260 and ensure PR c++/45383 is fixed gcc/cp/ c++/42260 * call.c (add_builtin_candidate): At this point the resulting type of an indirection operator should be complete. gcc/testsuite/ c++/42260 c++/45383 * g++.dg/conversion/cast2.C: New test. * g++.dg/conversion/cond4/C: Likewise. Ensures we don't regress on PR c++/45383 --- gcc/cp/call.c | 1 + gcc/testsuite/g++.dg/conversion/cast2.C | 9 +++++++++ gcc/testsuite/g++.dg/conversion/cond4.C | 31 +++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 0 deletions(-) create mode 100644 gcc/testsuite/g++.dg/conversion/cast2.C create mode 100644 gcc/testsuite/g++.dg/conversion/cond4.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index bf9439f..12d55b5 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -2022,6 +2022,7 @@ add_builtin_candidate (struct z_candidate **candidates, enum tree_code code, case INDIRECT_REF: if (TREE_CODE (type1) == POINTER_TYPE + && is_complete (TREE_TYPE (type1)) && (TYPE_PTROB_P (type1) || TREE_CODE (TREE_TYPE (type1)) == FUNCTION_TYPE)) break; diff --git a/gcc/testsuite/g++.dg/conversion/cast2.C b/gcc/testsuite/g++.dg/conversion/cast2.C new file mode 100644 index 0000000..ac83297 --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/cast2.C @@ -0,0 +1,9 @@ +// Origin: PR c++/42260 +// { dg-do compile } + +struct A +{ + template operator T*(); +}; + +int i = *A();// { dg-error "no match" } diff --git a/gcc/testsuite/g++.dg/conversion/cond4.C b/gcc/testsuite/g++.dg/conversion/cond4.C new file mode 100644 index 0000000..3bd6476 --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/cond4.C @@ -0,0 +1,31 @@ +// Origin: PR c++/45383 +// { dg-do run } + +struct null { + null() {} + template + operator T*() const { + return 0; + } + + template + operator T C::*() const { + return 0; + } +private: + null(const null&); + null& operator=(const null&); + void operator&() const; +}; + +static struct null null; + +int +main() +{ + int* ptr = null; + if (ptr == null) + return 0; + if (ptr != null) + return 1; +}