diff mbox

15/n: trans-mem: compiler C++ changes

Message ID 4EB43B9B.6060606@redhat.com
State New
Headers show

Commit Message

Richard Henderson Nov. 4, 2011, 7:23 p.m. UTC 
On 11/04/2011 11:31 AM, Jason Merrill wrote:
> Speaking of which, that function's comment doesn't match the code:
> 
>> +   transaction-expression:
>> +     __transaction_atomic txn-exception-spec[opt] compound-statement
>> +     __transaction_relaxed txn-exception-spec[opt] compound-statement

Fixed.

>> +   function-definition:
>> +     decl-specifier-seq [opt] declarator function-atomic-block
> 
> function-atomic-block should be function-transaction-block, right?

Yes.  The spec has changed a couple of times, and the comments didn't
make all the same changes.


r~
* cp/parser.c (cp_parser_init_declarator): Fix production comments.
	(cp_parser_transaction_expression): Don't parse txn-attributes here.

Comments

Jason Merrill Nov. 4, 2011, 7:26 p.m. UTC | #1 
On 11/04/2011 03:23 PM, Richard Henderson wrote:
> 	(cp_parser_transaction_expression): Don't parse txn-attributes here.

There's also the issue that the grammar uses compound-statement, whereas 
the code parses a parenthesized expression.

Jason
Richard Henderson Nov. 4, 2011, 8:14 p.m. UTC | #2 
On 11/04/2011 12:26 PM, Jason Merrill wrote:
> On 11/04/2011 03:23 PM, Richard Henderson wrote:
>>     (cp_parser_transaction_expression): Don't parse txn-attributes here.
> 
> There's also the issue that the grammar uses compound-statement, whereas the code parses a parenthesized expression.

There are two different items, actually:

  __transaction_* { } 		// transaction statement, can be aborted
  __transaction_* (expr)	// transaction expression, cannot, and returns the value

but we really do require the ( in the second case, so the grammar shouldn't
require any lookahead.


r~
Jason Merrill Nov. 4, 2011, 8:56 p.m. UTC | #3 
On 11/04/2011 04:14 PM, Richard Henderson wrote:
> On 11/04/2011 12:26 PM, Jason Merrill wrote:
>> On 11/04/2011 03:23 PM, Richard Henderson wrote:
>>>      (cp_parser_transaction_expression): Don't parse txn-attributes here.
>>
>> There's also the issue that the grammar uses compound-statement, whereas the code parses a parenthesized expression.
>
> There are two different items, actually:
>
>    __transaction_* { } 		// transaction statement, can be aborted
>    __transaction_* (expr)	// transaction expression, cannot, and returns the value
>
> but we really do require the ( in the second case, so the grammar shouldn't
> require any lookahead.

Right, my point is that the comment before 
cp_parser_transaction_expression refers to compound_statement, which 
seems wrong.

Jason
Jason Merrill Nov. 4, 2011, 8:59 p.m. UTC | #4 
On 11/04/2011 04:56 PM, Jason Merrill wrote:
> On 11/04/2011 04:14 PM, Richard Henderson wrote:
>> On 11/04/2011 12:26 PM, Jason Merrill wrote:
>>> On 11/04/2011 03:23 PM, Richard Henderson wrote:
>>>> (cp_parser_transaction_expression): Don't parse txn-attributes here.
>>>
>>> There's also the issue that the grammar uses compound-statement,
>>> whereas the code parses a parenthesized expression.
>>
>> There are two different items, actually:
>>
>> __transaction_* { } // transaction statement, can be aborted
>> __transaction_* (expr) // transaction expression, cannot, and returns
>> the value
>>
>> but we really do require the ( in the second case, so the grammar
>> shouldn't
>> require any lookahead.
>
> Right, my point is that the comment before
> cp_parser_transaction_expression refers to compound_statement, which
> seems wrong.

...because the compound-statement variant is handled in 
cp_parser_transaction.

Jason
Richard Henderson Nov. 4, 2011, 9:56 p.m. UTC | #5 
On 11/04/2011 01:59 PM, Jason Merrill wrote:
> Right, my point is that the comment before
> cp_parser_transaction_expression refers to compound_statement, which
> seems wrong.
> 
> ...because the compound-statement variant is handled in cp_parser_transaction.

Oh, duh.  I was so focused on the attribute portion that I didn't notice
the compound-statement mistake.  Will fix.


r~
diff mbox

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d7cdb9c..ed70178 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15116,7 +15116,7 @@  cp_parser_asm_definition (cp_parser* parser)
    TM Extension:
 
    function-definition:
-     decl-specifier-seq [opt] declarator function-atomic-block
+     decl-specifier-seq [opt] declarator function-transaction-block
 
    The DECL_SPECIFIERS apply to this declarator.  Returns a
    representation of the entity declared.  If MEMBER_P is TRUE, then
@@ -26652,7 +26652,7 @@  cp_parser_transaction_expression (cp_parser *parser, enum rid keyword)
   unsigned char old_in = parser->in_transaction;
   unsigned char this_in = 1;
   cp_token *token;
-  tree ret, attrs;
+  tree ret;
 
   gcc_assert (keyword == RID_TRANSACTION_ATOMIC
       || keyword == RID_TRANSACTION_RELAXED);
@@ -26663,12 +26663,6 @@  cp_parser_transaction_expression (cp_parser *parser, enum rid keyword)
 
   if (keyword == RID_TRANSACTION_RELAXED)
     this_in |= TM_STMT_ATTR_RELAXED;
-  else
-    {
-      attrs = cp_parser_txn_attribute_opt (parser);
-      if (attrs)
-        this_in |= parse_tm_stmt_attr (attrs, 0);
-    }
 
   parser->in_transaction = this_in;
   if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN))