From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7993 invoked by alias); 29 Jun 2018 15:23:12 -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 7979 invoked by uid 89); 29 Jun 2018 15:23:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.8 required=5.0 tests=AWL,BAYES_50,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=save_exprs, build_this, 86184, non-lvalue X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 29 Jun 2018 15:23:10 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D8D89818F042 for ; Fri, 29 Jun 2018 15:23:08 +0000 (UTC) Received: from redhat.com (ovpn-122-98.rdu2.redhat.com [10.10.122.98]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7F6CF1CBBF; Fri, 29 Jun 2018 15:23:08 +0000 (UTC) Date: Fri, 29 Jun 2018 15:25:00 -0000 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: Re: C++ PATCH for c++/86184, rejects-valid with ?: and omitted operand Message-ID: <20180629152307.GA5927@redhat.com> References: <20180621182231.GP15879@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.0 (2018-05-17) X-SW-Source: 2018-06/txt/msg01886.txt.bz2 On Wed, Jun 27, 2018 at 05:47:25PM -0400, Jason Merrill wrote: > On Thu, Jun 21, 2018 at 2:22 PM, Marek Polacek wrote: > > The following testcase is rejected because, for this line: > > > > bool b = X() ?: false; > > > > arg2 is missing and arg1 is a TARGET_EXPR. A TARGET_EXPR is a class > > prvalue so we wrap it in a SAVE_EXPR. Later when building 'this' we > > call build_this (SAVE_EXPR >) which triggers lvalue_error: > > 5856 cp_lvalue_kind kind = lvalue_kind (arg); > > 5857 if (kind == clk_none) > > 5858 { > > 5859 if (complain & tf_error) > > 5860 lvalue_error (input_location, lv_addressof); > > because all SAVE_EXPRs are non-lvalue. > > > > Since > > a) cp_build_addr_expr_1 can process xvalues and class prvalues, > > b) TARGET_EXPRs are only evaluated once (gimplify_target_expr), > > I thought we could do the following. The testcase ensures that > > with the omitted operand we only construct X once. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2018-06-21 Marek Polacek > > > > PR c++/86184 > > * call.c (build_conditional_expr_1): Don't wrap TARGET_EXPRs > > in a SAVE_EXPR. > > > > * g++.dg/ext/cond3.C: New test. > > > > --- gcc/cp/call.c > > +++ gcc/cp/call.c > > @@ -4806,6 +4806,10 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3, > > /* Make sure that lvalues remain lvalues. See g++.oliva/ext1.C. */ > > if (lvalue_p (arg1)) > > arg2 = arg1 = cp_stabilize_reference (arg1); > > + else if (TREE_CODE (arg1) == TARGET_EXPR) > > + /* TARGET_EXPRs are only expanded once, don't wrap it in a SAVE_EXPR, > > + rendering it clk_none of clk_class. */ > > + arg2 = arg1; > > else > > arg2 = arg1 = cp_save_expr (arg1); > > How about adding the special handling in cp_save_expr rather than > here, so other callers also benefit? > > OK with that change. Thanks, this is what I'll commit (bootstrap/regtest passed on x86_64): 2018-06-29 Marek Polacek PR c++/86184 * tree.c (cp_save_expr): Don't call save_expr for TARGET_EXPRs. * g++.dg/ext/cond3.C: New test. diff --git gcc/cp/tree.c gcc/cp/tree.c index e7bd79b6276..361248d4b52 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -4918,6 +4918,11 @@ cp_save_expr (tree expr) tree codes. */ if (processing_template_decl) return expr; + + /* TARGET_EXPRs are only expanded once. */ + if (TREE_CODE (expr) == TARGET_EXPR) + return expr; + return save_expr (expr); } diff --git gcc/testsuite/g++.dg/ext/cond3.C gcc/testsuite/g++.dg/ext/cond3.C index e69de29bb2d..6390dc4270b 100644 --- gcc/testsuite/g++.dg/ext/cond3.C +++ gcc/testsuite/g++.dg/ext/cond3.C @@ -0,0 +1,20 @@ +// PR c++/86184 +// { dg-do run } +// { dg-options "" } + +int j; +struct X { + X() { j++; } + operator bool() { return true; } +}; + +/* Only create X once. */ +bool b = X() ?: false; +bool b2 = X() ? X() : false; + +int +main () +{ + if (j != 3) + __builtin_abort (); +} Marek