From patchwork Tue Sep 24 17:50:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 277557 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 5A5792C00C9 for ; Wed, 25 Sep 2013 03:51:07 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=MzsFw3phAyko1vEXe 87zuJCNCHCSAfQ7t1T6UOC4Bbda6jZG79pXRpUyyZI1rqxUiN3SUcTbtgEpad5UM 64TwpyIsm+HAdv/wSbCWzynBTegdoNFf+pO0FdhZg6pyKa7+2Hr1uRVq9NI9/lU/ BNEGhm8sQYa/IlzgMS3Exe4oAU= 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:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=GOii/QtirxF8qqBu++rZDKx CaXI=; b=VVtgRdXE6NBYWJKcmWatvJs4lMAbegWSCOeJOv89ol+1EM5LFSqGwuW ktgL44/YvCUaPDGNDNEU1kG0CFGT+QfaJukpacr4qd4xfZk3xy7s1rz8fRrZYXnk cBjYQT9KmMwmvhbfRFQ0h7DruHLcUhZYQPsOzKyXVvDV9arKMgkA= Received: (qmail 2349 invoked by alias); 24 Sep 2013 17:50:26 -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 2315 invoked by uid 89); 24 Sep 2013 17:50:26 -0000 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; Tue, 24 Sep 2013 17:50:26 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com 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.14.4/8.14.4) with ESMTP id r8OHoOo1022238 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 24 Sep 2013 13:50:24 -0400 Received: from redhat.com (ovpn-116-100.ams2.redhat.com [10.36.116.100]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8OHoI1c011303 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 24 Sep 2013 13:50:22 -0400 Date: Tue, 24 Sep 2013 19:50:18 +0200 From: Marek Polacek To: Paolo Carlini Cc: GCC Patches , jason@gcc.gnu.org Subject: Re: [PATCH] Set expr loc safely (PR c++/58516) Message-ID: <20130924175017.GF12296@redhat.com> References: <20130924163558.GE12296@redhat.com> <5241C539.1020303@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5241C539.1020303@oracle.com> User-Agent: Mutt/1.5.20 (2009-06-14) On Tue, Sep 24, 2013 at 12:00:41PM -0500, Paolo Carlini wrote: > Hi, > > On 9/24/13 11:35 AM, Marek Polacek wrote: > >I admit I haven't spent much time on this, but it seems we should just > >check whether we can set the expr location before actually setting it... > > > >Regtested/bootstrapped on x86_64-linux, ok for trunk? > Two minor nits (assuming the general idea makes sense, first blush it does). > > First, solicited by the ChangeLog entry too and given that non-null > expression trees can always have locations, I wonder if we shouldn't > check EXPR_P instead, it seems more direct, unless we really fear > that body could be NULL_TREE. Well, seems build_must_not_throw_expr won't return NULL_TREE, and given: #define CAN_HAVE_LOCATION_P(NODE) ((NODE) && EXPR_P (NODE)) I think EXPR_P should do ;). > I would also phrase in a negative form > the comment before the check, like "This may not be true when..." Fixed. > Second, I'm pretty sure about this one, I think we are standardizing > on c++11 for testcases, by now c++0x is essentially legacy. Sure, adjusted. Thanks. Regtested on x86_64-linux. 2013-09-24 Marek Polacek PR c++/58516 cp/ * semantics.c (finish_transaction_stmt): Check for EXPR_P before setting the expr location. testsuite/ * g++.dg/tm/pr58516.C: New test. Marek --- gcc/cp/semantics.c.mp 2013-09-24 17:24:59.907548551 +0200 +++ gcc/cp/semantics.c 2013-09-24 19:14:18.590639066 +0200 @@ -5199,7 +5199,9 @@ finish_transaction_stmt (tree stmt, tree { tree body = build_must_not_throw_expr (TRANSACTION_EXPR_BODY (stmt), noex); - SET_EXPR_LOCATION (body, EXPR_LOCATION (TRANSACTION_EXPR_BODY (stmt))); + /* This may not be true when the STATEMENT_LIST is empty. */ + if (EXPR_P (body)) + SET_EXPR_LOCATION (body, EXPR_LOCATION (TRANSACTION_EXPR_BODY (stmt))); TREE_SIDE_EFFECTS (body) = 1; TRANSACTION_EXPR_BODY (stmt) = body; } --- gcc/testsuite/g++.dg/tm/pr58516.C.mp 2013-09-24 17:27:08.859055542 +0200 +++ gcc/testsuite/g++.dg/tm/pr58516.C 2013-09-24 19:13:36.958487511 +0200 @@ -0,0 +1,7 @@ +// { dg-do compile } +// { dg-options "-std=c++11 -fgnu-tm" } + +void foo() +{ + __transaction_atomic noexcept(false) {} +}