Re: [C++0x] Range-based for statements and ADL

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

Hi again.

Here it is my first try at this. I have changed the list to
gcc-patches, I don't know if cross post would be correct.
Please, note that this patch is not finished: the new test cases are
still missing, and expect format mistakes, misspellings and the
like...

A few comments:

1. I'm not sure about what should happen if the begin/end found in
class scope are not ordinary functions. My guess is that if it is a
function (static or non-static) it is called normally, and if it is a
member variable it is searched for an operator()(). If it is a type it
should fail. That is what I tried to implement (I think it works).

2. The function cp_parser_perform_range_for_lookup() does the same job
twice, but interleaved, so it's not easy to avoid typing everything
once and again. Maybe it should be split in several pieces.
Suggestions welcome.

3. In the function cp_parser_perform_range_for_lookup() I made quite a
shameful, but clever, abuse of the local variables and arguments. I've
seen similar patterns in the code, but if you think it is too much, I
can rewrite it.

4. The approach to error diagnostics can be done in so many ways that
I cannot make my mind. I think that the attached patch does a
reasonable job, but again...

5. Hidden in the new wording, there are a restriction about arrays of
incomplete type or unknown size. That restriction should have been
obvious before (easy to say now ;-), and it is quite unrelated to the
other changes. But I don't know if it is worth its own patch (3 lines
an a future test).

Awaiting for comments.

--
Rodrigo

[-- Attachment #2: range-for.txt --]
[-- Type: text/plain, Size: 6880 bytes --]

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3a60d0f..4e13a04 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1607,6 +1607,8 @@ static tree cp_parser_c_for
   (cp_parser *, tree, tree);
 static tree cp_parser_range_for
   (cp_parser *, tree, tree, tree);
+static void cp_parser_perform_range_for_lookup
+   (tree, tree *, tree *);
 static tree cp_parser_jump_statement
   (cp_parser *);
 static void cp_parser_declaration_statement
@@ -8555,14 +8557,20 @@ cp_parser_range_for (cp_parser *parser, tree scope, tree init, tree range_decl)
       }
 
    If RANGE_EXPR is an array:
-       BEGIN_EXPR = __range
-       END_EXPR = __range + ARRAY_SIZE(__range)
+	BEGIN_EXPR = __range
+	END_EXPR = __range + ARRAY_SIZE(__range)
+   Else if RANGE_EXPR has a member 'begin' or 'end':
+	BEGIN_EXPR = __range.begin()
+	END_EXPR = __range.end()
    Else:
 	BEGIN_EXPR = begin(__range)
 	END_EXPR = end(__range);
 
-   When calling begin()/end() we must use argument dependent
-   lookup, but always considering 'std' as an associated namespace.  */
+   If __range has a member 'begin' but not 'end', or vice versa, we must
+   still use the second alternative (it will surely fail, however).
+   When calling begin()/end() in the third alternative we must use
+   argument dependent lookup, but always considering 'std' as an associated
+   namespace.  */
 
 tree
 cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
@@ -8598,44 +8606,41 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
 
       if (TREE_CODE (TREE_TYPE (range_temp)) == ARRAY_TYPE)
 	{
-	  /* If RANGE_TEMP is an array we will use pointer arithmetic */
-	  iter_type = build_pointer_type (TREE_TYPE (TREE_TYPE (range_temp)));
-	  begin_expr = range_temp;
-	  end_expr
-	      = build_binary_op (input_location, PLUS_EXPR,
-				 range_temp,
-				 array_type_nelts_top (TREE_TYPE (range_temp)),
-				 0);
+	  if (!COMPLETE_TYPE_P (TREE_TYPE (range_temp)))
+	    {
+	      error ("range-based-for expression must be of a complete type");
+	      begin_expr = end_expr = iter_type = error_mark_node;
+	    }
+	  else
+	    {
+	      /* If RANGE_TEMP is an array we will use pointer arithmetic */
+	      iter_type = build_pointer_type (TREE_TYPE (TREE_TYPE (range_temp)));
+	      begin_expr = range_temp;
+	      end_expr
+		  = build_binary_op (input_location, PLUS_EXPR,
+				     range_temp,
+				     array_type_nelts_top (TREE_TYPE (range_temp)),
+				     0);
+	    }
 	}
       else
 	{
 	  /* If it is not an array, we must call begin(__range)/end__range() */
-	  VEC(tree,gc) *vec;
-
-	  begin_expr = get_identifier ("begin");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  begin_expr = perform_koenig_lookup (begin_expr, vec,
-					      /*include_std=*/true);
-	  begin_expr = finish_call_expr (begin_expr, &vec, false, true,
-					 tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  end_expr = get_identifier ("end");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  end_expr = perform_koenig_lookup (end_expr, vec,
-					    /*include_std=*/true);
-	  end_expr = finish_call_expr (end_expr, &vec, false, true,
-				       tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  /* The unqualified type of the __begin and __end temporaries should
-	   * be the same as required by the multiple auto declaration */
-	  iter_type = cv_unqualified (TREE_TYPE (begin_expr));
-	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (end_expr))))
-	    error ("inconsistent begin/end types in range-based for: %qT and %qT",
-		   TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
+	  cp_parser_perform_range_for_lookup (range_temp, &begin_expr, &end_expr);
+
+	  if (begin_expr == error_mark_node || end_expr == error_mark_node)
+	    /* If one of the expressions is an error do no more checks.  */
+	    begin_expr = end_expr = iter_type = error_mark_node;
+	  else
+	    {
+	      iter_type = cv_unqualified (TREE_TYPE (begin_expr));
+	      /* The unqualified type of the __begin and __end temporaries should
+	       * be the same as required by the multiple auto declaration */
+	      if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (end_expr))))
+		error ("inconsistent begin/end types in range-based for: "
+		       "%qT and %qT",
+		       TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
+	    }
 	}
     }
 
@@ -8680,6 +8685,84 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
   return statement;
 }
 
