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

login
register
mail settings
Submitter Rodrigo Rivas
Date April 14, 2011, 2:42 p.m.
Message ID <BANLkTik=3aznFpbzcudzXoDMtrpmp10DGw@mail.gmail.com>
Download mbox | patch
Permalink /patch/91245/
State New
Headers show

Comments

Rodrigo Rivas - April 14, 2011, 2:42 p.m.
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
commit b4a19cba29bfb76c38f9cb86888edb20260d8e64
Author: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
Date:   Thu Apr 14 16:43:04 2011 +0200

    range-for final
Jason Merrill - April 16, 2011, 12:29 a.m.
Applied (with a few formatting tweaks).

Thanks,
Jason

Patch

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'" }
         ;
 }