This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++0x] avoid extra tentative parse in range-based for loops


On 11/25/2010 04:46 AM, Rodrigo Rivas wrote:
But this issues: warning: variable 'x' set but not used
[-Wunused-but-set-variable].

Then, what about adding somewhere? TREE_USED (range_decl) = 1; DECL_READ_P (range_decl) = 1;

Sounds good.


+  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.

That was my first option but sadly it doesn't work. The problem is that cp_parser_expression_statement must be called after begin_for_stmt.

Why? You moved the scope stuff into begin_for_scope, which is called before cp_parser_for_init_statement.


+   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.

English is such an imprecise language! (my use of it at least). What I meant is what the code does. What about something like this?:

  If JUST_ONE_DECLARATOR is not NULL, the pointed tree will be set to the
  parsed declaration iff it is a single declarator without initializer.
If this is the case, the
  trailing `;', if present, will not be checked; in either way the
ending token will not be consumed.

That still doesn't cover what happens to *just_one_declarator in the other cases.


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?

Yes, we can. Basically, the difference would be in code like:


for (int a, b, c ! x)

My code gives: expected initializer before '!'.
Yours gives: expected ';' before '!'.

I don't have a strong preference for either, but I was just following
the 'least change in behavior' rule.

I don't think either message is significantly better, so I'd go with the simpler logic.


+         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.

Actually, not so simple. The condition (is_initialized&& initialization_kind == CPP_EOF) is TRUE when (just_one_declarator&& *just_one_declarator != error_mark_node) and an unknown token is seen instead of the initializer. It has a triple effect:

  * Do not issue the error "expected initializer".
  * Pass SD_INITIALIZED to start_decl.
  * Do not parse the initializer, since it is not here.
  * Do not call cp_finish_decl, since it will be called by the upper function.

Right. Not setting is_initialized covers #3, and I think the others should be handled by a separate flag instead of special values of the initialization variables.


When a known token is seen, (*just_one_declarator) is set to
error_mark_node, so this two conditions should be equal after that:
    (just_one_declarator&&  *just_one_declarator != error_mark_node)
    (is_initialized&&  initialization_kind == CPP_EOF)

These conditions could be interchanged, but all the tests are
necessary. You may want to add a boolean local variable with this same
value to make the conditions simpler, but I don't know if it is worth
it.

I think it's definitely worth it; that's a lot clearer than the magic CPP_EOF. Also, let's rename just_one_declarator to maybe_range_for_decl to make it clearer what's going on.


Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]