+static void
+cp_parser_perform_range_for_lookup (tree range, tree *begin, tree *end)
+{
+  tree id_begin, id_end;
+
+  id_begin = get_identifier ("begin");
+  *begin = build_qualified_name (/*type=*/NULL_TREE,
+				     TREE_TYPE (range),
+				     id_begin,
+				     /*template_p=*/false);
+  *begin = finish_class_member_access_expr(range, *begin,
+					       false, tf_none);
+
+  id_end = get_identifier ("end");
+  *end = build_qualified_name (/*type=*/NULL_TREE,
+			       TREE_TYPE (range),
+			       id_end,
+			       /*template_p=*/false);
+  *end = finish_class_member_access_expr(range, *end,
+					 false, tf_none);
+
+  VEC(tree,gc) *vec;
+  vec = make_tree_vector ();
+
+  if (*begin != error_mark_node || *end != error_mark_node)
+    {
+      if (*begin != error_mark_node)
+	{
+	  tree instance = TREE_OPERAND (*begin, 0);
+	  tree fn = TREE_OPERAND (*begin, 1);
+	  if (BASELINK_P (fn))
+	    *begin = build_new_method_call (instance, fn,
+					    NULL, NULL, LOOKUP_NORMAL,
+					    NULL, tf_warning_or_error);
+	  else
+	    *begin = finish_call_expr (*begin, &vec,
+				       /*disallow_virtual=*/false,
+				       /*koenig_p=*/false,
+				       tf_warning_or_error);
+	}
+      else
+	error ("range-based-for expression has an %<end%> member "
+	       "but not a %<begin%>");
+      if (*end != error_mark_node)
+	{
+	  tree instance = TREE_OPERAND (*end, 0);
+	  tree fn = TREE_OPERAND (*end, 1);
+	  if (BASELINK_P (fn))
+	    *end = build_new_method_call (instance, fn,
+					  NULL, NULL, LOOKUP_NORMAL,
+					  NULL, tf_warning_or_error);
+	  else
+	    *end = finish_call_expr (*end, &vec,
+				     /*disallow_virtual=*/false,
+				     /*koenig_p=*/false,
+				     tf_warning_or_error);
+	}
+      else
+	error ("range-based-for expression has a %<begin%> member "
+	       "but not an %<end%>");
+    }
+  else
+    {
+      VEC_safe_push (tree, gc, vec, range);
+
+      *begin = perform_koenig_lookup (id_begin, vec,
+					  /*include_std=*/true);
+      *begin = finish_call_expr (*begin, &vec, false, true,
+				 tf_warning_or_error);
+      *end = perform_koenig_lookup (id_end, vec,
+					/*include_std=*/true);
+      *end = finish_call_expr (*end, &vec, false, true,
+			       tf_warning_or_error);
+
+    }
+  release_tree_vector (vec);
+}
+
 
 /* Parse an iteration-statement.
 

Re: [C++0x] Range-based for statements and ADL

[Jonathan Wakely]

On 29 March 2011 01:49, Rodrigo Rivas wrote:
> Hi again.
>
> Here it is my first try at this. I have changed the list to
> gcc-patches, I don't know if cross post would be correct.
> Please, note that this patch is not finished: the new test cases are
> still missing, and expect format mistakes, misspellings and the
> like...
>
> A few comments:
>
> 1. I'm not sure about what should happen if the begin/end found in
> class scope are not ordinary functions. My guess is that if it is a
> function (static or non-static) it is called normally, and if it is a
> member variable it is searched for an operator()(). If it is a type it
> should fail. That is what I tried to implement (I think it works).

The wording was carefully written so that if name lookup finds any
"r.begin" then it will try "r.begin()" so yes, it should be able to
call data members of callable type.

> 2. The function cp_parser_perform_range_for_lookup() does the same job
> twice, but interleaved, so it's not easy to avoid typing everything
> once and again. Maybe it should be split in several pieces.
> Suggestions welcome.
>
> 3. In the function cp_parser_perform_range_for_lookup() I made quite a
> shameful, but clever, abuse of the local variables and arguments. I've
> seen similar patterns in the code, but if you think it is too much, I
> can rewrite it.
>
> 4. The approach to error diagnostics can be done in so many ways that
> I cannot make my mind. I think that the attached patch does a
> reasonable job, but again...

Should we consistently refer to %<for%> so the keyword is highlighted?

> 5. Hidden in the new wording, there are a restriction about arrays of
> incomplete type or unknown size. That restriction should have been
> obvious before (easy to say now ;-), and it is quite unrelated to the
> other changes. But I don't know if it is worth its own patch (3 lines
> an a future test).
>
> Awaiting for comments.
>
> --
> Rodrigo
>

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

On Tue, Mar 29, 2011 at 10:12 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> Should we consistently refer to %<for%> so the keyword is highlighted?
Now that you say... I've not been quite consistent. We could say
"range-based %<for%>", with only a dash between 'range' and 'based'.

Re: [C++0x] Range-based for statements and ADL

[Gabriel Dos Reis]

On Tue, Mar 29, 2011 at 4:38 AM, Rodrigo Rivas
<rodrigorivascosta@gmail.com> wrote:
> On Tue, Mar 29, 2011 at 10:12 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>> Should we consistently refer to %<for%> so the keyword is highlighted?
> Now that you say... I've not been quite consistent. We could say
> "range-based %<for%>", with only a dash between 'range' and 'based'.
>

or "new-style for loop"?

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

On Tue, Mar 29, 2011 at 5:46 PM, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
> or "new-style for loop"?
Well, that is what they are called in Java, isn't it? And the syntax
is just the same, so it would make kind of sense.
But in the C++0x draft and the GCC docs it is almost always called
"range-based for loops, so I think it is preferred.


Talking of this... I tried the simplest wrong range-based for, with my
latest patch:

int main()
{
for (int x : 0)
    ;
}

And that is what I got:

test.cpp: In function ‘int main()’:
test.cpp:3:18: error: ‘begin’ was not declared in this scope
test.cpp:3:18: error: ‘end’ was not declared in this scope

That's ok. But now, if I add #include <vector> I get:

test.cpp: In function ‘int main()’:
test.cpp:4:18: error: no matching function for call to ‘begin(int&)’
test.cpp:4:18: note: candidates are:
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/range_access.h:87:5:
note: template<class _Tp, unsigned int _Nm> _Tp* std::begin(_Tp
(&)[_Nm])
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/range_access.h:58:63:
note: template<class _Container> decltype (__cont->begin())
std::begin(const _Container&)
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/range_access.h:48:57:
note: template<class _Container> decltype (__cont->begin())
std::begin(_Container&)
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/initializer_list:86:38:
note: template<class _Tp> constexpr const _Tp*
std::begin(std::initializer_list<_Tp>)
test.cpp:4:18: error: no matching function for call to ‘end(int&)’
test.cpp:4:18: note: candidates are:
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/range_access.h:97:5:
note: template<class _Tp, unsigned int _Nm> _Tp* std::end(_Tp
(&)[_Nm])
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/range_access.h:78:59:
note: template<class _Container> decltype (__cont->end())
std::end(const _Container&)
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/range_access.h:68:53:
note: template<class _Container> decltype (__cont->end())
std::end(_Container&)
/home/rodrigo/local/gcc/lib/gcc/i686-pc-linux-gnu/4.7.0/../../../../include/c++/4.7.0/initializer_list:96:36:
note: template<class _Tp> constexpr const _Tp*
std::end(std::initializer_list<_Tp>)

Although the error messages are technically correct, I find them too
verbose and probably misleading to the novice: "'begin' not declared?
I'm not using 'begin', I'm using 'for').
Note also that the error could be corrected adding member functions
begin/end (if the range were of class type), but that is not suggested
in the error message.

Maybe a simpler error is better. such as:
"expression is not valid as range in a range-based %<for%> loop
because it lacks begin/end members or begin(int&) and end(int&)
overloads." (I leave the wording to someone more skilled with
English).

Regards.
--
Rodrigo

Re: [C++0x] Range-based for statements and ADL

[Jonathan Wakely]

On 29 March 2011 17:41, Rodrigo Rivas wrote:
>
> Maybe a simpler error is better. such as:
> "expression is not valid as range in a range-based %<for%> loop
> because it lacks begin/end members or begin(int&) and end(int&)
> overloads." (I leave the wording to someone more skilled with
> English).

If you can prevent the full lookup error and print something different
I think users would be very grateful indeed.

How about "No suitable %<begin%> and %<end%> functions found for range
expression of type %qT in %<for%> statement" ?

That

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

On Tue, Mar 29, 2011 at 8:22 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> How about "No suitable %<begin%> and %<end%> functions found for range
> expression of type %qT in %<for%> statement" ?

Nice.
But the problem here is that there are a lot of different error conditions:
1. begin member but not end member.
2. end member but not begin member.
3. begin and end member but one or both are not callable/accesible/whatever.
3. no begin/end members, a begin global function but not end.
4. no begin/end members, an end global function but not begin.
5. no begin/end members, begin and end functions but one or both not
callable/wrong overload/etc.
6. no begin/end members, no begin/end global functions. Since the
standard headers have the std::begin/std::end templates, this will not
happen when you include (most) STL headers.

What is the best message for each? Does it make sense to have up to 6
different messages? Too many?
I just can't tell.
--
Rodrigo

Re: [C++0x] Range-based for statements and ADL

[Jonathan Wakely]

On 29 March 2011 21:33, Rodrigo Rivas wrote:
> On Tue, Mar 29, 2011 at 8:22 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>> How about "No suitable %<begin%> and %<end%> functions found for range
>> expression of type %qT in %<for%> statement" ?
>
> Nice.
> But the problem here is that there are a lot of different error conditions:
> 1. begin member but not end member.
> 2. end member but not begin member.

I think these cases should say something like "invalid lookup results,
member 'begin' and non-member 'end'" (or vice versa) as we had in last
week's patch.

Or maybe it would be better to say "lookup for 'for' found member
'begin' but no member 'end'"

> 3. begin and end member but one or both are not callable/accesible/whatever.

I would definitely want to get an "access denied", "deleted function"
etc. error for that case.

> 3. no begin/end members, a begin global function but not end.
> 4. no begin/end members, an end global function but not begin.

Maybe something similar to the text I suggested above, but only
mentioning the lookup that failed, "no suitable 'begin' found for
range expression ..."

> 5. no begin/end members, begin and end functions but one or both not
> callable/wrong overload/etc.

In some cases I would probably want to know the actual error, not just
"cannot use range-based for"

If a begin() is found but can't be called, showing the "candidates
are..." list might be helpful to see the problem e.g. if
my::begin(my::type&) can't be called for const my::type.

> 6. no begin/end members, no begin/end global functions. Since the
> standard headers have the std::begin/std::end templates, this will not
> happen when you include (most) STL headers.

Maybe this is the only case where the text I suggested is useful, I
think a special "can't use range-based for" makes sense in this case,
instead of "begin not declared in this scope"

> What is the best message for each? Does it make sense to have up to 6
> different messages? Too many?

It would be nice for users, but it's certainly more work, and
potentially more places for bugs to creep in.

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

On Tue, Mar 29, 2011 at 11:13 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
Thank you for your suggestions!
IMO, error cases 3 (hey, two 3s!), 4 and 6 are not so likely, as
including any STL container header will make a begin and an end
functions declared, though maybe not usabe. In these cases the most
probable error case would be 5.

> It would be nice for users, but it's certainly more work, and
> potentially more places for bugs to creep in.
Yeah, you're right. I think I'll leave that for a future patch...

Rodrigo

Re: [C++0x] Range-based for statements and ADL

[Jason Merrill]

On 03/28/2011 08:28 PM, Rodrigo Rivas wrote:
> A few comments:
> 1. I'm not sure about what should happen if the begin/end found in class
> scope are not ordinary functions.

Whatever range.begin() would mean if written explicitly.

> My guess is that if it is a function
> (static or non-static) it is called normally, and if it is a member
> variable it is searched for an operator()(). If it is a type it should
> fail.

Yes, because we can't use . syntax to name type members.

> +	      error ("range-based-for expression must be of a complete type");

I like your "range-based %<for%>" suggestion.  I'd also say "must have 
complete type".

>   static void
>   cp_parser_perform_range_for_lookup (tree range, tree *begin, tree *end)

This function needs a comment.

> +  *end = finish_class_member_access_expr(range, *end,
> +                                        false, tf_none);
> +
> +  VEC(tree,gc) *vec;
> +  vec = make_tree_vector ();

The declaration of vec needs to move to the top of the function.

> +  id_begin = get_identifier ("begin");
> +  *begin = build_qualified_name (/*type=*/NULL_TREE,
> +                                    TREE_TYPE (range),
> +                                    id_begin,
> +                                    /*template_p=*/false);
> +  *begin = finish_class_member_access_expr(range, *begin,
> +                                              false, tf_none);
> +
> +  id_end = get_identifier ("end");
> +  *end = build_qualified_name (/*type=*/NULL_TREE,
> +                              TREE_TYPE (range),
> +                              id_end,
> +                              /*template_p=*/false);
> +  *end = finish_class_member_access_expr(range, *end,
> +                                        false, tf_none);

Don't call build_qualified_name here; the standard doesn't say 
range.T::begin(), just range.begin().

Also, we can't just call finish_class_member_access_expr here because it 
returns error_mark_node for any error condition, so we can't tell the 
difference between a lookup that didn't find anything (in which case we 
want to fall back to ADL) and an access violation (in which case we want 
to give an error).

We need to do the lookup directly first, and then do 
finish_class_member_access_expr after we've decided to use the members.

Jason

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

On Thu, Mar 31, 2011 at 7:22 PM, Jason Merrill <jason@redhat.com> wrote:
> On 03/28/2011 08:28 PM, Rodrigo Rivas wrote:
>>
>> A few comments:
>> 1. I'm not sure about what should happen if the begin/end found in class
>> scope are not ordinary functions.
>
> Whatever range.begin() would mean if written explicitly.
>
>> My guess is that if it is a function
>> (static or non-static) it is called normally, and if it is a member
>> variable it is searched for an operator()(). If it is a type it should
>> fail.
>
> Yes, because we can't use . syntax to name type members.

Yeah, actually what I meant is whether:

struct S { typedef int begin, end; };
//...
for (auto x : S()) ;

should fall back to ADL or else fail at once. My guess is that is
should fail, but curiously enough my patch does ADL...

>> +  id_begin = get_identifier ("begin");
>> +  *begin = build_qualified_name (/*type=*/NULL_TREE,
>> +                                    TREE_TYPE (range),
>> +                                    id_begin,
>> +                                    /*template_p=*/false);
>> +  *begin = finish_class_member_access_expr(range, *begin,
>> +                                              false, tf_none);
>
> Don't call build_qualified_name here; the standard doesn't say
> range.T::begin(), just range.begin().
That's curious, because I tested with virtual functions with a class
hierarchy, and it worked as expected. My understanding is that the
range.T::begin() syntax would require a call to
adjust_result_of_qualified_name_lookup.
But again, I've just tried removing the call to build_qualified_name
and it works just the same. It looks to me that
finish_class_member_access_expr is a super-smart functions and "just
works" with many kinds of input.

> Also, we can't just call finish_class_member_access_expr here because it
> returns error_mark_node for any error condition, so we can't tell the
> difference between a lookup that didn't find anything (in which case we want
> to fall back to ADL) and an access violation (in which case we want to give
> an error).

