This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++0x] avoid extra tentative parse in range-based for loops
- From: Jason Merrill <jason at redhat dot com>
- To: Rodrigo Rivas <rodrigorivascosta at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Magnus Fromreide <magfr at lysator dot liu dot se>
- Date: Tue, 23 Nov 2010 15:24:02 -0500
- Subject: Re: [C++0x] avoid extra tentative parse in range-based for loops
- References: <AANLkTinOGbM81pMy4yE5-mPXJ3aHG8iRmnHtXF+qoMud@mail.gmail.com> <4CE14363.2020001@redhat.com> <AANLkTimTvP7X=Z8FEL-ogbkEWt9UpttiDBs+wXy768+Y@mail.gmail.com>
On 11/15/2010 05:34 PM, Rodrigo Rivas wrote:
Should I write a testcase for this? I find difficult to write one that
looks minimally realistic:
for (struct S {} *s : { (S*)NULL, (S*)NULL, (S*)NULL }) ;
or:
for (struct S {} s : { S(), S(), S() }) ;
or even:
for (struct S { S(int xx):x(xx){} int x; } s : { 1, 2, 3 }) ;
are correct but mostly useless.
Testcases don't need to be useful. :)
By the way, now that we are changing things, I find somewhat
surprising that the name of the declarator is mandatory. Let's say
that I want to repeat a identical code twice:
for (int x : {0, 0})
{ /*...*/ }
But this issues: warning: variable 'x' set but not used
[-Wunused-but-set-variable].
Shouldn't this be allowed, then?
for (int : {0, 0})
{ /*...*/ }
Sure, that seems reasonable, but I think too late now.
+ if (kind == FOR_LOOP_EXPRESSION)
+ cp_parser_expression_statement (parser, NULL_TREE);
Why did this move here? It seems to fit better in
cp_parser_for_init_statement, where it was before. And then you don't
need to distinguish between _DECLARATION and _EXPRESSION.
+ If JUST_ONE_DECLARATOR is not NULL, the pointed tree will be set to the
+ parsed declaration iff it is a single declarator. In either way, the
+ trailing `;', if present, will not be checked nor consumed. */
This should also say that it's set to NULL_TREE or error_mark_node if 0
or >1 declarators are seen. And document that it's used by
cp_parser_for_init_statement.
- else if (token->type == CPP_SEMICOLON)
+ else if (token->type == CPP_SEMICOLON
+ || (just_one_declarator
+ && *just_one_declarator != error_mark_node))
But here you're checking the trailing ; if more than one declarator is
seen, which seems to contradict the comment. Can't we just say it's OK
if just_one_declarator is set, and let cp_parser_for_init_statement
complain if we see something unsuitable?
+ if (just_one_declarator && *just_one_declarator != error_mark_node)
+ {
+ is_initialized = SD_INITIALIZED;
+ }
...
+ if (is_initialized && initialization_kind != CPP_EOF)
...
+ if (!friend_p && decl && decl != error_mark_node
+ && (!is_initialized || initialization_kind != CPP_EOF))
It seems that the purpose of this code is to pass SD_INITIALIZED into
start_decl; I think it would be clearer to check just_one_declarator
just before the call to start_decl, with a comment about why.
Jason