[C++0x patch] implement range-based for loops
Jason Merrill
jason@redhat.com
Mon Aug 16 22:07:00 GMT 2010
Since it sounds like your assignment is coming along, I'll go ahead and
review this now. It looks good, but there are a few issues:
On 07/23/2010 08:13 AM, Rodrigo Rivas wrote:
> + tree max = TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (range_expr)));
> + end_expr = build_binary_op (UNKNOWN_LOCATION, PLUS_EXPR, range_expr,
> + build_binary_op (UNKNOWN_LOCATION, PLUS_EXPR, max, build_one_cst (integer_type_node), 0), 0);
It would be shorter to array_type_nelts_top (TREE_TYPE (range_expr)) here.
Also, a number of the lines in your patch go past 80 columns; please
reformat so they don't. And when wrapping function arguments to the
next line, they should start at the column after the '(' of the call.
> + iter_type = TREE_TYPE (begin_expr);
I believe this will get the wrong type if the begin/end functions have a
return type of const Class; the iterator should have the cv-unqualified
version.
> + if (pushed_scope) /* No scopes allowed here */
> + pop_scope(pushed_scope);
You don't need to test for NULL before calling pop_scope anymore; I
fixed it to accept a NULL argument recently.
Also, the second line here is indented too far, and you need a space
before the '('.
I'm guessing this doesn't work with templates yet? You don't have any
template testcases.
Thanks for the contribution!
Jason
More information about the Gcc-patches
mailing list