I'll dare say that you are wrong with this one, if only because I've
just debugged it. If the member begin is private, for instance,
finish_class_member_access_expr returns ok, and then the error is
emitted from build_new_method_call.

> We need to do the lookup directly first, and then do
> finish_class_member_access_expr after we've decided to use the members.
But maybe you are right here anyway, because I think that there may be
are errors from finish_class_member_access_expr that we want to
diagnose right away and errors that we want to silence, and the
tsubst_flags_t does not do this.

I'm preparing another patch with your suggestions and a few testcases.

Regards.
--
Rodrigo

Re: [C++0x] Range-based for statements and ADL

[Jonathan Wakely]

On 31 March 2011 21:22, Rodrigo Rivas wrote:
> On Thu, Mar 31, 2011 at 7:22 PM, Jason Merrill <jason@redhat.com> wrote:
>> On 03/28/2011 08:28 PM, Rodrigo Rivas wrote:
>>>
>>> A few comments:
>>> 1. I'm not sure about what should happen if the begin/end found in class
>>> scope are not ordinary functions.
>>
>> Whatever range.begin() would mean if written explicitly.
>>
>>> My guess is that if it is a function
>>> (static or non-static) it is called normally, and if it is a member
>>> variable it is searched for an operator()(). If it is a type it should
>>> fail.
>>
>> Yes, because we can't use . syntax to name type members.
>
> Yeah, actually what I meant is whether:
>
> struct S { typedef int begin, end; };
> //...
> for (auto x : S()) ;
>
> should fall back to ADL or else fail at once. My guess is that is
> should fail, but curiously enough my patch does ADL...

Yes, it should fail because the rules for class member access say that
if E2 is a nested type then the expression E1.E2 is ill-formed (which
is just the formal wording for what Jason said :-)

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

Hi!

It took some time but I finally re-wrote the patch and added a few testsuites.
A few comments as usual:

I've moved the array stuff into cp_parser_perform_range_for_lookup. I
think that it belongs there: it's just a special lookup case .

I finally agreed with Jason about calling lookup_member to check for
the existence of begin/end before. Although maybe it could be done
without this call, I don't like to rely on the subtle behavior of
finish_class_member_access_expr when dealing with errors. This way is
much easier to understand (and more obj-c++ friendly, probably).

I've split cp_parser_perform_range_for_lookup into another function,
cp_parser_range_for_member_function, that builds of the tree that does
the member function call.

I've changed a few error messages to "range-based %<for%>" or
"range-based %<for%> loop", and the related test cases.

Testsuites range-for2.C and range-for3.C worked, but just by chance.
I've corrected them.

About the new test cases, now the possibilities of compiler errors are
greatly increased. I've tried to cover most of them, but there are
surely many missing.

Regards.
--
Rodrigo

[-- Attachment #2: range-for.txt --]
[-- Type: text/plain, Size: 15085 bytes --]

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9ed3a1f..b040a53 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1607,6 +1607,10 @@ static tree cp_parser_c_for
   (cp_parser *, tree, tree);
 static tree cp_parser_range_for
   (cp_parser *, tree, tree, tree);
+static tree cp_parser_perform_range_for_lookup
+  (tree, tree *, tree *);
+static tree cp_parser_range_for_member_function
+  (tree, tree);
 static tree cp_parser_jump_statement
   (cp_parser *);
 static void cp_parser_declaration_statement
@@ -8555,14 +8559,20 @@ cp_parser_range_for (cp_parser *parser, tree scope, tree init, tree range_decl)
       }
 
    If RANGE_EXPR is an array:
-       BEGIN_EXPR = __range
-       END_EXPR = __range + ARRAY_SIZE(__range)
+	BEGIN_EXPR = __range
+	END_EXPR = __range + ARRAY_SIZE(__range)
+   Else if RANGE_EXPR has a member 'begin' or 'end':
+	BEGIN_EXPR = __range.begin()
+	END_EXPR = __range.end()
    Else:
 	BEGIN_EXPR = begin(__range)
 	END_EXPR = end(__range);
 
-   When calling begin()/end() we must use argument dependent
-   lookup, but always considering 'std' as an associated namespace.  */
+   If __range has a member 'begin' but not 'end', or vice versa, we must
+   still use the second alternative (it will surely fail, however).
+   When calling begin()/end() in the third alternative we must use
+   argument dependent lookup, but always considering 'std' as an associated
+   namespace.  */
 
 tree
 cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
@@ -8595,48 +8605,8 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
 		      LOOKUP_ONLYCONVERTING);
 
       range_temp = convert_from_reference (range_temp);
-
-      if (TREE_CODE (TREE_TYPE (range_temp)) == ARRAY_TYPE)
-	{
-	  /* If RANGE_TEMP is an array we will use pointer arithmetic */
-	  iter_type = build_pointer_type (TREE_TYPE (TREE_TYPE (range_temp)));
-	  begin_expr = range_temp;
-	  end_expr
-	      = build_binary_op (input_location, PLUS_EXPR,
-				 range_temp,
-				 array_type_nelts_top (TREE_TYPE (range_temp)),
-				 0);
-	}
-      else
-	{
-	  /* If it is not an array, we must call begin(__range)/end__range() */
-	  VEC(tree,gc) *vec;
-
-	  begin_expr = get_identifier ("begin");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  begin_expr = perform_koenig_lookup (begin_expr, vec,
-					      /*include_std=*/true);
-	  begin_expr = finish_call_expr (begin_expr, &vec, false, true,
-					 tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  end_expr = get_identifier ("end");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  end_expr = perform_koenig_lookup (end_expr, vec,
-					    /*include_std=*/true);
-	  end_expr = finish_call_expr (end_expr, &vec, false, true,
-				       tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  /* The unqualified type of the __begin and __end temporaries should
-	   * be the same as required by the multiple auto declaration */
-	  iter_type = cv_unqualified (TREE_TYPE (begin_expr));
-	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (end_expr))))
-	    error ("inconsistent begin/end types in range-based for: %qT and %qT",
-		   TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
-	}
+      iter_type = cp_parser_perform_range_for_lookup (range_temp,
+						      &begin_expr, &end_expr);
     }
 
   /* The new for initialization statement */
@@ -8680,6 +8650,143 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
   return statement;
 }
 
