From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 440 invoked by alias); 28 Feb 2018 05:24:33 -0000 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 Received: (qmail 131068 invoked by uid 89); 28 Feb 2018 05:24:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Feb 2018 05:24:20 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BCF8D81DED for ; Wed, 28 Feb 2018 05:24:13 +0000 (UTC) Received: from freie.home (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6A0705D9C9; Wed, 28 Feb 2018 05:24:12 +0000 (UTC) Received: from livre (livre.home [172.31.160.2]) by freie.home (8.15.2/8.15.2) with ESMTP id w1S5O16u010145; Wed, 28 Feb 2018 02:24:02 -0300 From: Alexandre Oliva To: Jason Merrill Cc: gcc-patches List Subject: Re: [C++] [PR84231] overload on cond_expr in template References: Date: Wed, 28 Feb 2018 05:24:00 -0000 In-Reply-To: (Jason Merrill's message of "Tue, 27 Feb 2018 13:21:41 -0500") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-02/txt/msg01552.txt.bz2 On Feb 27, 2018, Jason Merrill wrote: > Perhaps it would be easier to add the REFERENCE_TYPE in > build_conditional_expr_1, adjusting result_type based on > processing_template_decl and is_lvalue. It is, indeed! Here's the patch, regstrapped on i686- and x86_64-linux-gnu. The only unexpected glitch was the need for adjusting the fold expr parser to deal with an indirect_ref, lest g++.dg/cpp1x/fold6.C would fail to error at the line with the ternary operator. Ok to install? [C++] [PR84231] overload on cond_expr in template A non-type-dependent COND_EXPR within a template is reconstructed with the original operands, after one with non-dependent proxies is built to determine its result type. This is problematic because the operands of a COND_EXPR determined to be an rvalue may have been converted to denote their rvalue nature. The reconstructed one, however, won't have such conversions, so lvalue_kind may not recognize it as an rvalue, which may lead to e.g. incorrect overload resolution decisions. If we mistake such a COND_EXPR for an lvalue, overload resolution might regard a conversion sequence that binds it to a non-const reference as viable, and then select that over one that binds it to a const reference. Only after template substitution would we rebuild the COND_EXPR, realize it is an rvalue, and conclude the reference binding is ill-formed, but at that point we'd have long discarded any alternate candidates we could have used. This patch modifies the logic that determines whether a (non-type-dependent) COND_EXPR in a template is an lvalue, to rely on its type, more specifically, on the presence of a REFERENCE_TYPE wrapper. In order to avoid a type bootstrapping problem, the REFERENCE_TYPE that wraps the type of some such COND_EXPRs is introduced earlier, so that we don't have to test for lvalueness of the expression using the very code that we wish to change. for gcc/cp/ChangeLog PR c++/84231 * tree.c (lvalue_kind): Use presence/absence of REFERENCE_TYPE only while processing template decls. * typeck.c (build_x_conditional_expr): Move wrapping of reference type around type... * call.c (build_conditional_expr_1): ... here. * parser.c (cp_parser_fold_expression): Catch REFERENCE_REF_P INDIRECT_REF of COND_EXPR too. for gcc/testsuite/ChangeLog PR c++/84231 * g++.dg/pr84231.C: New. --- gcc/cp/call.c | 3 +++ gcc/cp/parser.c | 4 +++- gcc/cp/tree.c | 8 ++++++++ gcc/cp/typeck.c | 4 ---- gcc/testsuite/g++.dg/pr84231.C | 29 +++++++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr84231.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 11fe28292fb1..9d98a3d90d25 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -5348,6 +5348,9 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3, return error_mark_node; valid_operands: + if (processing_template_decl) + result_type = cp_build_reference_type (result_type, !is_lvalue); + result = build3_loc (loc, COND_EXPR, result_type, arg1, arg2, arg3); /* If the ARG2 and ARG3 are the same and don't have side-effects, diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index bcee1214c2f3..c483b6ce25ea 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -4961,7 +4961,9 @@ cp_parser_fold_expression (cp_parser *parser, tree expr1) else if (is_binary_op (TREE_CODE (expr1))) error_at (location_of (expr1), "binary expression in operand of fold-expression"); - else if (TREE_CODE (expr1) == COND_EXPR) + else if (TREE_CODE (expr1) == COND_EXPR + || (REFERENCE_REF_P (expr1) + && TREE_CODE (TREE_OPERAND (expr1, 0)) == COND_EXPR)) error_at (location_of (expr1), "conditional expression in operand of fold-expression"); diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 9b9e36a1173f..76148c876b71 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -194,6 +194,14 @@ lvalue_kind (const_tree ref) break; case COND_EXPR: + /* Except for type-dependent exprs, a REFERENCE_TYPE will + indicate whether its result is an lvalue or so. + REFERENCE_TYPEs are handled above, so if we reach this point, + we know we got an rvalue, unless we have a type-dependent + expr. */ + if (processing_template_decl + && !type_dependent_expression_p (CONST_CAST_TREE (ref))) + return clk_none; op1_lvalue_kind = lvalue_kind (TREE_OPERAND (ref, 1) ? TREE_OPERAND (ref, 1) : TREE_OPERAND (ref, 0)); diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 0e7c63dd1973..fba04c49ec2d 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -6565,10 +6565,6 @@ build_x_conditional_expr (location_t loc, tree ifexp, tree op1, tree op2, { tree min = build_min_non_dep (COND_EXPR, expr, orig_ifexp, orig_op1, orig_op2); - /* Remember that the result is an lvalue or xvalue. */ - if (glvalue_p (expr) && !glvalue_p (min)) - TREE_TYPE (min) = cp_build_reference_type (TREE_TYPE (min), - !lvalue_p (expr)); expr = convert_from_reference (min); } return expr; diff --git a/gcc/testsuite/g++.dg/pr84231.C b/gcc/testsuite/g++.dg/pr84231.C new file mode 100644 index 000000000000..de7c89a2ab69 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84231.C @@ -0,0 +1,29 @@ +// PR c++/84231 - overload resolution with cond_expr in a template + +// { dg-do compile } + +struct format { + template format& operator%(const T&) { return *this; } + template format& operator%(T&) { return *this; } +}; + +format f; + +template +void function_template(bool b) +{ + // Compiles OK with array lvalue: + f % (b ? "x" : "x"); + + // Used to fails with pointer rvalue: + f % (b ? "" : "x"); +} + +void normal_function(bool b) +{ + // Both cases compile OK in non-template function: + f % (b ? "x" : "x"); + f % (b ? "" : "x"); + + function_template(b); +} -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer