From patchwork Wed Jan 20 03:30:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 570487 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id CC4C1140B96 for ; Wed, 20 Jan 2016 14:30:21 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=gBc2hKZX; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :date:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; q=dns; s=default; b=R8NMQPU37dzeFZQP P7e9mFV/mJ7Y90ZnDQXej2cq05F7zTFUm77dXTh/XMzZQeQjOzfPz5SAkIZF11zV kSlAZyJYAEON967AMmgp+I3TTDwuDgDjQrQwqBLGLfewfVV5VmRexNSl4jsjMOub K68+IPDr8UuuaccutPxVOLZM/sw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :date:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; s=default; bh=wFZ32yU5rsmDEaszbzRGNF xQFeo=; b=gBc2hKZXB5fyIlhS6bijlw3G6Cvx14/4+rZnaWH5N9tJbhoN2MUtUk E0JFg0HSWUqRUskZTX4u/+b1OiB9ADvW5DYMEW9zXljZ2kZ5YedleMUf63ZeGuYc a/ZHoU4ODgsuge9+kZvMlFwA5ZwREymXqcMcU8oMQparzbDiuEBPs= Received: (qmail 121909 invoked by alias); 20 Jan 2016 03:30:13 -0000 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 Received: (qmail 121883 invoked by uid 89); 20 Jan 2016 03:30:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=exhibit, stripping, well!, light X-HELO: mail-qg0-f44.google.com Received: from mail-qg0-f44.google.com (HELO mail-qg0-f44.google.com) (209.85.192.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 20 Jan 2016 03:30:10 +0000 Received: by mail-qg0-f44.google.com with SMTP id o11so596297751qge.2 for ; Tue, 19 Jan 2016 19:30:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; bh=hzcwcjKop3EUjT2rV2v1ztK/EgfmHAdXBs6RJbFAlmY=; b=Xu/yRmi4rbXyTnNtKMk1wZukdl4lhjMY8A/sDKegt1XJnR6bvrgzpleHslkoI1I4qP StvShNHjvey9Vu1ItzojsU8Fl41mM+7Ida9oxrTiVIoQkJArLJDk3ZM26AbvLmPGpDzO PLTnmB8S/bNCsmDK/8AXspTMsy6oO8vJdR4amX7z9RClZijusRO8vWT+coozTMhmXWo/ RCHiPDDAjRFp8D2UnzX98vgKo0BhST8IkPS1SDKVH6Qvc+ONWiSCP6fls7VDhNQ3n/2f q2sh2E8B4TBaav4b6v/TgkbQx4YqNrwbSn7tr6ZURVFA8T/SZcc+xbpBnrenQmMklhem iN4g== X-Gm-Message-State: ALoCoQmlBB8y/dPhIKlFG/xhk2Rcve1NXWVHYZt1uF9RENC4p4LO2j/nycsWTBriTKtIMZVQ2NcLjYt47dPyhK0ZhZhtd5SyXA== X-Received: by 10.140.174.2 with SMTP id u2mr46276578qhu.62.1453260608482; Tue, 19 Jan 2016 19:30:08 -0800 (PST) Received: from [192.168.1.130] (ool-4353abbc.dyn.optonline.net. [67.83.171.188]) by smtp.gmail.com with ESMTPSA id p75sm13598435qki.29.2016.01.19.19.30.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Jan 2016 19:30:07 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 19 Jan 2016 22:30:02 -0500 (EST) To: Patrick Palka cc: Jason Merrill , GCC Patches Subject: Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early) In-Reply-To: Message-ID: References: <1450979869-5575-1-git-send-email-patrick@parcs.ath.cx> <567CACC0.5010705@redhat.com> <569D060F.6070708@redhat.com> <569D5360.1010805@redhat.com> User-Agent: Alpine 2.20.9 (DEB 106 2015-09-22) MIME-Version: 1.0 On Tue, 19 Jan 2016, Patrick Palka wrote: > On Mon, Jan 18, 2016 at 4:04 PM, Jason Merrill wrote: >> On 01/18/2016 02:12 PM, Patrick Palka wrote: >>> >>> On Mon, Jan 18, 2016 at 10:34 AM, Jason Merrill wrote: >>>> >>>> On 12/25/2015 12:37 PM, Patrick Palka wrote: >>>>> >>>>> >>>>> That alone would not be sufficient because more_specialized_fn() >>>>> doesn't call maybe_adjust_types_for_deduction() beforehand, yet we >>>>> have to do the decaying there too (and on both types, not just one of >>>>> them). >>>>> >>>>> And maybe_adjust_types_for_deduction() seems to operate on the >>>>> presumption that one type is the parameter type and one is the >>>>> argument type. But in more_specialized_fn() and in get_bindings() we >>>>> are really working with two parameter types and have to decay them >>>>> both. So sometimes we have to decay one of the types that are >>>>> eventually going to get passed to unify(), and other times we want to >>>>> decay both types that are going to get passed to unify(). >>>>> maybe_adjust_types_for_deduction() seems to only expect the former >>>>> case. >>>>> >>>>> Finally, maybe_adjust_types_for_deduction() is not called when >>>>> unifying a nested function declarator (because it is guarded by the >>>>> subr flag in unify_one_argument), so doing it there we would also >>>>> regress in the following test case: >>>> >>>> >>>> >>>> Ah, that makes sense. >>>> >>>> How about keeping the un-decayed type in the PARM_DECLs, so that we get >>>> the >>>> substitution failure in instantiate_template, but having the decayed type >>>> in >>>> the TYPE_ARG_TYPES, probably by doing the decay in grokparms, so it's >>>> already decayed when we're doing unification? >>> >>> >>> I just tried this, and it works well! With this approach, all but one >>> of the test cases pass. The failing test case is unify17.C: >>> >>> -- 8< -- >>> >>> void foo (int *); >>> >>> template >>> void bar (void (T[5])); // { dg-error "array of 'void'" } >>> >>> void >>> baz (void) >>> { >>> bar (0); // { dg-error "no matching function" } >>> } >>> >>> -- 8< -- >>> >>> Here, we don't get a substitution failure because we don't have a >>> corresponding FUNCTION_DECL for the nested function specifier, only a >>> FUNCTION_TYPE. So there is no PARM_DECL to recurse into during >>> substitution, that retains the un-decayed argument type "T[5]" of the >>> nested function specifier. >> >> >> Then your original patch is OK. Thanks for indulging me. > > I have committed the original patch, but I just noticed that it has > the unintended effect of changing whether certain template > declarations are considered duplicates or not. For instance, in the > following test case we no longer consider the three declarations of > "foo" to be duplicates and we instead register three overloads for > foo, so that when we go to use foo later on we now get an ambiguity > error: > > $ cat hmmmmm.cc > template > int foo (T [4]); > > template > int foo (T [3]); > > template > int foo (T *); > > int x = foo (0); > $ g++ hmmmmm.cc > hmmmmm.cc:10:20: error: call of overloaded ‘foo(int)’ is ambiguous > int x = foo (0); > ^ > hmmmmm.cc:2:5: note: candidate: int foo(T [4]) [with T = int] > int foo (T [4]); > ^~~ > hmmmmm.cc:5:5: note: candidate: int foo(T [3]) [with T = int] > int foo (T [3]); > ^~~ > hmmmmm.cc:8:5: note: candidate: int foo(T*) [with T = int] > int foo (T *); > ^~~ > > > Before the patch, this test case would have compiled cleanly because > each subsequent declaration of foo would have been considered a > duplicate of the first one (since we would have decayed the array > parameter types early on). > > I am not sure whether the previous or current behavior is correct, or > if maybe the first two decls ought to be considered duplicates and the > last one not. What do you think? > A much more significant side effect of this patch is that it changes the mangling of function templates that have dependent array parameters. For the following test case: template int foo (T [I]); int x = foo (0); We now output _Z3fooIiLi5EEiAT0__T_ (aka "int foo(int [5])") instead of _Z3fooIiLi5EEiPT_ (aka "int foo(int*)") because when mangling a specialized FUNCTION_DECL the mangler uses the undecayed TYPE_ARG_TYPES (TREE_TYPE (...)) of the corresponding TEMPLATE_DECL instead of using the decayed TYPE_ARG_TYPES (TREE_TYPE (...)) of that FUNCTION_DECL. In light of this, I am inclined to instead use your suggested approach (to decay the parameter types in grokparms, which become the FUNCTION_TYPE's TYPE_ARG_TYPES, while retaining the un-decayed types in the PARM_DECLs) since that approach does not exhibit this mangling issue or the above mentioned overloading issue. Here is a patch for that. -- 8< -- Subject: [PATCH] Use a different approach for avoiding to decay array parameters too early gcc/cp/ChangeLog: PR c++/11858 PR c++/24663 PR c++/24664 * decl.c (grokparms): Decay each parameter type after stripping its qualifiers. * pt.c (tsubst_decl) [FUNCTION_DECL]: Return error_mark_node if substition of the DECL_ARGUMENTS fails. (decay_dependent_array_parm_type): Delete. (type_unification_real): Remove calls to decay_dependent_array_parm_type. (more_specialized_fn): Likewise. (get_bindings): Likewise. gcc/testsuite/ChangeLog: PR c++/11858 PR c++/24663 PR c++/24664 * g++.dg/template/mangle2.C: New test. * g++.dg/template/unify17.C: XFAIL. --- gcc/cp/decl.c | 1 + gcc/cp/pt.c | 31 +++++-------------------------- gcc/testsuite/g++.dg/template/mangle2.C | 22 ++++++++++++++++++++++ gcc/testsuite/g++.dg/template/unify17.C | 4 ++-- 4 files changed, 30 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/mangle2.C diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index ceeef60..d681281 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -11702,6 +11702,7 @@ grokparms (tree parmlist, tree *parms) /* Top-level qualifiers on the parameters are ignored for function types. */ type = strip_top_quals (type); + type = type_decays_to (type); if (TREE_CODE (type) == METHOD_TYPE) { diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index fdef601..a7084a1 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -11740,6 +11740,10 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) complain, t); DECL_RESULT (r) = NULL_TREE; + for (tree parm = DECL_ARGUMENTS (r); parm; parm = DECL_CHAIN (parm)) + if (TREE_TYPE (parm) == error_mark_node) + RETURN (error_mark_node); + TREE_STATIC (r) = 0; TREE_PUBLIC (r) = TREE_PUBLIC (t); DECL_EXTERNAL (r) = 1; @@ -17742,23 +17746,6 @@ fn_type_unification (tree fn, return r; } -/* TYPE is the type of a function parameter. If TYPE is a (dependent) - ARRAY_TYPE, return the corresponding POINTER_TYPE to which it decays. - Otherwise return TYPE. (We shouldn't see non-dependent ARRAY_TYPE - parameters because they get decayed as soon as they are declared.) */ - -static tree -decay_dependent_array_parm_type (tree type) -{ - if (TREE_CODE (type) == ARRAY_TYPE) - { - gcc_assert (uses_template_parms (type)); - return type_decays_to (type); - } - - return type; -} - /* Adjust types before performing type deduction, as described in [temp.deduct.call] and [temp.deduct.conv]. The rules in these two sections are symmetric. PARM is the type of a function parameter @@ -18197,8 +18184,6 @@ type_unification_real (tree tparms, arg = args[ia]; ++ia; - parm = decay_dependent_array_parm_type (parm); - if (unify_one_argument (tparms, targs, parm, arg, subr, strict, explain_p)) return 1; @@ -20232,9 +20217,6 @@ more_specialized_fn (tree pat1, tree pat2, int len) len = 0; } - arg1 = decay_dependent_array_parm_type (arg1); - arg2 = decay_dependent_array_parm_type (arg2); - if (TREE_CODE (arg1) == REFERENCE_TYPE) { ref1 = TYPE_REF_IS_RVALUE (arg1) + 1; @@ -20520,10 +20502,7 @@ get_bindings (tree fn, tree decl, tree explicit_args, bool check_rettype) for (arg = decl_arg_types, ix = 0; arg != NULL_TREE && arg != void_list_node; arg = TREE_CHAIN (arg), ++ix) - { - args[ix] = TREE_VALUE (arg); - args[ix] = decay_dependent_array_parm_type (args[ix]); - } + args[ix] = TREE_VALUE (arg); if (fn_type_unification (fn, explicit_args, targs, args, ix, diff --git a/gcc/testsuite/g++.dg/template/mangle2.C b/gcc/testsuite/g++.dg/template/mangle2.C new file mode 100644 index 0000000..f9ea040 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/mangle2.C @@ -0,0 +1,22 @@ +template +int foo (T [I]); + +template +int bar (T [7]); + +template +int baz (int [I]); + +extern int x, y, z; + +int +main () +{ + x = foo (0); + y = bar (0); + z = baz<9> (0); +} + +// { dg-final { scan-assembler "_Z3fooIiLi5EEiPT_" } } +// { dg-final { scan-assembler "_Z3barIcEiPT_" } } +// { dg-final { scan-assembler "_Z3bazILi9EEiPi" } } diff --git a/gcc/testsuite/g++.dg/template/unify17.C b/gcc/testsuite/g++.dg/template/unify17.C index 2da8553..3ae7699 100644 --- a/gcc/testsuite/g++.dg/template/unify17.C +++ b/gcc/testsuite/g++.dg/template/unify17.C @@ -1,11 +1,11 @@ void foo (int *); template -void bar (void (T[5])); // { dg-error "array of 'void'" } +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail *-*-* } } void baz (void) { bar (foo); // { dg-bogus "" } - bar (0); // { dg-error "no matching function" } + bar (0); // { dg-error "no matching function" "" { xfail *-*-* } } }