+/* Solves BEGIN_EXPR and END_EXPR as described in cp_convert_range_for.
+   We need to solve both at the same time because the method used
+   depends on the existence of members begin or end.
+   Returns the type deduced for the iterator expression.  */
+
+static tree
+cp_parser_perform_range_for_lookup (tree range, tree *begin, tree *end)
+{
+  if (TREE_CODE (TREE_TYPE (range)) == ARRAY_TYPE)
+    {
+      if (!COMPLETE_TYPE_P (TREE_TYPE (range)))
+	{
+	  error ("range-based %<for%> expression must have complete type");
+	  *begin = *end = error_mark_node;
+	  return error_mark_node;
+	}
+      else
+	{
+	  /* If RANGE is an array we will use pointer arithmetic */
+	  *begin = range;
+	  *end = build_binary_op (input_location, PLUS_EXPR,
+				  range,
+				  array_type_nelts_top (TREE_TYPE (range)),
+				  0);
+	  return build_pointer_type (TREE_TYPE (TREE_TYPE (range)));
+	}
+    }
+  else
+    {
+      /* If it is not an array, we must do a bit of magic */
+      tree id_begin, id_end;
+      tree member_begin, member_end;
+
+      *begin = *end = error_mark_node;
+
+      id_begin = get_identifier ("begin");
+      id_end = get_identifier ("end");
+      member_begin = lookup_member (TREE_TYPE (range), id_begin,
+				    /*protect=*/2, /*want_type=*/false);
+      member_end = lookup_member (TREE_TYPE (range), id_end,
+				  /*protect=*/2, /*want_type=*/false);
+
+      if (member_begin != NULL_TREE || member_end != NULL_TREE)
+	/* Use the member functions */
+	{
+	  if (member_begin != NULL_TREE)
+	    *begin = cp_parser_range_for_member_function (range, id_begin);
+	  else
+	    error ("range-based %<for%> expression has an %<end%> member "
+		   "but not a %<begin%>");
+
+	  if (member_end != NULL_TREE)
+	    *end = cp_parser_range_for_member_function (range, id_end);
+	  else
+	    error ("range-based %<for%> expression has a %<begin%> member "
+		   "but not an %<end%>");
+	}
+      else
+	/* Use global functions with ADL */
+	{
+	  VEC(tree,gc) *vec;
+	  vec = make_tree_vector ();
+
+	  VEC_safe_push (tree, gc, vec, range);
+
+	  member_begin = perform_koenig_lookup (id_begin, vec,
+					  /*include_std=*/true);
+	  *begin = finish_call_expr (member_begin, &vec, false, true,
+				     tf_warning_or_error);
+	  member_end = perform_koenig_lookup (id_end, vec,
+					/*include_std=*/true);
+	  *end = finish_call_expr (member_end, &vec, false, true,
+				   tf_warning_or_error);
+
+	  release_tree_vector (vec);
+	}
+
+      /* Last common checks */
+      if (*begin == error_mark_node || *end == error_mark_node)
+	/* If one of the expressions is an error do no more checks.  */
+	{
+	  *begin = *end = error_mark_node;
+	  return error_mark_node;
+	}
+      else
+	{
+	  tree iter_type = cv_unqualified (TREE_TYPE (*begin));
+	  /* The unqualified type of the __begin and __end temporaries should
+	   * be the same as required by the multiple auto declaration */
+	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (*end))))
+	    error ("inconsistent begin/end types in range-based %<for%> "
+		   "statement: %qT and %qT",
+		   TREE_TYPE (*begin), TREE_TYPE (*end));
+	  return iter_type;
+	}
+    }
+}
+
+/* Helper function for cp_parser_perform_range_for_lookup.
+   Builds a tree for RANGE.IDENTIFIER().  */
+
+static tree
+cp_parser_range_for_member_function (tree range, tree identifier)
+{
+  tree member, instance, fn;
+
+  member = finish_class_member_access_expr (range, identifier,
+					    false, tf_warning_or_error);
+  if (member == error_mark_node)
+    return error_mark_node;
+
+  if (TREE_CODE (member) == COMPONENT_REF)
+    {
+      instance = TREE_OPERAND (member, 0);
+      fn = TREE_OPERAND (member, 1);
+    }
+  else
+    fn = NULL_TREE;
+
+  if (fn && BASELINK_P (fn))
+    return build_new_method_call (instance, fn,
+				    NULL, NULL, LOOKUP_NORMAL,
+				    NULL, tf_warning_or_error);
+  else
+    {
+      tree res;
+      VEC(tree,gc) *vec;
+
+      vec = make_tree_vector ();
+      res = finish_call_expr (member, &vec,
+			       /*disallow_virtual=*/false,
+			       /*koenig_p=*/false,
+			       tf_warning_or_error);
+      release_tree_vector (vec);
+      return res;
+    }
+}
 
 /* Parse an iteration-statement.
 
@@ -8828,7 +8935,7 @@ cp_parser_for_init_statement (cp_parser* parser, tree *decl)
 	  if (cxx_dialect < cxx0x)
 	    {
 	      error_at (cp_lexer_peek_token (parser->lexer)->location,
-			"range-based-for loops are not allowed "
+			"range-based %<for%> loops are not allowed "
 			"in C++98 mode");
 	      *decl = error_mark_node;
 	    }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for11.C b/gcc/testsuite/g++.dg/cpp0x/range-for11.C
new file mode 100644
index 0000000..d02519a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for11.C
@@ -0,0 +1,40 @@
+// Test for range-based for loop
+// Test the loop with a custom iterator
+// with begin/end as member functions
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+struct iterator
+{
+    int x;
+    explicit iterator(int v) :x(v) {}
+    iterator &operator ++() { ++x; return *this; }
+    int operator *() { return x; }
+    bool operator != (const iterator &o) { return x != o.x; }
+};
+
+namespace foo
+{
+    struct container
+    {
+        int min, max;
+        container(int a, int b) :min(a), max(b) {}
+
+        iterator begin()
+        {
+            return iterator(min);
+        }
+        iterator end()
+        {
+            return iterator(max + 1);
+        }
+    };
+}
+
+int main()
+{
+    foo::container c(1,4);
+    for (int it : c)
+        ;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for12.C b/gcc/testsuite/g++.dg/cpp0x/range-for12.C
new file mode 100644
index 0000000..9b405dc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for12.C
@@ -0,0 +1,116 @@
+// Test for range-based for loop with templates
+// and begin/end as member functions
+
+// { dg-do run }
+// { dg-options "-std=c++0x" }
+
+/* Preliminary declarations */
+namespace pre
+{
+  struct iterator
+  {
+    int x;
+    explicit iterator (int v) :x(v) {}
+    iterator &operator ++() { ++x; return *this; }
+    int operator *() { return x; }
+    bool operator != (const iterator &o) { return x != o.x; }
+  };
+
+  struct container
+  {
+    int min, max;
+    container(int a, int b) :min(a), max(b) {}
+    iterator begin() const
+    {
+        return iterator(min);
+    }
+    iterator end() const
+    {
+        return iterator(max);
+    }
+
+  };
+
+} //namespace pre
+
+using pre::container;
+extern "C" void abort(void);
+
+container run_me_just_once()
+{
+    static bool run = false;
+    if (run)
+        abort();
+    run = true;
+    return container(1,2);
+}
+
+/* Template with dependent expression. */
+template<typename T> int test1(const T &r)
+{
+  int t = 0;
+  for (int i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression and dependent declaration. */
+template<typename T> int test2(const container &r)
+{
+  int t = 0;
+  for (T i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression (array) and dependent declaration. */
+template<typename T> int test2(const int (&r)[4])
+{
+  int t = 0;
+  for (T i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression and auto declaration. */
+template<typename T> int test3(const container &r)
+{
+  int t = 0;
+  for (auto i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression (array) and auto declaration. */
+template<typename T> int test3(const int (&r)[4])
+{
+  int t = 0;
+  for (auto i : r)
+    t += i;
+  return t;
+}
+
+int main ()
+{
+  container c(1,5);
+  int a[4] = {5,6,7,8};
+
+  for (auto x : run_me_just_once())
+      ;
+
+  if (test1 (c) != 10)
+    abort();
+  if (test1 (a) != 26)
+    abort();
+
+  if (test2<int> (c) != 10)
+    abort();
+  if (test2<int> (a) != 26)
+    abort();
+
+  if (test3<int> (c) != 10)
+    abort();
+  if (test3<int> (a) != 26)
+    abort();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for13.C b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
new file mode 100644
index 0000000..6a0fb83
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
@@ -0,0 +1,96 @@
+// Test for errors in range-based for loops
+// with member begin/end
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+//These should not be used
+template<typename T> int *begin(T &t)
+{
+    T::fail;
+}
+template<typename T> int *end(T &t)
+{
+    T::fail;
+}
+
+struct container1
+{
+    int *begin();
+    //no end
+};
+
+struct container2
+{
+    int *end();
+    //no begin
+};
+
+struct container3
+{
+private:
+    int *begin(); // { dg-error "is private" }
+    int *end(); // { dg-error "is private" }
+};
+
+struct container4
+{
+    int *begin;
+    int *end;
+};
+
+struct container5
+{
+    typedef int *begin;
+    typedef int *end;
+};
+
+struct callable
+{
+    int *operator()();
+};
+
+struct container6
+{
+    callable begin;
+    callable end;
+};
+
+struct container7
+{
+    static callable begin;
+    static callable end;
+};
+
+struct private_callable
+{
+private:
+    int *operator()(); // { dg-error "is private" }
+};
+
+struct container8
+{
+    private_callable begin;
+    private_callable end;
+};
+
+struct container9
+{
+    typedef int *(*function)();
+
+    function begin;
+    static function end;
+};
+
+void test1()
+{
+  for (int x : container1()); // { dg-error "member but not" }
+  for (int x : container2()); // { dg-error "member but not" }
+  for (int x : container3()); // { dg-error "within this context" }
+  for (int x : container4()); // { dg-error "cannot be used as a function" }
+  for (int x : container5()); // { dg-error "invalid use of" }
+  for (int x : container6());
+  for (int x : container7());
+  for (int x : container8()); // { dg-error "within this context" }
+  for (int x : container9());
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for2.C b/gcc/testsuite/g++.dg/cpp0x/range-for2.C
index bfab376..17eb41d 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for2.C
@@ -8,7 +8,7 @@
 struct iterator
 {
     int x;
-    iterator(int v) :x(v) {}
+    explicit iterator(int v) :x(v) {}
     iterator &operator ++() { ++x; return *this; }
     int operator *() { return x; }
     bool operator != (const iterator &o) { return x != o.x; }
@@ -36,6 +36,6 @@ namespace foo
 int main()
 {
     foo::container c(1,4);
-    for (iterator it : c)
+    for (int it : c)
         ;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for3.C b/gcc/testsuite/g++.dg/cpp0x/range-for3.C
index 947f01c..85115a3 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for3.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for3.C
@@ -8,7 +8,7 @@
 struct iterator
 {
     int x;
-    iterator(int v) :x(v) {}
+    explicit iterator(int v) :x(v) {}
     iterator &operator ++() { ++x; return *this; }
     int operator *() { return x; }
     bool operator != (const iterator &o) { return x != o.x; }
@@ -36,7 +36,7 @@ namespace std
 int main()
 {
     container c(1,4);
-    for (iterator it : c)
+    for (int it : c)
     {
     }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for9.C b/gcc/testsuite/g++.dg/cpp0x/range-for9.C
index 96e9cb6..c51cbf9 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for9.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for9.C
@@ -6,6 +6,6 @@
 void test()
 {
     int a[] = {0,1,2};
-    for (int x : a)  // { dg-error "range-based-for" }
+    for (int x : a)  // { dg-error "range-based 'for'" }
         ;
 }

Re: [C++0x] Range-based for statements and ADL

[Jason Merrill]

Looks good, just a couple of tweaks:

On 04/06/2011 07:22 PM, Rodrigo Rivas wrote:
> +         error ("range-based %<for%> expression must have complete type");
> +           error ("range-based %<for%> expression has an %<end%> member "
> +                  "but not a %<begin%>");

Let's give the type of the range initializer in these error messages.

> +static tree
> +cp_parser_range_for_member_function (tree range, tree identifier)
> +{
> +  tree member, instance, fn;
> +
> +  member = finish_class_member_access_expr (range, identifier,
> +                                           false, tf_warning_or_error);
> +  if (member == error_mark_node)
> +    return error_mark_node;
> +
> +  if (TREE_CODE (member) == COMPONENT_REF)
> +    {
> +      instance = TREE_OPERAND (member, 0);
> +      fn = TREE_OPERAND (member, 1);
> +    }
> +  else
> +    fn = NULL_TREE;
> +
> +  if (fn && BASELINK_P (fn))
> +    return build_new_method_call (instance, fn,
> +                                   NULL, NULL, LOOKUP_NORMAL,
> +                                   NULL, tf_warning_or_error);
> +  else
> +    {
> +      tree res;
> +      VEC(tree,gc) *vec;
> +
> +      vec = make_tree_vector ();
> +      res = finish_call_expr (member, &vec,
> +                              /*disallow_virtual=*/false,
> +                              /*koenig_p=*/false,
> +                              tf_warning_or_error);
> +      release_tree_vector (vec);
> +      return res;
> +    }
> +}

Instead of handling this difference here, let's teach finish_call_expr 
to handle a COMPONENT_REF around a BASELINK.

Jason

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

[-- Attachment #1: Type: text/plain, Size: 2291 bytes --]

On Fri, Apr 8, 2011 at 5:45 PM, Jason Merrill <jason@redhat.com> wrote:
>> +         error ("range-based %<for%> expression must have complete
>> type");
>> +           error ("range-based %<for%> expression has an %<end%> member "
>> +                  "but not a %<begin%>");
>
> Let's give the type of the range initializer in these error messages.

Ok. I've changed the first one to:
+	  error ("range-based %<for%> expression of type %qT "
+		 "has incomplete type", TREE_TYPE (range));
Because the type of the expression must have complete type *only* if
it is an array. That could be stated in the error, but I don't think
it is necessary.

>> +static tree
>> +cp_parser_range_for_member_function (tree range, tree identifier)
>> ...
> Instead of handling this difference here, let's teach finish_call_expr to
> handle a COMPONENT_REF around a BASELINK.
Well, I just did that, but I'm not sure this is totally correct, I've
just begun to understand finish_call_expr. It works, if only because
I'm the only one using this new code path. Fell free to
revise/change/comment about it.

Also, I added a few more test cases: one with begin/end as member
functions with default arguments and member template functions with
default template arguments; other with begin/end member functions as
virtual functions.

And since we are approaching the acceptability, here it is the changelog:

gcc/cp

2011-04-11  Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>

	* parser.c (cp_convert_range_for): Split into
	cp_parser_perform_range_for_lookup.
	(cp_parser_perform_range_for_lookup): New.
	(cp_parser_range_for_member_function): New.
	(cp_parser_for_init_statement): Correct error message.
	* semantics.c (finish_call_expr): Accept COMPONENT_REF.

gcc/testsuite

2011-04-11  Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>

	* g++.dg/cpp0x/range-for2.C: Correct for declaration.
	* g++.dg/cpp0x/range-for3.C: Likewise.
	* g++.dg/cpp0x/range-for9.C: Correct error message.
	* g++.dg/cpp0x/range-for11.C: New.
	* g++.dg/cpp0x/range-for12.C: New.
	* g++.dg/cpp0x/range-for13.C: New.
	* g++.dg/cpp0x/range-for14.C: New.
	* g++.dg/cpp0x/range-for15.C: New.
	* g++.dg/cpp0x/range-for16.C: New.

Best regards.
--
Rodrigo

[-- Attachment #2: range-for.txt --]
[-- Type: text/plain, Size: 19466 bytes --]

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9ed3a1f..36b3a3c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1607,6 +1607,10 @@ static tree cp_parser_c_for
   (cp_parser *, tree, tree);
 static tree cp_parser_range_for
   (cp_parser *, tree, tree, tree);
+static tree cp_parser_perform_range_for_lookup
+  (tree, tree *, tree *);
+static tree cp_parser_range_for_member_function
+  (tree, tree);
 static tree cp_parser_jump_statement
   (cp_parser *);
 static void cp_parser_declaration_statement
@@ -8555,14 +8559,20 @@ cp_parser_range_for (cp_parser *parser, tree scope, tree init, tree range_decl)
       }
 
    If RANGE_EXPR is an array:
-       BEGIN_EXPR = __range
-       END_EXPR = __range + ARRAY_SIZE(__range)
+	BEGIN_EXPR = __range
+	END_EXPR = __range + ARRAY_SIZE(__range)
+   Else if RANGE_EXPR has a member 'begin' or 'end':
+	BEGIN_EXPR = __range.begin()
+	END_EXPR = __range.end()
    Else:
 	BEGIN_EXPR = begin(__range)
 	END_EXPR = end(__range);
 
-   When calling begin()/end() we must use argument dependent
-   lookup, but always considering 'std' as an associated namespace.  */
+   If __range has a member 'begin' but not 'end', or vice versa, we must
+   still use the second alternative (it will surely fail, however).
+   When calling begin()/end() in the third alternative we must use
+   argument dependent lookup, but always considering 'std' as an associated
+   namespace.  */
 
 tree
 cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
@@ -8595,48 +8605,8 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
 		      LOOKUP_ONLYCONVERTING);
 
       range_temp = convert_from_reference (range_temp);
-
-      if (TREE_CODE (TREE_TYPE (range_temp)) == ARRAY_TYPE)
-	{
-	  /* If RANGE_TEMP is an array we will use pointer arithmetic */
-	  iter_type = build_pointer_type (TREE_TYPE (TREE_TYPE (range_temp)));
-	  begin_expr = range_temp;
-	  end_expr
-	      = build_binary_op (input_location, PLUS_EXPR,
-				 range_temp,
-				 array_type_nelts_top (TREE_TYPE (range_temp)),
-				 0);
-	}
-      else
-	{
-	  /* If it is not an array, we must call begin(__range)/end__range() */
-	  VEC(tree,gc) *vec;
-
-	  begin_expr = get_identifier ("begin");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  begin_expr = perform_koenig_lookup (begin_expr, vec,
-					      /*include_std=*/true);
-	  begin_expr = finish_call_expr (begin_expr, &vec, false, true,
-					 tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  end_expr = get_identifier ("end");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  end_expr = perform_koenig_lookup (end_expr, vec,
-					    /*include_std=*/true);
-	  end_expr = finish_call_expr (end_expr, &vec, false, true,
-				       tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  /* The unqualified type of the __begin and __end temporaries should
-	   * be the same as required by the multiple auto declaration */
-	  iter_type = cv_unqualified (TREE_TYPE (begin_expr));
-	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (end_expr))))
-	    error ("inconsistent begin/end types in range-based for: %qT and %qT",
-		   TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
-	}
+      iter_type = cp_parser_perform_range_for_lookup (range_temp,
+						      &begin_expr, &end_expr);
     }
 
   /* The new for initialization statement */
@@ -8680,6 +8650,145 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
   return statement;
 }
 
+/* Solves BEGIN_EXPR and END_EXPR as described in cp_convert_range_for.
+   We need to solve both at the same time because the method used
+   depends on the existence of members begin or end.
+   Returns the type deduced for the iterator expression.  */
+
+static tree
+cp_parser_perform_range_for_lookup (tree range, tree *begin, tree *end)
+{
+  if (TREE_CODE (TREE_TYPE (range)) == ARRAY_TYPE)
+    {
+      if (!COMPLETE_TYPE_P (TREE_TYPE (range)))
+	{
+	  error ("range-based %<for%> expression of type %qT "
+		 "has incomplete type", TREE_TYPE (range));
+	  *begin = *end = error_mark_node;
+	  return error_mark_node;
+	}
+      else
+	{
+	  /* If RANGE is an array we will use pointer arithmetic */
+	  *begin = range;
+	  *end = build_binary_op (input_location, PLUS_EXPR,
+				  range,
+				  array_type_nelts_top (TREE_TYPE (range)),
+				  0);
+	  return build_pointer_type (TREE_TYPE (TREE_TYPE (range)));
+	}
+    }
+  else
+    {
+      /* If it is not an array, we must do a bit of magic */
+      tree id_begin, id_end;
+      tree member_begin, member_end;
+
+      *begin = *end = error_mark_node;
+
+      id_begin = get_identifier ("begin");
+      id_end = get_identifier ("end");
+      member_begin = lookup_member (TREE_TYPE (range), id_begin,
+				    /*protect=*/2, /*want_type=*/false);
+      member_end = lookup_member (TREE_TYPE (range), id_end,
+				  /*protect=*/2, /*want_type=*/false);
+
+      if (member_begin != NULL_TREE || member_end != NULL_TREE)
+	/* Use the member functions */
+	{
+	  if (member_begin != NULL_TREE)
+	    *begin = cp_parser_range_for_member_function (range, id_begin);
+	  else
+	    error ("range-based %<for%> expression of type %qT has an "
+		   "%<end%> member but not a %<begin%>", TREE_TYPE (range));
+
+	  if (member_end != NULL_TREE)
+	    *end = cp_parser_range_for_member_function (range, id_end);
+	  else
+	    error ("range-based %<for%> expression of type %qT has a "
+		   "%<begin%> member but not an %<end%>", TREE_TYPE (range));
+	}
+      else
+	/* Use global functions with ADL */
+	{
+	  VEC(tree,gc) *vec;
+	  vec = make_tree_vector ();
+
+	  VEC_safe_push (tree, gc, vec, range);
+
+	  member_begin = perform_koenig_lookup (id_begin, vec,
+					  /*include_std=*/true);
+	  *begin = finish_call_expr (member_begin, &vec, false, true,
+				     tf_warning_or_error);
+	  member_end = perform_koenig_lookup (id_end, vec,
+					/*include_std=*/true);
+	  *end = finish_call_expr (member_end, &vec, false, true,
+				   tf_warning_or_error);
+
+	  release_tree_vector (vec);
+	}
+
+      /* Last common checks */
+      if (*begin == error_mark_node || *end == error_mark_node)
+	/* If one of the expressions is an error do no more checks.  */
+	{
+	  *begin = *end = error_mark_node;
+	  return error_mark_node;
+	}
+      else
+	{
+	  tree iter_type = cv_unqualified (TREE_TYPE (*begin));
+	  /* The unqualified type of the __begin and __end temporaries should
+	   * be the same as required by the multiple auto declaration */
+	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (*end))))
+	    error ("inconsistent begin/end types in range-based %<for%> "
+		   "statement: %qT and %qT",
+		   TREE_TYPE (*begin), TREE_TYPE (*end));
+	  return iter_type;
+	}
+    }
+}
+
+/* Helper function for cp_parser_perform_range_for_lookup.
+   Builds a tree for RANGE.IDENTIFIER().  */
+
+static tree
+cp_parser_range_for_member_function (tree range, tree identifier)
+{
+  tree member, instance, fn;
+
+  member = finish_class_member_access_expr (range, identifier,
+					    false, tf_warning_or_error);
+  if (member == error_mark_node)
+    return error_mark_node;
+
+  /*
+  if (TREE_CODE (member) == COMPONENT_REF)
+    {
+      instance = TREE_OPERAND (member, 0);
+      fn = TREE_OPERAND (member, 1);
+    }
+  else
+    fn = NULL_TREE;
+
+  if (fn && BASELINK_P (fn))
+    return build_new_method_call (instance, fn,
+				    NULL, NULL, LOOKUP_NORMAL,
+				    NULL, tf_warning_or_error);
+  else*/
+    {
+      tree res;
+      VEC(tree,gc) *vec;
+
+      vec = make_tree_vector ();
+      res = finish_call_expr (member, &vec,
+			       /*disallow_virtual=*/false,
+			       /*koenig_p=*/false,
+			       tf_warning_or_error);
+      release_tree_vector (vec);
+      return res;
+    }
+}
 
 /* Parse an iteration-statement.
 
@@ -8828,7 +8937,7 @@ cp_parser_for_init_statement (cp_parser* parser, tree *decl)
 	  if (cxx_dialect < cxx0x)
 	    {
 	      error_at (cp_lexer_peek_token (parser->lexer)->location,
-			"range-based-for loops are not allowed "
+			"range-based %<for%> loops are not allowed "
 			"in C++98 mode");
 	      *decl = error_mark_node;
 	    }
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index a15740a..607b1e6 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2071,6 +2071,21 @@ finish_call_expr (tree fn, VEC(tree,gc) **args, bool disallow_virtual,
       make_args_non_dependent (*args);
     }
 
+  if (TREE_CODE (fn) == COMPONENT_REF)
+    {
+      tree member = TREE_OPERAND (fn, 1);
+      if (BASELINK_P (member))
+	{
+	  tree object = TREE_OPERAND (fn, 0);
+	  return build_new_method_call (object, member, 
+					args, NULL_TREE,
+					(disallow_virtual
+					 ? LOOKUP_NONVIRTUAL : 0),
+					/*fn_p=*/NULL,
+					complain);
+	}
+    }
+
   if (is_overloaded_fn (fn))
     fn = baselink_for_fns (fn);
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for11.C b/gcc/testsuite/g++.dg/cpp0x/range-for11.C
new file mode 100644
index 0000000..d02519a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for11.C
@@ -0,0 +1,40 @@
+// Test for range-based for loop
+// Test the loop with a custom iterator
+// with begin/end as member functions
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+struct iterator
+{
+    int x;
+    explicit iterator(int v) :x(v) {}
+    iterator &operator ++() { ++x; return *this; }
+    int operator *() { return x; }
+    bool operator != (const iterator &o) { return x != o.x; }
+};
+
+namespace foo
+{
+    struct container
+    {
+        int min, max;
+        container(int a, int b) :min(a), max(b) {}
+
+        iterator begin()
+        {
+            return iterator(min);
+        }
+        iterator end()
+        {
+            return iterator(max + 1);
+        }
+    };
+}
+
+int main()
+{
+    foo::container c(1,4);
+    for (int it : c)
+        ;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for12.C b/gcc/testsuite/g++.dg/cpp0x/range-for12.C
new file mode 100644
index 0000000..9b405dc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for12.C
@@ -0,0 +1,116 @@
+// Test for range-based for loop with templates
+// and begin/end as member functions
+
+// { dg-do run }
+// { dg-options "-std=c++0x" }
+
+/* Preliminary declarations */
+namespace pre
+{
+  struct iterator
+  {
+    int x;
+    explicit iterator (int v) :x(v) {}
+    iterator &operator ++() { ++x; return *this; }
+    int operator *() { return x; }
+    bool operator != (const iterator &o) { return x != o.x; }
+  };
+
+  struct container
+  {
+    int min, max;
+    container(int a, int b) :min(a), max(b) {}
+    iterator begin() const
+    {
+        return iterator(min);
+    }
+    iterator end() const
+    {
+        return iterator(max);
+    }
+
+  };
+
+} //namespace pre
+
+using pre::container;
+extern "C" void abort(void);
+
+container run_me_just_once()
+{
+    static bool run = false;
+    if (run)
+        abort();
+    run = true;
+    return container(1,2);
+}
+
+/* Template with dependent expression. */
+template<typename T> int test1(const T &r)
+{
+  int t = 0;
+  for (int i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression and dependent declaration. */
+template<typename T> int test2(const container &r)
+{
+  int t = 0;
+  for (T i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression (array) and dependent declaration. */
+template<typename T> int test2(const int (&r)[4])
+{
+  int t = 0;
+  for (T i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression and auto declaration. */
+template<typename T> int test3(const container &r)
+{
+  int t = 0;
+  for (auto i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression (array) and auto declaration. */
+template<typename T> int test3(const int (&r)[4])
+{
+  int t = 0;
+  for (auto i : r)
+    t += i;
+  return t;
+}
+
+int main ()
+{
+  container c(1,5);
+  int a[4] = {5,6,7,8};
+
+  for (auto x : run_me_just_once())
+      ;
+
+  if (test1 (c) != 10)
+    abort();
+  if (test1 (a) != 26)
+    abort();
+
+  if (test2<int> (c) != 10)
+    abort();
+  if (test2<int> (a) != 26)
+    abort();
+
+  if (test3<int> (c) != 10)
+    abort();
+  if (test3<int> (a) != 26)
+    abort();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for13.C b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
new file mode 100644
index 0000000..7ebf0c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
@@ -0,0 +1,103 @@
+// Test for errors in range-based for loops
+// with member begin/end
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+//These should not be used
+template<typename T> int *begin(T &t)
+{
+    T::fail;
+}
+template<typename T> int *end(T &t)
+{
+    T::fail;
+}
+
+struct container1
+{
+    int *begin();
+    //no end
+};
+
+struct container2
+{
+    int *end();
+    //no begin
+};
+
+struct container3
+{
+private:
+    int *begin(); // { dg-error "is private" }
+    int *end(); // { dg-error "is private" }
+};
+
+struct container4
+{
+    int *begin;
+    int *end;
+};
+
+struct container5
+{
+    typedef int *begin;
+    typedef int *end;
+};
+
+struct callable
+{
+    int *operator()();
+};
+
+struct container6
+{
+    callable begin;
+    callable end;
+};
+
+struct container7
+{
+    static callable begin;
+    static callable end;
+};
+
+struct container8
+{
+    static int *begin();
+    int *end();
+};
+
+struct private_callable
+{
+private:
+    int *operator()(); // { dg-error "is private" }
+};
+
+struct container9
+{
+    private_callable begin;
+    private_callable end;
+};
+
+struct container10
+{
+    typedef int *(*function)();
+
+    function begin;
+    static function end;
+};
+
+void test1()
+{
+  for (int x : container1()); // { dg-error "member but not" }
+  for (int x : container2()); // { dg-error "member but not" }
+  for (int x : container3()); // { dg-error "within this context" }
+  for (int x : container4()); // { dg-error "cannot be used as a function" }
+  for (int x : container5()); // { dg-error "invalid use of" }
+  for (int x : container6());
+  for (int x : container7());
+  for (int x : container8());
+  for (int x : container9()); // { dg-error "within this context" }
+  for (int x : container10());
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for14.C b/gcc/testsuite/g++.dg/cpp0x/range-for14.C
new file mode 100644
index 0000000..82af67d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for14.C
@@ -0,0 +1,96 @@
+// Test for other range-based for loops with
+// begin/end member functions
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+//These should not be used
+template<typename T> int *begin(T &t)
+{
+    T::fail;
+}
+template<typename T> int *end(T &t)
+{
+    T::fail;
+}
+
+//Test for defaults
+
+struct default1
+{
+    int *begin(int x); // { dg-message "note" }
+    int *end();
+};
+
+struct default2
+{
+    int *begin(int x=0);
+    int *end();
+};
+
+struct default3
+{
+    template <typename T> T *begin(); // { dg-message "note" }
+    int *end();
+};
+
+struct default4
+{
+    template <typename T=int> T *begin();
+    int *end();
+};
+
+struct default5
+{
+    template <typename T=int> T *begin(int x=0);
+    int *end();
+};
+
+void test1()
+{
+  for (int x : default1()); // { dg-error "no matching function|note" }
+  for (int x : default2());
+  for (int x : default3()); // { dg-error "no matching function|note" }
+  for (int x : default4());
+  for (int x : default5());
+}
+
+//Inheritance tests
+
+struct base_begin
+{
+    int *begin(); // { dg-error "" }
+};
+
+struct base_end
+{
+    int *end();
+};
+
+struct derived1 : base_begin, base_end
+{
+};
+
+struct base_begin2 : base_begin
+{
+};
+
+struct derived2 : base_begin, base_end, base_begin2 // { dg-warning "" }
+{
+};
+
+struct base_begin3 : virtual base_begin
+{
+};
+
+struct derived3 : virtual base_begin, base_end, base_begin3
+{
+};
+
+void test2()
+{
+  for (int x : derived1());
+  for (int x : derived2()); // { dg-error "is ambiguous" }
+  for (int x : derived3());
+}
+
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for15.C b/gcc/testsuite/g++.dg/cpp0x/range-for15.C
new file mode 100644
index 0000000..9d5feca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for15.C
@@ -0,0 +1,59 @@
+// Test for range-based for loop with templates
+// and begin/end as member (non-)virtual functions
+
+// { dg-do run }
+// { dg-options "-std=c++0x" }
+
+unsigned int g;
+
+struct A
+{
+    virtual int *begin()
+    {
+        g |= 1;
+        return 0;
+    }
+    int *end()
+    {
+        g |= 2;
+        return 0;
+    }
+};
+
+struct B : A
+{
+    virtual int *begin()
+    {
+        g |= 4;
+        return 0;
+    }
+    int *end()
+    {
+        g |= 8;
+        return 0;
+    }
+};
+
+extern "C" void abort(void);
+
+int main ()
+{
+    A a;
+    B b;
+    A &aa = b;
+
+    g = 0;
+    for (int x : a);
+    if (g != (1 | 2)) 
+        abort();
+
+    g = 0;
+    for (int x : b);
+    if (g != (4 | 8))
+        abort();
+
+    g = 0;
+    for (int x : aa);
+    if (g != (4 | 2))
+        abort();
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for16.C b/gcc/testsuite/g++.dg/cpp0x/range-for16.C
new file mode 100644
index 0000000..a1f4bbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for16.C
@@ -0,0 +1,20 @@
+// Test for range-based for loop with arrays of 
+// incomplete type or unknown size
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+extern int a[10];
+extern int b[];
+
+struct S;
+extern S c[10];
+extern S d[];
+
+void test()
+{
+    for (int n : a);
+    for (int n : b); // { dg-error "incomplete type" }
+    for (S &n : c); // { dg-error "incomplete type" }
+    for (S &n : d); // { dg-error "incomplete type" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for2.C b/gcc/testsuite/g++.dg/cpp0x/range-for2.C
index bfab376..17eb41d 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for2.C
@@ -8,7 +8,7 @@
 struct iterator
 {
     int x;
-    iterator(int v) :x(v) {}
+    explicit iterator(int v) :x(v) {}
     iterator &operator ++() { ++x; return *this; }
     int operator *() { return x; }
     bool operator != (const iterator &o) { return x != o.x; }
@@ -36,6 +36,6 @@ namespace foo
 int main()
 {
     foo::container c(1,4);
-    for (iterator it : c)
+    for (int it : c)
         ;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for3.C b/gcc/testsuite/g++.dg/cpp0x/range-for3.C
index 947f01c..85115a3 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for3.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for3.C
@@ -8,7 +8,7 @@
 struct iterator
 {
     int x;
-    iterator(int v) :x(v) {}
+    explicit iterator(int v) :x(v) {}
     iterator &operator ++() { ++x; return *this; }
     int operator *() { return x; }
     bool operator != (const iterator &o) { return x != o.x; }
@@ -36,7 +36,7 @@ namespace std
 int main()
 {
     container c(1,4);
-    for (iterator it : c)
+    for (int it : c)
     {
     }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for9.C b/gcc/testsuite/g++.dg/cpp0x/range-for9.C
index 96e9cb6..c51cbf9 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for9.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for9.C
@@ -6,6 +6,6 @@
 void test()
 {
     int a[] = {0,1,2};
-    for (int x : a)  // { dg-error "range-based-for" }
+    for (int x : a)  // { dg-error "range-based 'for'" }
         ;
 }

Re: [C++0x] Range-based for statements and ADL

[Jason Merrill]

On 04/11/2011 04:50 PM, Rodrigo Rivas wrote:
> Because the type of the expression must have complete type *only* if
> it is an array.

Actually, if it has class type, it must also have a complete type or the 
class member lookup is ill-formed.  And you can't pass an expression of 
void type to a function call.  So we can just unconditionally require a 
complete type.

> Well, I just did that, but I'm not sure this is totally correct, I've
> just begun to understand finish_call_expr. It works, if only because
> I'm the only one using this new code path. Fell free to
> revise/change/comment about it.

It looks fine.

> +                                        ? LOOKUP_NONVIRTUAL : 0),

Except this should be ? LOOKUP_NORMAL|LOOKUP_NORVIRTUAL : LOOKUP_NORMAL. 
  The other calls to build_new_method_call here should be fixed that 
way, too.

Jason

Re: [C++0x] Range-based for statements and ADL

[Rodrigo Rivas]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On Wed, Apr 13, 2011 at 3:46 PM, Jason Merrill <jason@redhat.com> wrote:
> On 04/11/2011 04:50 PM, Rodrigo Rivas wrote:
>>
>> Because the type of the expression must have complete type *only* if
>> it is an array.
>
> Actually, if it has class type, it must also have a complete type or the
> class member lookup is ill-formed.

Oh, right. It used to be allowed, but not anymore. I've moved the
incompleteness check to the top of the function.

> Except this should be ? LOOKUP_NORMAL|LOOKUP_NORVIRTUAL : LOOKUP_NORMAL.
>  The other calls to build_new_method_call here should be fixed that way,
> too.

Ok. I've corrected my call and the one I copied from.

The changelog is the same as the last one.

Regards.
--
Rodrigo

[-- Attachment #2: range-for.txt --]
[-- Type: text/plain, Size: 19888 bytes --]

commit b4a19cba29bfb76c38f9cb86888edb20260d8e64
Author: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
Date:   Thu Apr 14 16:43:04 2011 +0200

    range-for final

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 051c1c8..64256ec 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1607,6 +1607,10 @@ static tree cp_parser_c_for
   (cp_parser *, tree, tree);
 static tree cp_parser_range_for
   (cp_parser *, tree, tree, tree);
+static tree cp_parser_perform_range_for_lookup
+  (tree, tree *, tree *);
+static tree cp_parser_range_for_member_function
+  (tree, tree);
 static tree cp_parser_jump_statement
   (cp_parser *);
 static void cp_parser_declaration_statement
@@ -8556,14 +8560,20 @@ cp_parser_range_for (cp_parser *parser, tree scope, tree init, tree range_decl)
       }
 
    If RANGE_EXPR is an array:
-       BEGIN_EXPR = __range
-       END_EXPR = __range + ARRAY_SIZE(__range)
+	BEGIN_EXPR = __range
+	END_EXPR = __range + ARRAY_SIZE(__range)
+   Else if RANGE_EXPR has a member 'begin' or 'end':
+	BEGIN_EXPR = __range.begin()
+	END_EXPR = __range.end()
    Else:
 	BEGIN_EXPR = begin(__range)
 	END_EXPR = end(__range);
 
-   When calling begin()/end() we must use argument dependent
-   lookup, but always considering 'std' as an associated namespace.  */
+   If __range has a member 'begin' but not 'end', or vice versa, we must
+   still use the second alternative (it will surely fail, however).
+   When calling begin()/end() in the third alternative we must use
+   argument dependent lookup, but always considering 'std' as an associated
+   namespace.  */
 
 tree
 cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
@@ -8596,48 +8606,8 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
 		      LOOKUP_ONLYCONVERTING);
 
       range_temp = convert_from_reference (range_temp);
-
-      if (TREE_CODE (TREE_TYPE (range_temp)) == ARRAY_TYPE)
-	{
-	  /* If RANGE_TEMP is an array we will use pointer arithmetic */
-	  iter_type = build_pointer_type (TREE_TYPE (TREE_TYPE (range_temp)));
-	  begin_expr = range_temp;
-	  end_expr
-	      = build_binary_op (input_location, PLUS_EXPR,
-				 range_temp,
-				 array_type_nelts_top (TREE_TYPE (range_temp)),
-				 0);
-	}
-      else
-	{
-	  /* If it is not an array, we must call begin(__range)/end__range() */
-	  VEC(tree,gc) *vec;
-
-	  begin_expr = get_identifier ("begin");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  begin_expr = perform_koenig_lookup (begin_expr, vec,
-					      /*include_std=*/true);
-	  begin_expr = finish_call_expr (begin_expr, &vec, false, true,
-					 tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  end_expr = get_identifier ("end");
-	  vec = make_tree_vector ();
-	  VEC_safe_push (tree, gc, vec, range_temp);
-	  end_expr = perform_koenig_lookup (end_expr, vec,
-					    /*include_std=*/true);
-	  end_expr = finish_call_expr (end_expr, &vec, false, true,
-				       tf_warning_or_error);
-	  release_tree_vector (vec);
-
-	  /* The unqualified type of the __begin and __end temporaries should
-	   * be the same as required by the multiple auto declaration */
-	  iter_type = cv_unqualified (TREE_TYPE (begin_expr));
-	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (end_expr))))
-	    error ("inconsistent begin/end types in range-based for: %qT and %qT",
-		   TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
-	}
+      iter_type = cp_parser_perform_range_for_lookup (range_temp,
+						      &begin_expr, &end_expr);
     }
 
   /* The new for initialization statement */
@@ -8681,6 +8651,124 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr)
   return statement;
 }
 
+/* Solves BEGIN_EXPR and END_EXPR as described in cp_convert_range_for.
+   We need to solve both at the same time because the method used
+   depends on the existence of members begin or end.
+   Returns the type deduced for the iterator expression.  */
+
+static tree
+cp_parser_perform_range_for_lookup (tree range, tree *begin, tree *end)
+{
+  if (!COMPLETE_TYPE_P (TREE_TYPE (range)))
+    {
+      error ("range-based %<for%> expression of type %qT "
+	     "has incomplete type", TREE_TYPE (range));
+      *begin = *end = error_mark_node;
+      return error_mark_node;
+    }
+  if (TREE_CODE (TREE_TYPE (range)) == ARRAY_TYPE)
+    {
+      /* If RANGE is an array we will use pointer arithmetic */
+      *begin = range;
+      *end = build_binary_op (input_location, PLUS_EXPR,
+			      range,
+			      array_type_nelts_top (TREE_TYPE (range)),
+			      0);
+      return build_pointer_type (TREE_TYPE (TREE_TYPE (range)));
+    }
+  else
+    {
+      /* If it is not an array, we must do a bit of magic */
+      tree id_begin, id_end;
+      tree member_begin, member_end;
+
+      *begin = *end = error_mark_node;
+
+      id_begin = get_identifier ("begin");
+      id_end = get_identifier ("end");
+      member_begin = lookup_member (TREE_TYPE (range), id_begin,
+				    /*protect=*/2, /*want_type=*/false);
+      member_end = lookup_member (TREE_TYPE (range), id_end,
+				  /*protect=*/2, /*want_type=*/false);
+
+      if (member_begin != NULL_TREE || member_end != NULL_TREE)
+	/* Use the member functions */
+	{
+	  if (member_begin != NULL_TREE)
+	    *begin = cp_parser_range_for_member_function (range, id_begin);
+	  else
+	    error ("range-based %<for%> expression of type %qT has an "
+		   "%<end%> member but not a %<begin%>", TREE_TYPE (range));
+
+	  if (member_end != NULL_TREE)
+	    *end = cp_parser_range_for_member_function (range, id_end);
+	  else
+	    error ("range-based %<for%> expression of type %qT has a "
+		   "%<begin%> member but not an %<end%>", TREE_TYPE (range));
+	}
+      else
+	/* Use global functions with ADL */
+	{
+	  VEC(tree,gc) *vec;
+	  vec = make_tree_vector ();
+
+	  VEC_safe_push (tree, gc, vec, range);
+
+	  member_begin = perform_koenig_lookup (id_begin, vec,
+					  /*include_std=*/true);
+	  *begin = finish_call_expr (member_begin, &vec, false, true,
+				     tf_warning_or_error);
+	  member_end = perform_koenig_lookup (id_end, vec,
+					/*include_std=*/true);
+	  *end = finish_call_expr (member_end, &vec, false, true,
+				   tf_warning_or_error);
+
+	  release_tree_vector (vec);
+	}
+
+      /* Last common checks */
+      if (*begin == error_mark_node || *end == error_mark_node)
+	/* If one of the expressions is an error do no more checks.  */
+	{
+	  *begin = *end = error_mark_node;
+	  return error_mark_node;
+	}
+      else
+	{
+	  tree iter_type = cv_unqualified (TREE_TYPE (*begin));
+	  /* The unqualified type of the __begin and __end temporaries should
+	   * be the same as required by the multiple auto declaration */
+	  if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (*end))))
+	    error ("inconsistent begin/end types in range-based %<for%> "
+		   "statement: %qT and %qT",
+		   TREE_TYPE (*begin), TREE_TYPE (*end));
+	  return iter_type;
+	}
+    }
+}
+
+/* Helper function for cp_parser_perform_range_for_lookup.
+   Builds a tree for RANGE.IDENTIFIER().  */
+
+static tree
+cp_parser_range_for_member_function (tree range, tree identifier)
+{
+    tree member, res;
+    VEC(tree,gc) *vec;
+
+    member = finish_class_member_access_expr (range, identifier,
+            false, tf_warning_or_error);
+    if (member == error_mark_node)
+        return error_mark_node;
+
+    vec = make_tree_vector ();
+    res = finish_call_expr (member, &vec,
+            /*disallow_virtual=*/false,
+            /*koenig_p=*/false,
+            tf_warning_or_error);
+    release_tree_vector (vec);
+    return res;
+}
 
 /* Parse an iteration-statement.
 
@@ -8829,7 +8917,7 @@ cp_parser_for_init_statement (cp_parser* parser, tree *decl)
 	  if (cxx_dialect < cxx0x)
 	    {
 	      error_at (cp_lexer_peek_token (parser->lexer)->location,
-			"range-based-for loops are not allowed "
+			"range-based %<for%> loops are not allowed "
 			"in C++98 mode");
 	      *decl = error_mark_node;
 	    }
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index c763f81..81a2688 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2071,6 +2071,22 @@ finish_call_expr (tree fn, VEC(tree,gc) **args, bool disallow_virtual,
       make_args_non_dependent (*args);
     }
 
+  if (TREE_CODE (fn) == COMPONENT_REF)
+    {
+      tree member = TREE_OPERAND (fn, 1);
+      if (BASELINK_P (member))
+	{
+	  tree object = TREE_OPERAND (fn, 0);
+	  return build_new_method_call (object, member,
+					args, NULL_TREE,
+                                        (disallow_virtual
+                                         ? LOOKUP_NORMAL | LOOKUP_NONVIRTUAL :
+                                         LOOKUP_NORMAL),
+					/*fn_p=*/NULL,
+					complain);
+	}
+    }
+
   if (is_overloaded_fn (fn))
     fn = baselink_for_fns (fn);
 
@@ -2114,7 +2130,8 @@ finish_call_expr (tree fn, VEC(tree,gc) **args, bool disallow_virtual,
 
       result = build_new_method_call (object, fn, args, NULL_TREE,
 				      (disallow_virtual
-				       ? LOOKUP_NONVIRTUAL : 0),
+                                       ? LOOKUP_NORMAL | LOOKUP_NONVIRTUAL : 
+                                       LOOKUP_NORMAL),
 				      /*fn_p=*/NULL,
 				      complain);
     }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for11.C b/gcc/testsuite/g++.dg/cpp0x/range-for11.C
new file mode 100644
index 0000000..d02519a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for11.C
@@ -0,0 +1,40 @@
+// Test for range-based for loop
+// Test the loop with a custom iterator
+// with begin/end as member functions
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+struct iterator
+{
+    int x;
+    explicit iterator(int v) :x(v) {}
+    iterator &operator ++() { ++x; return *this; }
+    int operator *() { return x; }
+    bool operator != (const iterator &o) { return x != o.x; }
+};
+
+namespace foo
+{
+    struct container
+    {
+        int min, max;
+        container(int a, int b) :min(a), max(b) {}
+
+        iterator begin()
+        {
+            return iterator(min);
+        }
+        iterator end()
+        {
+            return iterator(max + 1);
+        }
+    };
+}
+
+int main()
+{
+    foo::container c(1,4);
+    for (int it : c)
+        ;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for12.C b/gcc/testsuite/g++.dg/cpp0x/range-for12.C
new file mode 100644
index 0000000..9b405dc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for12.C
@@ -0,0 +1,116 @@
+// Test for range-based for loop with templates
+// and begin/end as member functions
+
+// { dg-do run }
+// { dg-options "-std=c++0x" }
+
+/* Preliminary declarations */
+namespace pre
+{
+  struct iterator
+  {
+    int x;
+    explicit iterator (int v) :x(v) {}
+    iterator &operator ++() { ++x; return *this; }
+    int operator *() { return x; }
+    bool operator != (const iterator &o) { return x != o.x; }
+  };
+
+  struct container
+  {
+    int min, max;
+    container(int a, int b) :min(a), max(b) {}
+    iterator begin() const
+    {
+        return iterator(min);
+    }
+    iterator end() const
+    {
+        return iterator(max);
+    }
+
+  };
+
+} //namespace pre
+
+using pre::container;
+extern "C" void abort(void);
+
+container run_me_just_once()
+{
+    static bool run = false;
+    if (run)
+        abort();
+    run = true;
+    return container(1,2);
+}
+
+/* Template with dependent expression. */
+template<typename T> int test1(const T &r)
+{
+  int t = 0;
+  for (int i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression and dependent declaration. */
+template<typename T> int test2(const container &r)
+{
+  int t = 0;
+  for (T i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression (array) and dependent declaration. */
+template<typename T> int test2(const int (&r)[4])
+{
+  int t = 0;
+  for (T i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression and auto declaration. */
+template<typename T> int test3(const container &r)
+{
+  int t = 0;
+  for (auto i : r)
+    t += i;
+  return t;
+}
+
+/* Template with non-dependent expression (array) and auto declaration. */
+template<typename T> int test3(const int (&r)[4])
+{
+  int t = 0;
+  for (auto i : r)
+    t += i;
+  return t;
+}
+
+int main ()
+{
+  container c(1,5);
+  int a[4] = {5,6,7,8};
+
+  for (auto x : run_me_just_once())
+      ;
+
+  if (test1 (c) != 10)
+    abort();
+  if (test1 (a) != 26)
+    abort();
+
+  if (test2<int> (c) != 10)
+    abort();
+  if (test2<int> (a) != 26)
+    abort();
+
+  if (test3<int> (c) != 10)
+    abort();
+  if (test3<int> (a) != 26)
+    abort();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for13.C b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
new file mode 100644
index 0000000..7ebf0c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
@@ -0,0 +1,103 @@
+// Test for errors in range-based for loops
+// with member begin/end
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+//These should not be used
+template<typename T> int *begin(T &t)
+{
+    T::fail;
+}
+template<typename T> int *end(T &t)
+{
+    T::fail;
+}
+
+struct container1
+{
+    int *begin();
+    //no end
+};
+
+struct container2
+{
+    int *end();
+    //no begin
+};
+
+struct container3
+{
+private:
+    int *begin(); // { dg-error "is private" }
+    int *end(); // { dg-error "is private" }
+};
+
+struct container4
+{
+    int *begin;
+    int *end;
+};
+
+struct container5
+{
+    typedef int *begin;
+    typedef int *end;
+};
+
+struct callable
+{
+    int *operator()();
+};
+
+struct container6
+{
+    callable begin;
+    callable end;
+};
+
+struct container7
+{
+    static callable begin;
+    static callable end;
+};
+
+struct container8
+{
+    static int *begin();
+    int *end();
+};
+
+struct private_callable
+{
+private:
+    int *operator()(); // { dg-error "is private" }
+};
+
+struct container9
+{
+    private_callable begin;
+    private_callable end;
+};
+
+struct container10
+{
+    typedef int *(*function)();
+
+    function begin;
+    static function end;
+};
+
+void test1()
+{
+  for (int x : container1()); // { dg-error "member but not" }
+  for (int x : container2()); // { dg-error "member but not" }
+  for (int x : container3()); // { dg-error "within this context" }
+  for (int x : container4()); // { dg-error "cannot be used as a function" }
+  for (int x : container5()); // { dg-error "invalid use of" }
+  for (int x : container6());
+  for (int x : container7());
+  for (int x : container8());
+  for (int x : container9()); // { dg-error "within this context" }
+  for (int x : container10());
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for14.C b/gcc/testsuite/g++.dg/cpp0x/range-for14.C
new file mode 100644
index 0000000..26ae477
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for14.C
@@ -0,0 +1,95 @@
+// Test for other range-based for loops with
+// begin/end member functions
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+//These should not be used
+template<typename T> int *begin(T &t)
+{
+    T::fail;
+}
+template<typename T> int *end(T &t)
+{
+    T::fail;
+}
+
+//Test for defaults
+
+struct default1
+{
+    int *begin(int x); // { dg-message "note" }
+    int *end();
+};
+
+struct default2
+{
+    int *begin(int x=0);
+    int *end();
+};
+
+struct default3
+{
+    template <typename T> T *begin(); // { dg-message "note" }
+    int *end();
+};
+
+struct default4
+{
+    template <typename T=int> T *begin();
+    int *end();
+};
+
+struct default5
+{
+    template <typename T=int> T *begin(int x=0);
+    int *end();
+};
+
+void test1()
+{
+  for (int x : default1()); // { dg-error "no matching function|note" }
+  for (int x : default2());
+  for (int x : default3()); // { dg-error "no matching function|note" }
+  for (int x : default4());
+  for (int x : default5());
+}
+
+//Inheritance tests
+
+struct base_begin
+{
+    int *begin(); // { dg-error "" }
+};
+
+struct base_end
+{
+    int *end();
+};
+
+struct derived1 : base_begin, base_end
+{
+};
+
+struct base_begin2 : base_begin
+{
+};
+
+struct derived2 : base_begin, base_end, base_begin2 // { dg-warning "" }
+{
+};
+
+struct base_begin3 : virtual base_begin
+{
+};
+
+struct derived3 : virtual base_begin, base_end, base_begin3
+{
+};
+
+void test2()
+{
+  for (int x : derived1());
+  for (int x : derived2()); // { dg-error "is ambiguous" }
+  for (int x : derived3());
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for15.C b/gcc/testsuite/g++.dg/cpp0x/range-for15.C
new file mode 100644
index 0000000..38f3307
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for15.C
@@ -0,0 +1,59 @@
+// Test for range-based for loop with templates
+// and begin/end as member (non-)virtual functions
+
+// { dg-do run }
+// { dg-options "-std=c++0x" }
+
+unsigned int g;
+
+struct A
+{
+    virtual int *begin()
+    {
+        g |= 1;
+        return 0;
+    }
+    int *end()
+    {
+        g |= 2;
+        return 0;
+    }
+};
+
+struct B : A
+{
+    virtual int *begin()
+    {
+        g |= 4;
+        return 0;
+    }
+    int *end()
+    {
+        g |= 8;
+        return 0;
+    }
+};
+
+extern "C" void abort(void);
+
+int main ()
+{
+    A a;
+    B b;
+    A &aa = b;
+
+    g = 0;
+    for (int x : a);
+    if (g != (1 | 2))
+        abort();
+
+    g = 0;
+    for (int x : b);
+    if (g != (4 | 8))
+        abort();
+
+    g = 0;
+    for (int x : aa);
+    if (g != (4 | 2))
+        abort();
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for16.C b/gcc/testsuite/g++.dg/cpp0x/range-for16.C
new file mode 100644
index 0000000..86cc2a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for16.C
@@ -0,0 +1,21 @@
+// Test for range-based for loop with arrays of
+// incomplete type or unknown size
+
+// { dg-do compile }
+// { dg-options "-std=c++0x" }
+
+extern int a[10];
+extern int b[];
+
+struct S;
+extern S c[10];
+extern S d[];
+
+void test()
+{
+    for (int n : a);
+    for (int n : b); // { dg-error "incomplete type" }
+    for (S &n : c); // { dg-error "incomplete type" }
+    for (S &n : d); // { dg-error "incomplete type" }
+    for (int n : *c); // { dg-error "incomplete type" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for2.C b/gcc/testsuite/g++.dg/cpp0x/range-for2.C
index bfab376..17eb41d 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for2.C
@@ -8,7 +8,7 @@
 struct iterator
 {
     int x;
-    iterator(int v) :x(v) {}
+    explicit iterator(int v) :x(v) {}
     iterator &operator ++() { ++x; return *this; }
     int operator *() { return x; }
     bool operator != (const iterator &o) { return x != o.x; }
@@ -36,6 +36,6 @@ namespace foo
 int main()
 {
     foo::container c(1,4);
-    for (iterator it : c)
+    for (int it : c)
         ;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for3.C b/gcc/testsuite/g++.dg/cpp0x/range-for3.C
index 947f01c..85115a3 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for3.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for3.C
@@ -8,7 +8,7 @@
 struct iterator
 {
     int x;
-    iterator(int v) :x(v) {}
+    explicit iterator(int v) :x(v) {}
     iterator &operator ++() { ++x; return *this; }
     int operator *() { return x; }
     bool operator != (const iterator &o) { return x != o.x; }
@@ -36,7 +36,7 @@ namespace std
 int main()
 {
     container c(1,4);
-    for (iterator it : c)
+    for (int it : c)
     {
     }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for9.C b/gcc/testsuite/g++.dg/cpp0x/range-for9.C
index 96e9cb6..c51cbf9 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for9.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for9.C
@@ -6,6 +6,6 @@
 void test()
 {
     int a[] = {0,1,2};
-    for (int x : a)  // { dg-error "range-based-for" }
+    for (int x : a)  // { dg-error "range-based 'for'" }
         ;
 }

Re: [C++0x] Range-based for statements and ADL

[Jason Merrill]

Applied (with a few formatting tweaks).

Thanks,
Jason