diff mbox

Set expr loc safely (PR c++/58516)

Message ID 20130924175017.GF12296@redhat.com
State New
Headers show

Commit Message

Marek Polacek Sept. 24, 2013, 5:50 p.m. UTC 
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  <polacek@redhat.com>

	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

Comments

Jason Merrill Sept. 24, 2013, 9:06 p.m. UTC | #1 
OK.

Jason
diff mbox

Patch

--- 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) {}
+}