From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124874 invoked by alias); 2 Mar 2018 17:14:37 -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 123118 invoked by uid 89); 2 Mar 2018 17:14:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy= X-HELO: mail-io0-f182.google.com Received: from mail-io0-f182.google.com (HELO mail-io0-f182.google.com) (209.85.223.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Mar 2018 17:14:35 +0000 Received: by mail-io0-f182.google.com with SMTP id l12so11301715ioc.10 for ; Fri, 02 Mar 2018 09:14:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=iKsYgfyzPopq3LOcuNvdtbNc35fR9D4Qkqmm0IBsJFM=; b=UrWEzBo9Hm8mDQ1Fbqru6i9qnKTvB/UOeBwRANeq23B+LPXHvQTiXfrMop+yZI1y4W Qm3IYaAA1qbGy8Q0WsUoHkVO8JtgUWO41QsIW7QaEriAw+XgX2GN8bMMYKr79rE4EPzL XryR+MuQ+m18M/6sY0zLovtbZqG0uD7e+oPdlHztoJiOUjNZqC6qfdL1ArUEKZOY99jv W2EDacn6jYLzAORRCUg2cwnw/sgWzjKBcpDwxHC1vbX1NVHMutUbDRYFo5pNJLg+tYpl /7BRU3Phe/jL+iZ4HIho5O3uzG75Mt4GchRWw2WUc2sEp98Ps6S+GTF6jktwmbXXaYx+ 8mFA== X-Gm-Message-State: AElRT7Fc41hDz2m7RO07Ls3rQUXfodnJ9eTBEPRVqssQhQ7TNYo3FDcI NjcU2hb43Eu3S1d0foX/oFyAuddx6oXDNbdF/9keGg== X-Google-Smtp-Source: AG47ELvfQok0Q4JxryKaCgjry1FCo0WK4bgWoNjJucNao4U/6u+LLrSJgwfKll4xBfkLFYnN51rZaEyzRBtCn/NaNXk= X-Received: by 10.107.183.76 with SMTP id h73mr7470914iof.201.1520010873551; Fri, 02 Mar 2018 09:14:33 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.17.200 with HTTP; Fri, 2 Mar 2018 09:14:13 -0800 (PST) In-Reply-To: References: From: Jason Merrill Date: Fri, 02 Mar 2018 17:14:00 -0000 Message-ID: Subject: Re: [C++] [PR84231] overload on cond_expr in template To: Alexandre Oliva Cc: gcc-patches List Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-03/txt/msg00141.txt.bz2 On Fri, Mar 2, 2018 at 2:57 AM, Alexandre Oliva wrote: > On Feb 28, 2018, Jason Merrill wrote: > >> On Wed, Feb 28, 2018 at 12:24 AM, Alexandre Oliva wrote: >>> + if (processing_template_decl) >>> + result_type = cp_build_reference_type (result_type, !is_lvalue); > >> If !is_lvalue, we don't want a reference type at all, as the result is >> a prvalue. is_lvalue should probably rename to is_glvalue. > > I ended up moving the above to the path in which we deal with lvalues > and xvalues. I still renamed the variable as suggested, though I no > longer use it. > >> The second argument to cp_build_reference_type should be xvalue_p (arg2). > > I thought of adding a comment as to why testing just arg2 was correct, > but then moving the code kind of made it obvious, didn't it? > >>> + /* Except for type-dependent exprs, a REFERENCE_TYPE will >>> + indicate whether its result is an lvalue or so. > >> "or not" ? > > I meant "or so" as in "or other kinds of reference values". > >>> + if (processing_template_decl >>> + && !type_dependent_expression_p (CONST_CAST_TREE (ref))) >>> + return clk_none; > >> I think we want to return clk_class for a COND_EXPR of class type. > > or array, like the default case, I suppose. > >> Can we actually get here for a type-dependent expression? I'd think >> we'd never get as far as asking whether a type-dependent expression is >> an lvalue, since in general we can't answer that question. > > We shouldn't, indeed. And AFAICT we don't; I've added an assert to make > sure. > > [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. Rename > is_lvalue to is_glvalue. > * 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 | 11 +++++++---- > gcc/cp/parser.c | 4 +++- > gcc/cp/tree.c | 15 +++++++++++++++ > gcc/cp/typeck.c | 4 ---- > gcc/testsuite/g++.dg/pr84231.C | 29 +++++++++++++++++++++++++++++ > 5 files changed, 54 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/pr84231.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 11fe28292fb1..6707aa2d3f02 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -4782,7 +4782,7 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3, > tree arg3_type; > tree result = NULL_TREE; > tree result_type = NULL_TREE; > - bool is_lvalue = true; > + bool is_glvalue = true; > struct z_candidate *candidates = 0; > struct z_candidate *cand; > void *p; > @@ -5037,7 +5037,7 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3, > return error_mark_node; > } > > - is_lvalue = false; > + is_glvalue = false; > goto valid_operands; > } > /* [expr.cond] > @@ -5155,6 +5155,9 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3, > && same_type_p (arg2_type, arg3_type)) > { > result_type = arg2_type; > + if (processing_template_decl) > + result_type = cp_build_reference_type (result_type, xvalue_p (arg2)); Let's add a comment along the lines of /* Let lvalue_kind know this was a glvalue. */ OK with that change. Jason