[RFA] tree-iterator: C++11 range-for and tree_stmt_iterator

Message ID 20210501162901.3164931-1-jason@redhat.com
State New
Headers show
Series
  • [RFA] tree-iterator: C++11 range-for and tree_stmt_iterator
Related show

Commit Message

Dragan Mladjenovic via Gcc-patches May 1, 2021, 4:29 p.m.
Like my recent patch to add ovl_range and lkp_range in the C++ front end,
this patch adds the tsi_range adaptor for using C++11 range-based 'for' with
a STATEMENT_LIST, e.g.

  for (tree stmt : tsi_range (stmt_list)) { ... }

This also involves adding some operators to tree_stmt_iterator that are
needed for range-for iterators, and should also be useful in code that uses
the iterators directly.

The patch updates the suitable loops in the C++ front end, but does not
touch any loops elsewhere in the compiler.

gcc/ChangeLog:

	* tree-iterator.h (struct tree_stmt_iterator): Add operator++,
	operator--, operator*, operator==, and operator!=.
	(class tsi_range): New.

gcc/cp/ChangeLog:

	* constexpr.c (build_data_member_initialization): Use tsi_range.
	(build_constexpr_constructor_member_initializers): Likewise.
	(constexpr_fn_retval, cxx_eval_statement_list): Likewise.
	(potential_constant_expression_1): Likewise.
	* coroutines.cc (await_statement_expander): Likewise.
	(await_statement_walker): Likewise.
	* module.cc (trees_out::core_vals): Likewise.
	* pt.c (tsubst_expr): Likewise.
	* semantics.c (set_cleanup_locs): Likewise.
---
 gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----
 gcc/cp/constexpr.c   | 42 ++++++++++++++----------------------------
 gcc/cp/coroutines.cc | 10 ++++------
 gcc/cp/module.cc     |  5 ++---
 gcc/cp/pt.c          |  5 ++---
 gcc/cp/semantics.c   |  5 ++---
 6 files changed, 47 insertions(+), 48 deletions(-)


base-commit: 3c65858787dc52b65b26fa7018587c01510f442c
prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95
-- 
2.27.0

Comments

Dragan Mladjenovic via Gcc-patches May 13, 2021, 7:26 p.m. | #1
Ping.

On 5/1/21 12:29 PM, Jason Merrill wrote:
> Like my recent patch to add ovl_range and lkp_range in the C++ front end,

> this patch adds the tsi_range adaptor for using C++11 range-based 'for' with

> a STATEMENT_LIST, e.g.

> 

>    for (tree stmt : tsi_range (stmt_list)) { ... }

> 

> This also involves adding some operators to tree_stmt_iterator that are

> needed for range-for iterators, and should also be useful in code that uses

> the iterators directly.

> 

> The patch updates the suitable loops in the C++ front end, but does not

> touch any loops elsewhere in the compiler.

> 

> gcc/ChangeLog:

> 

> 	* tree-iterator.h (struct tree_stmt_iterator): Add operator++,

> 	operator--, operator*, operator==, and operator!=.

> 	(class tsi_range): New.

> 

> gcc/cp/ChangeLog:

> 

> 	* constexpr.c (build_data_member_initialization): Use tsi_range.

> 	(build_constexpr_constructor_member_initializers): Likewise.

> 	(constexpr_fn_retval, cxx_eval_statement_list): Likewise.

> 	(potential_constant_expression_1): Likewise.

> 	* coroutines.cc (await_statement_expander): Likewise.

> 	(await_statement_walker): Likewise.

> 	* module.cc (trees_out::core_vals): Likewise.

> 	* pt.c (tsubst_expr): Likewise.

> 	* semantics.c (set_cleanup_locs): Likewise.

> ---

>   gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

>   gcc/cp/constexpr.c   | 42 ++++++++++++++----------------------------

>   gcc/cp/coroutines.cc | 10 ++++------

>   gcc/cp/module.cc     |  5 ++---

>   gcc/cp/pt.c          |  5 ++---

>   gcc/cp/semantics.c   |  5 ++---

>   6 files changed, 47 insertions(+), 48 deletions(-)

> 

> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

> index 076fff8644c..f57456bb473 100644

> --- a/gcc/tree-iterator.h

> +++ b/gcc/tree-iterator.h

> @@ -1,4 +1,4 @@

> -/* Iterator routines for manipulating GENERIC tree statement list.

> +/* Iterator routines for manipulating GENERIC tree statement list. -*- C++ -*-

>      Copyright (C) 2003-2021 Free Software Foundation, Inc.

>      Contributed by Andrew MacLeod  <amacleod@redhat.com>

>   

> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

>   struct tree_stmt_iterator {

>     struct tree_statement_list_node *ptr;

>     tree container;

> +

> +  bool operator== (tree_stmt_iterator b) const

> +    { return b.ptr == ptr && b.container == container; }

> +  bool operator!= (tree_stmt_iterator b) const { return !(*this == b); }

> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }

> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }

> +  tree &operator* () { return ptr->stmt; }

>   };

>   

>   static inline tree_stmt_iterator

> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

>   static inline void

>   tsi_next (tree_stmt_iterator *i)

>   {

> -  i->ptr = i->ptr->next;

> +  ++(*i);

>   }

>   

>   static inline void

>   tsi_prev (tree_stmt_iterator *i)

>   {

> -  i->ptr = i->ptr->prev;

> +  --(*i);

>   }

>   

>   static inline tree *

>   tsi_stmt_ptr (tree_stmt_iterator i)

>   {

> -  return &i.ptr->stmt;

> +  return &(*i);

>   }

>   

>   static inline tree

>   tsi_stmt (tree_stmt_iterator i)

>   {

> -  return i.ptr->stmt;

> +  return *i;

>   }

>   

> +/* Make tree_stmt_iterator work as a C++ range, e.g.

> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

> +class tsi_range

> +{

> +  tree t;

> + public:

> +  tsi_range (tree t): t(t) { }

> +  tree_stmt_iterator begin() { return tsi_start (t); }

> +  tree_stmt_iterator end() { return { nullptr, t }; }

> +};

> +

>   enum tsi_iterator_update

>   {

>     TSI_NEW_STMT,		/* Only valid when single statement is added, move

> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

> index 9481a5bfd3c..260b0122f59 100644

> --- a/gcc/cp/constexpr.c

> +++ b/gcc/cp/constexpr.c

> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t, vec<constructor_elt, va_gc> **vec)

>       return false;

>     if (TREE_CODE (t) == STATEMENT_LIST)

>       {

> -      tree_stmt_iterator i;

> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> -	{

> -	  if (! build_data_member_initialization (tsi_stmt (i), vec))

> -	    return false;

> -	}

> +      for (tree stmt : tsi_range (t))

> +	if (! build_data_member_initialization (stmt, vec))

> +	  return false;

>         return true;

>       }

>     if (TREE_CODE (t) == CLEANUP_STMT)

> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers (tree type, tree body)

>   	break;

>   

>         case STATEMENT_LIST:

> -	for (tree_stmt_iterator i = tsi_start (body);

> -	     !tsi_end_p (i); tsi_next (&i))

> +	for (tree stmt : tsi_range (body))

>   	  {

> -	    body = tsi_stmt (i);

> +	    body = stmt;

>   	    if (TREE_CODE (body) == BIND_EXPR)

>   	      break;

>   	  }

> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers (tree type, tree body)

>       }

>     else if (TREE_CODE (body) == STATEMENT_LIST)

>       {

> -      tree_stmt_iterator i;

> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

> +      for (tree stmt : tsi_range (body))

>   	{

> -	  ok = build_data_member_initialization (tsi_stmt (i), &vec);

> +	  ok = build_data_member_initialization (stmt, &vec);

>   	  if (!ok)

>   	    break;

>   	}

> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

>       {

>       case STATEMENT_LIST:

>         {

> -	tree_stmt_iterator i;

>   	tree expr = NULL_TREE;

> -	for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

> +	for (tree stmt : tsi_range (body))

>   	  {

> -	    tree s = constexpr_fn_retval (tsi_stmt (i));

> +	    tree s = constexpr_fn_retval (stmt);

>   	    if (s == error_mark_node)

>   	      return error_mark_node;

>   	    else if (s == NULL_TREE)

> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx *ctx, tree t,

>   			 bool *non_constant_p, bool *overflow_p,

>   			 tree *jump_target)

>   {

> -  tree_stmt_iterator i;

>     tree local_target;

>     /* In a statement-expression we want to return the last value.

>        For empty statement expression return void_node.  */

> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx *ctx, tree t,

>         local_target = NULL_TREE;

>         jump_target = &local_target;

>       }

> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> +  for (tree stmt : tsi_range (t))

>       {

> -      tree stmt = tsi_stmt (i);

>         /* We've found a continue, so skip everything until we reach

>   	 the label its jumping to.  */

>         if (continues (jump_target))

> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,

>         }

>   

>       case STATEMENT_LIST:

> -      {

> -	tree_stmt_iterator i;

> -	for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> -	  {

> -	    if (!RECUR (tsi_stmt (i), any))

> -	      return false;

> -	  }

> -	return true;

> -      }

> -      break;

> +      for (tree stmt : tsi_range (t))

> +	if (!RECUR (stmt, any))

> +	  return false;

> +      return true;

>   

>       case MODIFY_EXPR:

>         if (cxx_dialect < cxx14)

> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> index dbd703a67cc..9b498f9d0b4 100644

> --- a/gcc/cp/coroutines.cc

> +++ b/gcc/cp/coroutines.cc

> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int *do_subtree, void *d)

>       return NULL_TREE; /* Just process the sub-trees.  */

>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>       {

> -      tree_stmt_iterator i;

> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

> +      for (tree &s : tsi_range (*stmt))

>   	{

> -	  res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

> +	  res = cp_walk_tree (&s, await_statement_expander,

>   			      d, NULL);

>   	  if (res)

>   	    return res;

> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int *do_subtree, void *d)

>       }

>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>       {

> -      tree_stmt_iterator i;

> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

> +      for (tree &s : tsi_range (*stmt))

>   	{

> -	  res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

> +	  res = cp_walk_tree (&s, await_statement_walker,

>   			      d, NULL);

>   	  if (res)

>   	    return res;

> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

> index 02c19f55548..f0fb0144706 100644

> --- a/gcc/cp/module.cc

> +++ b/gcc/cp/module.cc

> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

>         break;

>   

>       case STATEMENT_LIST:

> -      for (tree_stmt_iterator iter = tsi_start (t);

> -	   !tsi_end_p (iter); tsi_next (&iter))

> -	if (tree stmt = tsi_stmt (iter))

> +      for (tree stmt : tsi_range (t))

> +	if (stmt)

>   	  WT (stmt);

>         WT (NULL_TREE);

>         break;

> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

> index 116bdd2e42a..ad140cfd586 100644

> --- a/gcc/cp/pt.c

> +++ b/gcc/cp/pt.c

> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,

>       {

>       case STATEMENT_LIST:

>         {

> -	tree_stmt_iterator i;

> -	for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> -	  RECUR (tsi_stmt (i));

> +	for (tree stmt : tsi_range (t))

> +	  RECUR (stmt);

>   	break;

>         }

>   

> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

> index 6224f49f189..2912efad9be 100644

> --- a/gcc/cp/semantics.c

> +++ b/gcc/cp/semantics.c

> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

>         set_cleanup_locs (CLEANUP_BODY (stmts), loc);

>       }

>     else if (TREE_CODE (stmts) == STATEMENT_LIST)

> -    for (tree_stmt_iterator i = tsi_start (stmts);

> -	 !tsi_end_p (i); tsi_next (&i))

> -      set_cleanup_locs (tsi_stmt (i), loc);

> +    for (tree stmt : tsi_range (stmts))

> +      set_cleanup_locs (stmt, loc);

>   }

>   

>   /* Finish a scope.  */

> 

> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

>
Dragan Mladjenovic via Gcc-patches May 13, 2021, 11:21 p.m. | #2
On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:
> Ping.

> 

> On 5/1/21 12:29 PM, Jason Merrill wrote:

>> Like my recent patch to add ovl_range and lkp_range in the C++ front end,

>> this patch adds the tsi_range adaptor for using C++11 range-based 

>> 'for' with

>> a STATEMENT_LIST, e.g.

>>

>>    for (tree stmt : tsi_range (stmt_list)) { ... }

>>

>> This also involves adding some operators to tree_stmt_iterator that are

>> needed for range-for iterators, and should also be useful in code that 

>> uses

>> the iterators directly.

>>

>> The patch updates the suitable loops in the C++ front end, but does not

>> touch any loops elsewhere in the compiler.


I like the modernization of the loops.

I can't find anything terribly wrong with the iterator but let me
at least pick on some nits ;)

>>

>> gcc/ChangeLog:

>>

>>     * tree-iterator.h (struct tree_stmt_iterator): Add operator++,

>>     operator--, operator*, operator==, and operator!=.

>>     (class tsi_range): New.

>>

>> gcc/cp/ChangeLog:

>>

>>     * constexpr.c (build_data_member_initialization): Use tsi_range.

>>     (build_constexpr_constructor_member_initializers): Likewise.

>>     (constexpr_fn_retval, cxx_eval_statement_list): Likewise.

>>     (potential_constant_expression_1): Likewise.

>>     * coroutines.cc (await_statement_expander): Likewise.

>>     (await_statement_walker): Likewise.

>>     * module.cc (trees_out::core_vals): Likewise.

>>     * pt.c (tsubst_expr): Likewise.

>>     * semantics.c (set_cleanup_locs): Likewise.

>> ---

>>   gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

>>   gcc/cp/constexpr.c   | 42 ++++++++++++++----------------------------

>>   gcc/cp/coroutines.cc | 10 ++++------

>>   gcc/cp/module.cc     |  5 ++---

>>   gcc/cp/pt.c          |  5 ++---

>>   gcc/cp/semantics.c   |  5 ++---

>>   6 files changed, 47 insertions(+), 48 deletions(-)

>>

>> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

>> index 076fff8644c..f57456bb473 100644

>> --- a/gcc/tree-iterator.h

>> +++ b/gcc/tree-iterator.h

>> @@ -1,4 +1,4 @@

>> -/* Iterator routines for manipulating GENERIC tree statement list.

>> +/* Iterator routines for manipulating GENERIC tree statement list. 

>> -*- C++ -*-

>>      Copyright (C) 2003-2021 Free Software Foundation, Inc.

>>      Contributed by Andrew MacLeod  <amacleod@redhat.com>

>> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

>>   struct tree_stmt_iterator {

>>     struct tree_statement_list_node *ptr;

>>     tree container;


I assume the absence of ctors is intentional.  If so, I suggest
to add a comment explaing why.  Otherwise, I would provide one
(or as many as needed).

>> +

>> +  bool operator== (tree_stmt_iterator b) const

>> +    { return b.ptr == ptr && b.container == container; }

>> +  bool operator!= (tree_stmt_iterator b) const { return !(*this == b); }

>> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }

>> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }


I would suggest to add postincrement and postdecrement.

>> +  tree &operator* () { return ptr->stmt; }


Given the pervasive lack of const-safety in GCC and the by-value
semantics of the iterator this probably isn't worth it but maybe
add a const overload.  operator-> would probably never be used.

>>   };

>>   static inline tree_stmt_iterator

>> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

>>   static inline void

>>   tsi_next (tree_stmt_iterator *i)

>>   {

>> -  i->ptr = i->ptr->next;

>> +  ++(*i);

>>   }

>>   static inline void

>>   tsi_prev (tree_stmt_iterator *i)

>>   {

>> -  i->ptr = i->ptr->prev;

>> +  --(*i);

>>   }

>>   static inline tree *

>>   tsi_stmt_ptr (tree_stmt_iterator i)

>>   {

>> -  return &i.ptr->stmt;

>> +  return &(*i);

>>   }

>>   static inline tree

>>   tsi_stmt (tree_stmt_iterator i)

>>   {

>> -  return i.ptr->stmt;

>> +  return *i;

>>   }

>> +/* Make tree_stmt_iterator work as a C++ range, e.g.

>> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

>> +class tsi_range

>> +{

>> +  tree t;

>> + public:

>> +  tsi_range (tree t): t(t) { }

>> +  tree_stmt_iterator begin() { return tsi_start (t); }

>> +  tree_stmt_iterator end() { return { nullptr, t }; }


Those member functions could be made const.

Martin

>> +};

>> +

>>   enum tsi_iterator_update

>>   {

>>     TSI_NEW_STMT,        /* Only valid when single statement is added, 

>> move

>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

>> index 9481a5bfd3c..260b0122f59 100644

>> --- a/gcc/cp/constexpr.c

>> +++ b/gcc/cp/constexpr.c

>> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t, 

>> vec<constructor_elt, va_gc> **vec)

>>       return false;

>>     if (TREE_CODE (t) == STATEMENT_LIST)

>>       {

>> -      tree_stmt_iterator i;

>> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>> -    {

>> -      if (! build_data_member_initialization (tsi_stmt (i), vec))

>> -        return false;

>> -    }

>> +      for (tree stmt : tsi_range (t))

>> +    if (! build_data_member_initialization (stmt, vec))

>> +      return false;

>>         return true;

>>       }

>>     if (TREE_CODE (t) == CLEANUP_STMT)

>> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers 

>> (tree type, tree body)

>>       break;

>>         case STATEMENT_LIST:

>> -    for (tree_stmt_iterator i = tsi_start (body);

>> -         !tsi_end_p (i); tsi_next (&i))

>> +    for (tree stmt : tsi_range (body))

>>         {

>> -        body = tsi_stmt (i);

>> +        body = stmt;

>>           if (TREE_CODE (body) == BIND_EXPR)

>>             break;

>>         }

>> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers 

>> (tree type, tree body)

>>       }

>>     else if (TREE_CODE (body) == STATEMENT_LIST)

>>       {

>> -      tree_stmt_iterator i;

>> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>> +      for (tree stmt : tsi_range (body))

>>       {

>> -      ok = build_data_member_initialization (tsi_stmt (i), &vec);

>> +      ok = build_data_member_initialization (stmt, &vec);

>>         if (!ok)

>>           break;

>>       }

>> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

>>       {

>>       case STATEMENT_LIST:

>>         {

>> -    tree_stmt_iterator i;

>>       tree expr = NULL_TREE;

>> -    for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>> +    for (tree stmt : tsi_range (body))

>>         {

>> -        tree s = constexpr_fn_retval (tsi_stmt (i));

>> +        tree s = constexpr_fn_retval (stmt);

>>           if (s == error_mark_node)

>>             return error_mark_node;

>>           else if (s == NULL_TREE)

>> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx 

>> *ctx, tree t,

>>                bool *non_constant_p, bool *overflow_p,

>>                tree *jump_target)

>>   {

>> -  tree_stmt_iterator i;

>>     tree local_target;

>>     /* In a statement-expression we want to return the last value.

>>        For empty statement expression return void_node.  */

>> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx 

>> *ctx, tree t,

>>         local_target = NULL_TREE;

>>         jump_target = &local_target;

>>       }

>> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>> +  for (tree stmt : tsi_range (t))

>>       {

>> -      tree stmt = tsi_stmt (i);

>>         /* We've found a continue, so skip everything until we reach

>>        the label its jumping to.  */

>>         if (continues (jump_target))

>> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool 

>> want_rval, bool strict, bool now,

>>         }

>>       case STATEMENT_LIST:

>> -      {

>> -    tree_stmt_iterator i;

>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>> -      {

>> -        if (!RECUR (tsi_stmt (i), any))

>> -          return false;

>> -      }

>> -    return true;

>> -      }

>> -      break;

>> +      for (tree stmt : tsi_range (t))

>> +    if (!RECUR (stmt, any))

>> +      return false;

>> +      return true;

>>       case MODIFY_EXPR:

>>         if (cxx_dialect < cxx14)

>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

>> index dbd703a67cc..9b498f9d0b4 100644

>> --- a/gcc/cp/coroutines.cc

>> +++ b/gcc/cp/coroutines.cc

>> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int 

>> *do_subtree, void *d)

>>       return NULL_TREE; /* Just process the sub-trees.  */

>>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>       {

>> -      tree_stmt_iterator i;

>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>> +      for (tree &s : tsi_range (*stmt))

>>       {

>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

>> +      res = cp_walk_tree (&s, await_statement_expander,

>>                     d, NULL);

>>         if (res)

>>           return res;

>> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int 

>> *do_subtree, void *d)

>>       }

>>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>       {

>> -      tree_stmt_iterator i;

>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>> +      for (tree &s : tsi_range (*stmt))

>>       {

>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

>> +      res = cp_walk_tree (&s, await_statement_walker,

>>                     d, NULL);

>>         if (res)

>>           return res;

>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

>> index 02c19f55548..f0fb0144706 100644

>> --- a/gcc/cp/module.cc

>> +++ b/gcc/cp/module.cc

>> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

>>         break;

>>       case STATEMENT_LIST:

>> -      for (tree_stmt_iterator iter = tsi_start (t);

>> -       !tsi_end_p (iter); tsi_next (&iter))

>> -    if (tree stmt = tsi_stmt (iter))

>> +      for (tree stmt : tsi_range (t))

>> +    if (stmt)

>>         WT (stmt);

>>         WT (NULL_TREE);

>>         break;

>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

>> index 116bdd2e42a..ad140cfd586 100644

>> --- a/gcc/cp/pt.c

>> +++ b/gcc/cp/pt.c

>> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t 

>> complain, tree in_decl,

>>       {

>>       case STATEMENT_LIST:

>>         {

>> -    tree_stmt_iterator i;

>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>> -      RECUR (tsi_stmt (i));

>> +    for (tree stmt : tsi_range (t))

>> +      RECUR (stmt);

>>       break;

>>         }

>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

>> index 6224f49f189..2912efad9be 100644

>> --- a/gcc/cp/semantics.c

>> +++ b/gcc/cp/semantics.c

>> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

>>         set_cleanup_locs (CLEANUP_BODY (stmts), loc);

>>       }

>>     else if (TREE_CODE (stmts) == STATEMENT_LIST)

>> -    for (tree_stmt_iterator i = tsi_start (stmts);

>> -     !tsi_end_p (i); tsi_next (&i))

>> -      set_cleanup_locs (tsi_stmt (i), loc);

>> +    for (tree stmt : tsi_range (stmts))

>> +      set_cleanup_locs (stmt, loc);

>>   }

>>   /* Finish a scope.  */

>>

>> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

>> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

>>

>
Dragan Mladjenovic via Gcc-patches May 14, 2021, 1:50 a.m. | #3
On 5/13/21 7:21 PM, Martin Sebor wrote:
> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:

>> Ping.

>>

>> On 5/1/21 12:29 PM, Jason Merrill wrote:

>>> Like my recent patch to add ovl_range and lkp_range in the C++ front 

>>> end,

>>> this patch adds the tsi_range adaptor for using C++11 range-based 

>>> 'for' with

>>> a STATEMENT_LIST, e.g.

>>>

>>>    for (tree stmt : tsi_range (stmt_list)) { ... }

>>>

>>> This also involves adding some operators to tree_stmt_iterator that are

>>> needed for range-for iterators, and should also be useful in code 

>>> that uses

>>> the iterators directly.

>>>

>>> The patch updates the suitable loops in the C++ front end, but does not

>>> touch any loops elsewhere in the compiler.

> 

> I like the modernization of the loops.

> 

> I can't find anything terribly wrong with the iterator but let me

> at least pick on some nits ;)

> 

>>>

>>> gcc/ChangeLog:

>>>

>>>     * tree-iterator.h (struct tree_stmt_iterator): Add operator++,

>>>     operator--, operator*, operator==, and operator!=.

>>>     (class tsi_range): New.

>>>

>>> gcc/cp/ChangeLog:

>>>

>>>     * constexpr.c (build_data_member_initialization): Use tsi_range.

>>>     (build_constexpr_constructor_member_initializers): Likewise.

>>>     (constexpr_fn_retval, cxx_eval_statement_list): Likewise.

>>>     (potential_constant_expression_1): Likewise.

>>>     * coroutines.cc (await_statement_expander): Likewise.

>>>     (await_statement_walker): Likewise.

>>>     * module.cc (trees_out::core_vals): Likewise.

>>>     * pt.c (tsubst_expr): Likewise.

>>>     * semantics.c (set_cleanup_locs): Likewise.

>>> ---

>>>   gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

>>>   gcc/cp/constexpr.c   | 42 ++++++++++++++----------------------------

>>>   gcc/cp/coroutines.cc | 10 ++++------

>>>   gcc/cp/module.cc     |  5 ++---

>>>   gcc/cp/pt.c          |  5 ++---

>>>   gcc/cp/semantics.c   |  5 ++---

>>>   6 files changed, 47 insertions(+), 48 deletions(-)

>>>

>>> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

>>> index 076fff8644c..f57456bb473 100644

>>> --- a/gcc/tree-iterator.h

>>> +++ b/gcc/tree-iterator.h

>>> @@ -1,4 +1,4 @@

>>> -/* Iterator routines for manipulating GENERIC tree statement list.

>>> +/* Iterator routines for manipulating GENERIC tree statement list. 

>>> -*- C++ -*-

>>>      Copyright (C) 2003-2021 Free Software Foundation, Inc.

>>>      Contributed by Andrew MacLeod  <amacleod@redhat.com>

>>> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

>>>   struct tree_stmt_iterator {

>>>     struct tree_statement_list_node *ptr;

>>>     tree container;

> 

> I assume the absence of ctors is intentional.  If so, I suggest

> to add a comment explaing why.  Otherwise, I would provide one

> (or as many as needed).

> 

>>> +

>>> +  bool operator== (tree_stmt_iterator b) const

>>> +    { return b.ptr == ptr && b.container == container; }

>>> +  bool operator!= (tree_stmt_iterator b) const { return !(*this == 

>>> b); }

>>> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }

>>> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }

> 

> I would suggest to add postincrement and postdecrement.

> 

>>> +  tree &operator* () { return ptr->stmt; }

> 

> Given the pervasive lack of const-safety in GCC and the by-value

> semantics of the iterator this probably isn't worth it but maybe

> add a const overload.  operator-> would probably never be used.

> 

>>>   };

>>>   static inline tree_stmt_iterator

>>> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

>>>   static inline void

>>>   tsi_next (tree_stmt_iterator *i)

>>>   {

>>> -  i->ptr = i->ptr->next;

>>> +  ++(*i);

>>>   }

>>>   static inline void

>>>   tsi_prev (tree_stmt_iterator *i)

>>>   {

>>> -  i->ptr = i->ptr->prev;

>>> +  --(*i);

>>>   }

>>>   static inline tree *

>>>   tsi_stmt_ptr (tree_stmt_iterator i)

>>>   {

>>> -  return &i.ptr->stmt;

>>> +  return &(*i);

>>>   }

>>>   static inline tree

>>>   tsi_stmt (tree_stmt_iterator i)

>>>   {

>>> -  return i.ptr->stmt;

>>> +  return *i;

>>>   }

>>> +/* Make tree_stmt_iterator work as a C++ range, e.g.

>>> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

>>> +class tsi_range

>>> +{

>>> +  tree t;

>>> + public:

>>> +  tsi_range (tree t): t(t) { }

>>> +  tree_stmt_iterator begin() { return tsi_start (t); }

>>> +  tree_stmt_iterator end() { return { nullptr, t }; }

> 

> Those member functions could be made const.


Sure:

>>> +};

>>> +

>>>   enum tsi_iterator_update

>>>   {

>>>     TSI_NEW_STMT,        /* Only valid when single statement is 

>>> added, move

>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

>>> index 9481a5bfd3c..260b0122f59 100644

>>> --- a/gcc/cp/constexpr.c

>>> +++ b/gcc/cp/constexpr.c

>>> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t, 

>>> vec<constructor_elt, va_gc> **vec)

>>>       return false;

>>>     if (TREE_CODE (t) == STATEMENT_LIST)

>>>       {

>>> -      tree_stmt_iterator i;

>>> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>> -    {

>>> -      if (! build_data_member_initialization (tsi_stmt (i), vec))

>>> -        return false;

>>> -    }

>>> +      for (tree stmt : tsi_range (t))

>>> +    if (! build_data_member_initialization (stmt, vec))

>>> +      return false;

>>>         return true;

>>>       }

>>>     if (TREE_CODE (t) == CLEANUP_STMT)

>>> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers 

>>> (tree type, tree body)

>>>       break;

>>>         case STATEMENT_LIST:

>>> -    for (tree_stmt_iterator i = tsi_start (body);

>>> -         !tsi_end_p (i); tsi_next (&i))

>>> +    for (tree stmt : tsi_range (body))

>>>         {

>>> -        body = tsi_stmt (i);

>>> +        body = stmt;

>>>           if (TREE_CODE (body) == BIND_EXPR)

>>>             break;

>>>         }

>>> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers 

>>> (tree type, tree body)

>>>       }

>>>     else if (TREE_CODE (body) == STATEMENT_LIST)

>>>       {

>>> -      tree_stmt_iterator i;

>>> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>>> +      for (tree stmt : tsi_range (body))

>>>       {

>>> -      ok = build_data_member_initialization (tsi_stmt (i), &vec);

>>> +      ok = build_data_member_initialization (stmt, &vec);

>>>         if (!ok)

>>>           break;

>>>       }

>>> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

>>>       {

>>>       case STATEMENT_LIST:

>>>         {

>>> -    tree_stmt_iterator i;

>>>       tree expr = NULL_TREE;

>>> -    for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>>> +    for (tree stmt : tsi_range (body))

>>>         {

>>> -        tree s = constexpr_fn_retval (tsi_stmt (i));

>>> +        tree s = constexpr_fn_retval (stmt);

>>>           if (s == error_mark_node)

>>>             return error_mark_node;

>>>           else if (s == NULL_TREE)

>>> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx 

>>> *ctx, tree t,

>>>                bool *non_constant_p, bool *overflow_p,

>>>                tree *jump_target)

>>>   {

>>> -  tree_stmt_iterator i;

>>>     tree local_target;

>>>     /* In a statement-expression we want to return the last value.

>>>        For empty statement expression return void_node.  */

>>> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx 

>>> *ctx, tree t,

>>>         local_target = NULL_TREE;

>>>         jump_target = &local_target;

>>>       }

>>> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>> +  for (tree stmt : tsi_range (t))

>>>       {

>>> -      tree stmt = tsi_stmt (i);

>>>         /* We've found a continue, so skip everything until we reach

>>>        the label its jumping to.  */

>>>         if (continues (jump_target))

>>> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool 

>>> want_rval, bool strict, bool now,

>>>         }

>>>       case STATEMENT_LIST:

>>> -      {

>>> -    tree_stmt_iterator i;

>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>> -      {

>>> -        if (!RECUR (tsi_stmt (i), any))

>>> -          return false;

>>> -      }

>>> -    return true;

>>> -      }

>>> -      break;

>>> +      for (tree stmt : tsi_range (t))

>>> +    if (!RECUR (stmt, any))

>>> +      return false;

>>> +      return true;

>>>       case MODIFY_EXPR:

>>>         if (cxx_dialect < cxx14)

>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

>>> index dbd703a67cc..9b498f9d0b4 100644

>>> --- a/gcc/cp/coroutines.cc

>>> +++ b/gcc/cp/coroutines.cc

>>> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int 

>>> *do_subtree, void *d)

>>>       return NULL_TREE; /* Just process the sub-trees.  */

>>>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>>       {

>>> -      tree_stmt_iterator i;

>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>>> +      for (tree &s : tsi_range (*stmt))

>>>       {

>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

>>> +      res = cp_walk_tree (&s, await_statement_expander,

>>>                     d, NULL);

>>>         if (res)

>>>           return res;

>>> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int 

>>> *do_subtree, void *d)

>>>       }

>>>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>>       {

>>> -      tree_stmt_iterator i;

>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>>> +      for (tree &s : tsi_range (*stmt))

>>>       {

>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

>>> +      res = cp_walk_tree (&s, await_statement_walker,

>>>                     d, NULL);

>>>         if (res)

>>>           return res;

>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

>>> index 02c19f55548..f0fb0144706 100644

>>> --- a/gcc/cp/module.cc

>>> +++ b/gcc/cp/module.cc

>>> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

>>>         break;

>>>       case STATEMENT_LIST:

>>> -      for (tree_stmt_iterator iter = tsi_start (t);

>>> -       !tsi_end_p (iter); tsi_next (&iter))

>>> -    if (tree stmt = tsi_stmt (iter))

>>> +      for (tree stmt : tsi_range (t))

>>> +    if (stmt)

>>>         WT (stmt);

>>>         WT (NULL_TREE);

>>>         break;

>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

>>> index 116bdd2e42a..ad140cfd586 100644

>>> --- a/gcc/cp/pt.c

>>> +++ b/gcc/cp/pt.c

>>> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, 

>>> tsubst_flags_t complain, tree in_decl,

>>>       {

>>>       case STATEMENT_LIST:

>>>         {

>>> -    tree_stmt_iterator i;

>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>> -      RECUR (tsi_stmt (i));

>>> +    for (tree stmt : tsi_range (t))

>>> +      RECUR (stmt);

>>>       break;

>>>         }

>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

>>> index 6224f49f189..2912efad9be 100644

>>> --- a/gcc/cp/semantics.c

>>> +++ b/gcc/cp/semantics.c

>>> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

>>>         set_cleanup_locs (CLEANUP_BODY (stmts), loc);

>>>       }

>>>     else if (TREE_CODE (stmts) == STATEMENT_LIST)

>>> -    for (tree_stmt_iterator i = tsi_start (stmts);

>>> -     !tsi_end_p (i); tsi_next (&i))

>>> -      set_cleanup_locs (tsi_stmt (i), loc);

>>> +    for (tree stmt : tsi_range (stmts))

>>> +      set_cleanup_locs (stmt, loc);

>>>   }

>>>   /* Finish a scope.  */

>>>

>>> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

>>> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

>>>

>>

>
From 964005e6f08c3eff8b5cd8dfb4e4def5dfc28709 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>

Date: Thu, 13 May 2021 21:21:23 -0400
Subject: [PATCH] fixup! tree-iterator: C++11 range-for and tree_stmt_iterator
To: gcc-patches@gcc.gnu.org

---
 gcc/tree-iterator.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h
index f57456bb473..a72d0d37f1c 100644
--- a/gcc/tree-iterator.h
+++ b/gcc/tree-iterator.h
@@ -33,12 +33,20 @@ struct tree_stmt_iterator {
   struct tree_statement_list_node *ptr;
   tree container;
 
+  /* No need for user-defined constructors, the implicit definitions (or
+     aggregate initialization) are fine.  */
+
   bool operator== (tree_stmt_iterator b) const
     { return b.ptr == ptr && b.container == container; }
   bool operator!= (tree_stmt_iterator b) const { return !(*this == b); }
   tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }
   tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }
+  tree_stmt_iterator operator++ (int)
+    { tree_stmt_iterator x = *this; ++*this; return x; }
+  tree_stmt_iterator operator-- (int)
+    { tree_stmt_iterator x = *this; --*this; return x; }
   tree &operator* () { return ptr->stmt; }
+  tree operator* () const { return ptr->stmt; }
 };
 
 static inline tree_stmt_iterator
@@ -106,8 +114,8 @@ class tsi_range
   tree t;
  public:
   tsi_range (tree t): t(t) { }
-  tree_stmt_iterator begin() { return tsi_start (t); }
-  tree_stmt_iterator end() { return { nullptr, t }; }
+  tree_stmt_iterator begin() const { return tsi_start (t); }
+  tree_stmt_iterator end() const { return { nullptr, t }; }
 };
 
 enum tsi_iterator_update
-- 
2.27.0
Dragan Mladjenovic via Gcc-patches May 17, 2021, 7:56 a.m. | #4
On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>

> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:

> > Ping.

> >

> > On 5/1/21 12:29 PM, Jason Merrill wrote:

> >> Like my recent patch to add ovl_range and lkp_range in the C++ front end,

> >> this patch adds the tsi_range adaptor for using C++11 range-based

> >> 'for' with

> >> a STATEMENT_LIST, e.g.

> >>

> >>    for (tree stmt : tsi_range (stmt_list)) { ... }

> >>

> >> This also involves adding some operators to tree_stmt_iterator that are

> >> needed for range-for iterators, and should also be useful in code that

> >> uses

> >> the iterators directly.

> >>

> >> The patch updates the suitable loops in the C++ front end, but does not

> >> touch any loops elsewhere in the compiler.

>

> I like the modernization of the loops.


The only worry I have (and why I stopped looking at range-for) is that
this adds another style of looping over stmts without opening the
possibility to remove another or even unify all of them.  That's because
range-for isn't powerful enough w/o jumping through hoops and/or
we cannot use what appearantly ranges<> was intended for (fix
this limitation).

That said, if some C++ literate could see if for example
what gimple-iterator.h provides can be completely modernized
then that would be great of course.

There's stuff like reverse iteration, iteration skipping debug stmts,
compares of iterators like gsi_one_before_end_p, etc.

Given my failed tries (but I'm a C++ illiterate) my TODO list now
only contains turning the iterators into STL style ones, thus
gsi_stmt (it) -> *it, gsi_next (&it) -> ++it, etc. - but even
it != end_p looks a bit awkward there.

Richard.

> I can't find anything terribly wrong with the iterator but let me

> at least pick on some nits ;)

>

> >>

> >> gcc/ChangeLog:

> >>

> >>     * tree-iterator.h (struct tree_stmt_iterator): Add operator++,

> >>     operator--, operator*, operator==, and operator!=.

> >>     (class tsi_range): New.

> >>

> >> gcc/cp/ChangeLog:

> >>

> >>     * constexpr.c (build_data_member_initialization): Use tsi_range.

> >>     (build_constexpr_constructor_member_initializers): Likewise.

> >>     (constexpr_fn_retval, cxx_eval_statement_list): Likewise.

> >>     (potential_constant_expression_1): Likewise.

> >>     * coroutines.cc (await_statement_expander): Likewise.

> >>     (await_statement_walker): Likewise.

> >>     * module.cc (trees_out::core_vals): Likewise.

> >>     * pt.c (tsubst_expr): Likewise.

> >>     * semantics.c (set_cleanup_locs): Likewise.

> >> ---

> >>   gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

> >>   gcc/cp/constexpr.c   | 42 ++++++++++++++----------------------------

> >>   gcc/cp/coroutines.cc | 10 ++++------

> >>   gcc/cp/module.cc     |  5 ++---

> >>   gcc/cp/pt.c          |  5 ++---

> >>   gcc/cp/semantics.c   |  5 ++---

> >>   6 files changed, 47 insertions(+), 48 deletions(-)

> >>

> >> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

> >> index 076fff8644c..f57456bb473 100644

> >> --- a/gcc/tree-iterator.h

> >> +++ b/gcc/tree-iterator.h

> >> @@ -1,4 +1,4 @@

> >> -/* Iterator routines for manipulating GENERIC tree statement list.

> >> +/* Iterator routines for manipulating GENERIC tree statement list.

> >> -*- C++ -*-

> >>      Copyright (C) 2003-2021 Free Software Foundation, Inc.

> >>      Contributed by Andrew MacLeod  <amacleod@redhat.com>

> >> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

> >>   struct tree_stmt_iterator {

> >>     struct tree_statement_list_node *ptr;

> >>     tree container;

>

> I assume the absence of ctors is intentional.  If so, I suggest

> to add a comment explaing why.  Otherwise, I would provide one

> (or as many as needed).

>

> >> +

> >> +  bool operator== (tree_stmt_iterator b) const

> >> +    { return b.ptr == ptr && b.container == container; }

> >> +  bool operator!= (tree_stmt_iterator b) const { return !(*this == b); }

> >> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }

> >> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }

>

> I would suggest to add postincrement and postdecrement.

>

> >> +  tree &operator* () { return ptr->stmt; }

>

> Given the pervasive lack of const-safety in GCC and the by-value

> semantics of the iterator this probably isn't worth it but maybe

> add a const overload.  operator-> would probably never be used.

>

> >>   };

> >>   static inline tree_stmt_iterator

> >> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

> >>   static inline void

> >>   tsi_next (tree_stmt_iterator *i)

> >>   {

> >> -  i->ptr = i->ptr->next;

> >> +  ++(*i);

> >>   }

> >>   static inline void

> >>   tsi_prev (tree_stmt_iterator *i)

> >>   {

> >> -  i->ptr = i->ptr->prev;

> >> +  --(*i);

> >>   }

> >>   static inline tree *

> >>   tsi_stmt_ptr (tree_stmt_iterator i)

> >>   {

> >> -  return &i.ptr->stmt;

> >> +  return &(*i);

> >>   }

> >>   static inline tree

> >>   tsi_stmt (tree_stmt_iterator i)

> >>   {

> >> -  return i.ptr->stmt;

> >> +  return *i;

> >>   }

> >> +/* Make tree_stmt_iterator work as a C++ range, e.g.

> >> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

> >> +class tsi_range

> >> +{

> >> +  tree t;

> >> + public:

> >> +  tsi_range (tree t): t(t) { }

> >> +  tree_stmt_iterator begin() { return tsi_start (t); }

> >> +  tree_stmt_iterator end() { return { nullptr, t }; }

>

> Those member functions could be made const.

>

> Martin

>

> >> +};

> >> +

> >>   enum tsi_iterator_update

> >>   {

> >>     TSI_NEW_STMT,        /* Only valid when single statement is added,

> >> move

> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

> >> index 9481a5bfd3c..260b0122f59 100644

> >> --- a/gcc/cp/constexpr.c

> >> +++ b/gcc/cp/constexpr.c

> >> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t,

> >> vec<constructor_elt, va_gc> **vec)

> >>       return false;

> >>     if (TREE_CODE (t) == STATEMENT_LIST)

> >>       {

> >> -      tree_stmt_iterator i;

> >> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >> -    {

> >> -      if (! build_data_member_initialization (tsi_stmt (i), vec))

> >> -        return false;

> >> -    }

> >> +      for (tree stmt : tsi_range (t))

> >> +    if (! build_data_member_initialization (stmt, vec))

> >> +      return false;

> >>         return true;

> >>       }

> >>     if (TREE_CODE (t) == CLEANUP_STMT)

> >> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers

> >> (tree type, tree body)

> >>       break;

> >>         case STATEMENT_LIST:

> >> -    for (tree_stmt_iterator i = tsi_start (body);

> >> -         !tsi_end_p (i); tsi_next (&i))

> >> +    for (tree stmt : tsi_range (body))

> >>         {

> >> -        body = tsi_stmt (i);

> >> +        body = stmt;

> >>           if (TREE_CODE (body) == BIND_EXPR)

> >>             break;

> >>         }

> >> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers

> >> (tree type, tree body)

> >>       }

> >>     else if (TREE_CODE (body) == STATEMENT_LIST)

> >>       {

> >> -      tree_stmt_iterator i;

> >> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

> >> +      for (tree stmt : tsi_range (body))

> >>       {

> >> -      ok = build_data_member_initialization (tsi_stmt (i), &vec);

> >> +      ok = build_data_member_initialization (stmt, &vec);

> >>         if (!ok)

> >>           break;

> >>       }

> >> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

> >>       {

> >>       case STATEMENT_LIST:

> >>         {

> >> -    tree_stmt_iterator i;

> >>       tree expr = NULL_TREE;

> >> -    for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

> >> +    for (tree stmt : tsi_range (body))

> >>         {

> >> -        tree s = constexpr_fn_retval (tsi_stmt (i));

> >> +        tree s = constexpr_fn_retval (stmt);

> >>           if (s == error_mark_node)

> >>             return error_mark_node;

> >>           else if (s == NULL_TREE)

> >> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx

> >> *ctx, tree t,

> >>                bool *non_constant_p, bool *overflow_p,

> >>                tree *jump_target)

> >>   {

> >> -  tree_stmt_iterator i;

> >>     tree local_target;

> >>     /* In a statement-expression we want to return the last value.

> >>        For empty statement expression return void_node.  */

> >> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx

> >> *ctx, tree t,

> >>         local_target = NULL_TREE;

> >>         jump_target = &local_target;

> >>       }

> >> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >> +  for (tree stmt : tsi_range (t))

> >>       {

> >> -      tree stmt = tsi_stmt (i);

> >>         /* We've found a continue, so skip everything until we reach

> >>        the label its jumping to.  */

> >>         if (continues (jump_target))

> >> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool

> >> want_rval, bool strict, bool now,

> >>         }

> >>       case STATEMENT_LIST:

> >> -      {

> >> -    tree_stmt_iterator i;

> >> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >> -      {

> >> -        if (!RECUR (tsi_stmt (i), any))

> >> -          return false;

> >> -      }

> >> -    return true;

> >> -      }

> >> -      break;

> >> +      for (tree stmt : tsi_range (t))

> >> +    if (!RECUR (stmt, any))

> >> +      return false;

> >> +      return true;

> >>       case MODIFY_EXPR:

> >>         if (cxx_dialect < cxx14)

> >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> >> index dbd703a67cc..9b498f9d0b4 100644

> >> --- a/gcc/cp/coroutines.cc

> >> +++ b/gcc/cp/coroutines.cc

> >> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int

> >> *do_subtree, void *d)

> >>       return NULL_TREE; /* Just process the sub-trees.  */

> >>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

> >>       {

> >> -      tree_stmt_iterator i;

> >> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

> >> +      for (tree &s : tsi_range (*stmt))

> >>       {

> >> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

> >> +      res = cp_walk_tree (&s, await_statement_expander,

> >>                     d, NULL);

> >>         if (res)

> >>           return res;

> >> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int

> >> *do_subtree, void *d)

> >>       }

> >>     else if (TREE_CODE (*stmt) == STATEMENT_LIST)

> >>       {

> >> -      tree_stmt_iterator i;

> >> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

> >> +      for (tree &s : tsi_range (*stmt))

> >>       {

> >> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

> >> +      res = cp_walk_tree (&s, await_statement_walker,

> >>                     d, NULL);

> >>         if (res)

> >>           return res;

> >> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

> >> index 02c19f55548..f0fb0144706 100644

> >> --- a/gcc/cp/module.cc

> >> +++ b/gcc/cp/module.cc

> >> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

> >>         break;

> >>       case STATEMENT_LIST:

> >> -      for (tree_stmt_iterator iter = tsi_start (t);

> >> -       !tsi_end_p (iter); tsi_next (&iter))

> >> -    if (tree stmt = tsi_stmt (iter))

> >> +      for (tree stmt : tsi_range (t))

> >> +    if (stmt)

> >>         WT (stmt);

> >>         WT (NULL_TREE);

> >>         break;

> >> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

> >> index 116bdd2e42a..ad140cfd586 100644

> >> --- a/gcc/cp/pt.c

> >> +++ b/gcc/cp/pt.c

> >> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t

> >> complain, tree in_decl,

> >>       {

> >>       case STATEMENT_LIST:

> >>         {

> >> -    tree_stmt_iterator i;

> >> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >> -      RECUR (tsi_stmt (i));

> >> +    for (tree stmt : tsi_range (t))

> >> +      RECUR (stmt);

> >>       break;

> >>         }

> >> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

> >> index 6224f49f189..2912efad9be 100644

> >> --- a/gcc/cp/semantics.c

> >> +++ b/gcc/cp/semantics.c

> >> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

> >>         set_cleanup_locs (CLEANUP_BODY (stmts), loc);

> >>       }

> >>     else if (TREE_CODE (stmts) == STATEMENT_LIST)

> >> -    for (tree_stmt_iterator i = tsi_start (stmts);

> >> -     !tsi_end_p (i); tsi_next (&i))

> >> -      set_cleanup_locs (tsi_stmt (i), loc);

> >> +    for (tree stmt : tsi_range (stmts))

> >> +      set_cleanup_locs (stmt, loc);

> >>   }

> >>   /* Finish a scope.  */

> >>

> >> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

> >> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

> >>

> >

>
Dragan Mladjenovic via Gcc-patches May 17, 2021, 5:58 p.m. | #5
On 5/17/21 3:56 AM, Richard Biener wrote:
> On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches

> <gcc-patches@gcc.gnu.org> wrote:

>>

>> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:

>>> Ping.

>>>

>>> On 5/1/21 12:29 PM, Jason Merrill wrote:

>>>> Like my recent patch to add ovl_range and lkp_range in the C++ front end,

>>>> this patch adds the tsi_range adaptor for using C++11 range-based

>>>> 'for' with

>>>> a STATEMENT_LIST, e.g.

>>>>

>>>>     for (tree stmt : tsi_range (stmt_list)) { ... }

>>>>

>>>> This also involves adding some operators to tree_stmt_iterator that are

>>>> needed for range-for iterators, and should also be useful in code that

>>>> uses

>>>> the iterators directly.

>>>>

>>>> The patch updates the suitable loops in the C++ front end, but does not

>>>> touch any loops elsewhere in the compiler.

>>

>> I like the modernization of the loops.

> 

> The only worry I have (and why I stopped looking at range-for) is that

> this adds another style of looping over stmts without opening the

> possibility to remove another or even unify all of them.  That's because

> range-for isn't powerful enough w/o jumping through hoops and/or

> we cannot use what appearantly ranges<> was intended for (fix

> this limitation).


The range-for enabled by my patch simplifies the common case of simple 
iteration over elements; that seems worth doing to me even if it doesn't 
replace all loops.  Just as FOR_EACH_VEC_ELT isn't suitable for all 
loops over a vector.

> That said, if some C++ literate could see if for example

> what gimple-iterator.h provides can be completely modernized

> then that would be great of course.

> 

> There's stuff like reverse iteration


This is typically done with the reverse_iterator<> adaptor, which we 
could get from <iterator> or duplicate.  I didn't see enough reverse 
iterations to make it seem worth the bother.

> iteration skipping debug stmts,


There you can move the condition into the loop:

if (gimple_code (stmt) == GIMPLE_DEBUG)
   continue;

> compares of iterators like gsi_one_before_end_p, etc.


Certainly anything where you want to mess with the iterators directly 
doesn't really translate to range-for.

> Given my failed tries (but I'm a C++ illiterate) my TODO list now

> only contains turning the iterators into STL style ones, thus

> gsi_stmt (it) -> *it, gsi_next (&it) -> ++it, etc. - but even

> it != end_p looks a bit awkward there.


Well, it < end_val is pretty typical for loops involving integer 
iterators.  But you don't have to use that style if you'd rather not. 
You could continue to use gsi_end_p, or just *it, since we know that *it 
is NULL at the end of the sequence.

>> I can't find anything terribly wrong with the iterator but let me

>> at least pick on some nits ;)

>>

>>>>

>>>> gcc/ChangeLog:

>>>>

>>>>      * tree-iterator.h (struct tree_stmt_iterator): Add operator++,

>>>>      operator--, operator*, operator==, and operator!=.

>>>>      (class tsi_range): New.

>>>>

>>>> gcc/cp/ChangeLog:

>>>>

>>>>      * constexpr.c (build_data_member_initialization): Use tsi_range.

>>>>      (build_constexpr_constructor_member_initializers): Likewise.

>>>>      (constexpr_fn_retval, cxx_eval_statement_list): Likewise.

>>>>      (potential_constant_expression_1): Likewise.

>>>>      * coroutines.cc (await_statement_expander): Likewise.

>>>>      (await_statement_walker): Likewise.

>>>>      * module.cc (trees_out::core_vals): Likewise.

>>>>      * pt.c (tsubst_expr): Likewise.

>>>>      * semantics.c (set_cleanup_locs): Likewise.

>>>> ---

>>>>    gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

>>>>    gcc/cp/constexpr.c   | 42 ++++++++++++++----------------------------

>>>>    gcc/cp/coroutines.cc | 10 ++++------

>>>>    gcc/cp/module.cc     |  5 ++---

>>>>    gcc/cp/pt.c          |  5 ++---

>>>>    gcc/cp/semantics.c   |  5 ++---

>>>>    6 files changed, 47 insertions(+), 48 deletions(-)

>>>>

>>>> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

>>>> index 076fff8644c..f57456bb473 100644

>>>> --- a/gcc/tree-iterator.h

>>>> +++ b/gcc/tree-iterator.h

>>>> @@ -1,4 +1,4 @@

>>>> -/* Iterator routines for manipulating GENERIC tree statement list.

>>>> +/* Iterator routines for manipulating GENERIC tree statement list.

>>>> -*- C++ -*-

>>>>       Copyright (C) 2003-2021 Free Software Foundation, Inc.

>>>>       Contributed by Andrew MacLeod  <amacleod@redhat.com>

>>>> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

>>>>    struct tree_stmt_iterator {

>>>>      struct tree_statement_list_node *ptr;

>>>>      tree container;

>>

>> I assume the absence of ctors is intentional.  If so, I suggest

>> to add a comment explaing why.  Otherwise, I would provide one

>> (or as many as needed).

>>

>>>> +

>>>> +  bool operator== (tree_stmt_iterator b) const

>>>> +    { return b.ptr == ptr && b.container == container; }

>>>> +  bool operator!= (tree_stmt_iterator b) const { return !(*this == b); }

>>>> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }

>>>> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }

>>

>> I would suggest to add postincrement and postdecrement.

>>

>>>> +  tree &operator* () { return ptr->stmt; }

>>

>> Given the pervasive lack of const-safety in GCC and the by-value

>> semantics of the iterator this probably isn't worth it but maybe

>> add a const overload.  operator-> would probably never be used.

>>

>>>>    };

>>>>    static inline tree_stmt_iterator

>>>> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

>>>>    static inline void

>>>>    tsi_next (tree_stmt_iterator *i)

>>>>    {

>>>> -  i->ptr = i->ptr->next;

>>>> +  ++(*i);

>>>>    }

>>>>    static inline void

>>>>    tsi_prev (tree_stmt_iterator *i)

>>>>    {

>>>> -  i->ptr = i->ptr->prev;

>>>> +  --(*i);

>>>>    }

>>>>    static inline tree *

>>>>    tsi_stmt_ptr (tree_stmt_iterator i)

>>>>    {

>>>> -  return &i.ptr->stmt;

>>>> +  return &(*i);

>>>>    }

>>>>    static inline tree

>>>>    tsi_stmt (tree_stmt_iterator i)

>>>>    {

>>>> -  return i.ptr->stmt;

>>>> +  return *i;

>>>>    }

>>>> +/* Make tree_stmt_iterator work as a C++ range, e.g.

>>>> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

>>>> +class tsi_range

>>>> +{

>>>> +  tree t;

>>>> + public:

>>>> +  tsi_range (tree t): t(t) { }

>>>> +  tree_stmt_iterator begin() { return tsi_start (t); }

>>>> +  tree_stmt_iterator end() { return { nullptr, t }; }

>>

>> Those member functions could be made const.

>>

>> Martin

>>

>>>> +};

>>>> +

>>>>    enum tsi_iterator_update

>>>>    {

>>>>      TSI_NEW_STMT,        /* Only valid when single statement is added,

>>>> move

>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

>>>> index 9481a5bfd3c..260b0122f59 100644

>>>> --- a/gcc/cp/constexpr.c

>>>> +++ b/gcc/cp/constexpr.c

>>>> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t,

>>>> vec<constructor_elt, va_gc> **vec)

>>>>        return false;

>>>>      if (TREE_CODE (t) == STATEMENT_LIST)

>>>>        {

>>>> -      tree_stmt_iterator i;

>>>> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>> -    {

>>>> -      if (! build_data_member_initialization (tsi_stmt (i), vec))

>>>> -        return false;

>>>> -    }

>>>> +      for (tree stmt : tsi_range (t))

>>>> +    if (! build_data_member_initialization (stmt, vec))

>>>> +      return false;

>>>>          return true;

>>>>        }

>>>>      if (TREE_CODE (t) == CLEANUP_STMT)

>>>> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers

>>>> (tree type, tree body)

>>>>        break;

>>>>          case STATEMENT_LIST:

>>>> -    for (tree_stmt_iterator i = tsi_start (body);

>>>> -         !tsi_end_p (i); tsi_next (&i))

>>>> +    for (tree stmt : tsi_range (body))

>>>>          {

>>>> -        body = tsi_stmt (i);

>>>> +        body = stmt;

>>>>            if (TREE_CODE (body) == BIND_EXPR)

>>>>              break;

>>>>          }

>>>> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers

>>>> (tree type, tree body)

>>>>        }

>>>>      else if (TREE_CODE (body) == STATEMENT_LIST)

>>>>        {

>>>> -      tree_stmt_iterator i;

>>>> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>>>> +      for (tree stmt : tsi_range (body))

>>>>        {

>>>> -      ok = build_data_member_initialization (tsi_stmt (i), &vec);

>>>> +      ok = build_data_member_initialization (stmt, &vec);

>>>>          if (!ok)

>>>>            break;

>>>>        }

>>>> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

>>>>        {

>>>>        case STATEMENT_LIST:

>>>>          {

>>>> -    tree_stmt_iterator i;

>>>>        tree expr = NULL_TREE;

>>>> -    for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>>>> +    for (tree stmt : tsi_range (body))

>>>>          {

>>>> -        tree s = constexpr_fn_retval (tsi_stmt (i));

>>>> +        tree s = constexpr_fn_retval (stmt);

>>>>            if (s == error_mark_node)

>>>>              return error_mark_node;

>>>>            else if (s == NULL_TREE)

>>>> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx

>>>> *ctx, tree t,

>>>>                 bool *non_constant_p, bool *overflow_p,

>>>>                 tree *jump_target)

>>>>    {

>>>> -  tree_stmt_iterator i;

>>>>      tree local_target;

>>>>      /* In a statement-expression we want to return the last value.

>>>>         For empty statement expression return void_node.  */

>>>> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx

>>>> *ctx, tree t,

>>>>          local_target = NULL_TREE;

>>>>          jump_target = &local_target;

>>>>        }

>>>> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>> +  for (tree stmt : tsi_range (t))

>>>>        {

>>>> -      tree stmt = tsi_stmt (i);

>>>>          /* We've found a continue, so skip everything until we reach

>>>>         the label its jumping to.  */

>>>>          if (continues (jump_target))

>>>> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool

>>>> want_rval, bool strict, bool now,

>>>>          }

>>>>        case STATEMENT_LIST:

>>>> -      {

>>>> -    tree_stmt_iterator i;

>>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>> -      {

>>>> -        if (!RECUR (tsi_stmt (i), any))

>>>> -          return false;

>>>> -      }

>>>> -    return true;

>>>> -      }

>>>> -      break;

>>>> +      for (tree stmt : tsi_range (t))

>>>> +    if (!RECUR (stmt, any))

>>>> +      return false;

>>>> +      return true;

>>>>        case MODIFY_EXPR:

>>>>          if (cxx_dialect < cxx14)

>>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

>>>> index dbd703a67cc..9b498f9d0b4 100644

>>>> --- a/gcc/cp/coroutines.cc

>>>> +++ b/gcc/cp/coroutines.cc

>>>> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int

>>>> *do_subtree, void *d)

>>>>        return NULL_TREE; /* Just process the sub-trees.  */

>>>>      else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>>>        {

>>>> -      tree_stmt_iterator i;

>>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>>>> +      for (tree &s : tsi_range (*stmt))

>>>>        {

>>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

>>>> +      res = cp_walk_tree (&s, await_statement_expander,

>>>>                      d, NULL);

>>>>          if (res)

>>>>            return res;

>>>> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int

>>>> *do_subtree, void *d)

>>>>        }

>>>>      else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>>>        {

>>>> -      tree_stmt_iterator i;

>>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>>>> +      for (tree &s : tsi_range (*stmt))

>>>>        {

>>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

>>>> +      res = cp_walk_tree (&s, await_statement_walker,

>>>>                      d, NULL);

>>>>          if (res)

>>>>            return res;

>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

>>>> index 02c19f55548..f0fb0144706 100644

>>>> --- a/gcc/cp/module.cc

>>>> +++ b/gcc/cp/module.cc

>>>> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

>>>>          break;

>>>>        case STATEMENT_LIST:

>>>> -      for (tree_stmt_iterator iter = tsi_start (t);

>>>> -       !tsi_end_p (iter); tsi_next (&iter))

>>>> -    if (tree stmt = tsi_stmt (iter))

>>>> +      for (tree stmt : tsi_range (t))

>>>> +    if (stmt)

>>>>          WT (stmt);

>>>>          WT (NULL_TREE);

>>>>          break;

>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

>>>> index 116bdd2e42a..ad140cfd586 100644

>>>> --- a/gcc/cp/pt.c

>>>> +++ b/gcc/cp/pt.c

>>>> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t

>>>> complain, tree in_decl,

>>>>        {

>>>>        case STATEMENT_LIST:

>>>>          {

>>>> -    tree_stmt_iterator i;

>>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>> -      RECUR (tsi_stmt (i));

>>>> +    for (tree stmt : tsi_range (t))

>>>> +      RECUR (stmt);

>>>>        break;

>>>>          }

>>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

>>>> index 6224f49f189..2912efad9be 100644

>>>> --- a/gcc/cp/semantics.c

>>>> +++ b/gcc/cp/semantics.c

>>>> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

>>>>          set_cleanup_locs (CLEANUP_BODY (stmts), loc);

>>>>        }

>>>>      else if (TREE_CODE (stmts) == STATEMENT_LIST)

>>>> -    for (tree_stmt_iterator i = tsi_start (stmts);

>>>> -     !tsi_end_p (i); tsi_next (&i))

>>>> -      set_cleanup_locs (tsi_stmt (i), loc);

>>>> +    for (tree stmt : tsi_range (stmts))

>>>> +      set_cleanup_locs (stmt, loc);

>>>>    }

>>>>    /* Finish a scope.  */

>>>>

>>>> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

>>>> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

>>>>

>>>

>>

>
Dragan Mladjenovic via Gcc-patches May 26, 2021, 1:56 p.m. | #6
Ping.

On 5/17/21 1:58 PM, Jason Merrill wrote:
> On 5/17/21 3:56 AM, Richard Biener wrote:

>> On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches

>> <gcc-patches@gcc.gnu.org> wrote:

>>>

>>> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:

>>>> Ping.

>>>>

>>>> On 5/1/21 12:29 PM, Jason Merrill wrote:

>>>>> Like my recent patch to add ovl_range and lkp_range in the C++ 

>>>>> front end,

>>>>> this patch adds the tsi_range adaptor for using C++11 range-based

>>>>> 'for' with

>>>>> a STATEMENT_LIST, e.g.

>>>>>

>>>>>     for (tree stmt : tsi_range (stmt_list)) { ... }

>>>>>

>>>>> This also involves adding some operators to tree_stmt_iterator that 

>>>>> are

>>>>> needed for range-for iterators, and should also be useful in code that

>>>>> uses

>>>>> the iterators directly.

>>>>>

>>>>> The patch updates the suitable loops in the C++ front end, but does 

>>>>> not

>>>>> touch any loops elsewhere in the compiler.

>>>

>>> I like the modernization of the loops.

>>

>> The only worry I have (and why I stopped looking at range-for) is that

>> this adds another style of looping over stmts without opening the

>> possibility to remove another or even unify all of them.  That's because

>> range-for isn't powerful enough w/o jumping through hoops and/or

>> we cannot use what appearantly ranges<> was intended for (fix

>> this limitation).

> 

> The range-for enabled by my patch simplifies the common case of simple 

> iteration over elements; that seems worth doing to me even if it doesn't 

> replace all loops.  Just as FOR_EACH_VEC_ELT isn't suitable for all 

> loops over a vector.

> 

>> That said, if some C++ literate could see if for example

>> what gimple-iterator.h provides can be completely modernized

>> then that would be great of course.

>>

>> There's stuff like reverse iteration

> 

> This is typically done with the reverse_iterator<> adaptor, which we 

> could get from <iterator> or duplicate.  I didn't see enough reverse 

> iterations to make it seem worth the bother.

> 

>> iteration skipping debug stmts,

> 

> There you can move the condition into the loop:

> 

> if (gimple_code (stmt) == GIMPLE_DEBUG)

>   continue;

> 

>> compares of iterators like gsi_one_before_end_p, etc.

> 

> Certainly anything where you want to mess with the iterators directly 

> doesn't really translate to range-for.

> 

>> Given my failed tries (but I'm a C++ illiterate) my TODO list now

>> only contains turning the iterators into STL style ones, thus

>> gsi_stmt (it) -> *it, gsi_next (&it) -> ++it, etc. - but even

>> it != end_p looks a bit awkward there.

> 

> Well, it < end_val is pretty typical for loops involving integer 

> iterators.  But you don't have to use that style if you'd rather not. 

> You could continue to use gsi_end_p, or just *it, since we know that *it 

> is NULL at the end of the sequence.

> 

>>> I can't find anything terribly wrong with the iterator but let me

>>> at least pick on some nits ;)

>>>

>>>>>

>>>>> gcc/ChangeLog:

>>>>>

>>>>>      * tree-iterator.h (struct tree_stmt_iterator): Add operator++,

>>>>>      operator--, operator*, operator==, and operator!=.

>>>>>      (class tsi_range): New.

>>>>>

>>>>> gcc/cp/ChangeLog:

>>>>>

>>>>>      * constexpr.c (build_data_member_initialization): Use tsi_range.

>>>>>      (build_constexpr_constructor_member_initializers): Likewise.

>>>>>      (constexpr_fn_retval, cxx_eval_statement_list): Likewise.

>>>>>      (potential_constant_expression_1): Likewise.

>>>>>      * coroutines.cc (await_statement_expander): Likewise.

>>>>>      (await_statement_walker): Likewise.

>>>>>      * module.cc (trees_out::core_vals): Likewise.

>>>>>      * pt.c (tsubst_expr): Likewise.

>>>>>      * semantics.c (set_cleanup_locs): Likewise.

>>>>> ---

>>>>>    gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

>>>>>    gcc/cp/constexpr.c   | 42 

>>>>> ++++++++++++++----------------------------

>>>>>    gcc/cp/coroutines.cc | 10 ++++------

>>>>>    gcc/cp/module.cc     |  5 ++---

>>>>>    gcc/cp/pt.c          |  5 ++---

>>>>>    gcc/cp/semantics.c   |  5 ++---

>>>>>    6 files changed, 47 insertions(+), 48 deletions(-)

>>>>>

>>>>> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

>>>>> index 076fff8644c..f57456bb473 100644

>>>>> --- a/gcc/tree-iterator.h

>>>>> +++ b/gcc/tree-iterator.h

>>>>> @@ -1,4 +1,4 @@

>>>>> -/* Iterator routines for manipulating GENERIC tree statement list.

>>>>> +/* Iterator routines for manipulating GENERIC tree statement list.

>>>>> -*- C++ -*-

>>>>>       Copyright (C) 2003-2021 Free Software Foundation, Inc.

>>>>>       Contributed by Andrew MacLeod  <amacleod@redhat.com>

>>>>> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

>>>>>    struct tree_stmt_iterator {

>>>>>      struct tree_statement_list_node *ptr;

>>>>>      tree container;

>>>

>>> I assume the absence of ctors is intentional.  If so, I suggest

>>> to add a comment explaing why.  Otherwise, I would provide one

>>> (or as many as needed).

>>>

>>>>> +

>>>>> +  bool operator== (tree_stmt_iterator b) const

>>>>> +    { return b.ptr == ptr && b.container == container; }

>>>>> +  bool operator!= (tree_stmt_iterator b) const { return !(*this == 

>>>>> b); }

>>>>> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return 

>>>>> *this; }

>>>>> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return 

>>>>> *this; }

>>>

>>> I would suggest to add postincrement and postdecrement.

>>>

>>>>> +  tree &operator* () { return ptr->stmt; }

>>>

>>> Given the pervasive lack of const-safety in GCC and the by-value

>>> semantics of the iterator this probably isn't worth it but maybe

>>> add a const overload.  operator-> would probably never be used.

>>>

>>>>>    };

>>>>>    static inline tree_stmt_iterator

>>>>> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

>>>>>    static inline void

>>>>>    tsi_next (tree_stmt_iterator *i)

>>>>>    {

>>>>> -  i->ptr = i->ptr->next;

>>>>> +  ++(*i);

>>>>>    }

>>>>>    static inline void

>>>>>    tsi_prev (tree_stmt_iterator *i)

>>>>>    {

>>>>> -  i->ptr = i->ptr->prev;

>>>>> +  --(*i);

>>>>>    }

>>>>>    static inline tree *

>>>>>    tsi_stmt_ptr (tree_stmt_iterator i)

>>>>>    {

>>>>> -  return &i.ptr->stmt;

>>>>> +  return &(*i);

>>>>>    }

>>>>>    static inline tree

>>>>>    tsi_stmt (tree_stmt_iterator i)

>>>>>    {

>>>>> -  return i.ptr->stmt;

>>>>> +  return *i;

>>>>>    }

>>>>> +/* Make tree_stmt_iterator work as a C++ range, e.g.

>>>>> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

>>>>> +class tsi_range

>>>>> +{

>>>>> +  tree t;

>>>>> + public:

>>>>> +  tsi_range (tree t): t(t) { }

>>>>> +  tree_stmt_iterator begin() { return tsi_start (t); }

>>>>> +  tree_stmt_iterator end() { return { nullptr, t }; }

>>>

>>> Those member functions could be made const.

>>>

>>> Martin

>>>

>>>>> +};

>>>>> +

>>>>>    enum tsi_iterator_update

>>>>>    {

>>>>>      TSI_NEW_STMT,        /* Only valid when single statement is 

>>>>> added,

>>>>> move

>>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

>>>>> index 9481a5bfd3c..260b0122f59 100644

>>>>> --- a/gcc/cp/constexpr.c

>>>>> +++ b/gcc/cp/constexpr.c

>>>>> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t,

>>>>> vec<constructor_elt, va_gc> **vec)

>>>>>        return false;

>>>>>      if (TREE_CODE (t) == STATEMENT_LIST)

>>>>>        {

>>>>> -      tree_stmt_iterator i;

>>>>> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>>> -    {

>>>>> -      if (! build_data_member_initialization (tsi_stmt (i), vec))

>>>>> -        return false;

>>>>> -    }

>>>>> +      for (tree stmt : tsi_range (t))

>>>>> +    if (! build_data_member_initialization (stmt, vec))

>>>>> +      return false;

>>>>>          return true;

>>>>>        }

>>>>>      if (TREE_CODE (t) == CLEANUP_STMT)

>>>>> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers

>>>>> (tree type, tree body)

>>>>>        break;

>>>>>          case STATEMENT_LIST:

>>>>> -    for (tree_stmt_iterator i = tsi_start (body);

>>>>> -         !tsi_end_p (i); tsi_next (&i))

>>>>> +    for (tree stmt : tsi_range (body))

>>>>>          {

>>>>> -        body = tsi_stmt (i);

>>>>> +        body = stmt;

>>>>>            if (TREE_CODE (body) == BIND_EXPR)

>>>>>              break;

>>>>>          }

>>>>> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers

>>>>> (tree type, tree body)

>>>>>        }

>>>>>      else if (TREE_CODE (body) == STATEMENT_LIST)

>>>>>        {

>>>>> -      tree_stmt_iterator i;

>>>>> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>>>>> +      for (tree stmt : tsi_range (body))

>>>>>        {

>>>>> -      ok = build_data_member_initialization (tsi_stmt (i), &vec);

>>>>> +      ok = build_data_member_initialization (stmt, &vec);

>>>>>          if (!ok)

>>>>>            break;

>>>>>        }

>>>>> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

>>>>>        {

>>>>>        case STATEMENT_LIST:

>>>>>          {

>>>>> -    tree_stmt_iterator i;

>>>>>        tree expr = NULL_TREE;

>>>>> -    for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

>>>>> +    for (tree stmt : tsi_range (body))

>>>>>          {

>>>>> -        tree s = constexpr_fn_retval (tsi_stmt (i));

>>>>> +        tree s = constexpr_fn_retval (stmt);

>>>>>            if (s == error_mark_node)

>>>>>              return error_mark_node;

>>>>>            else if (s == NULL_TREE)

>>>>> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx

>>>>> *ctx, tree t,

>>>>>                 bool *non_constant_p, bool *overflow_p,

>>>>>                 tree *jump_target)

>>>>>    {

>>>>> -  tree_stmt_iterator i;

>>>>>      tree local_target;

>>>>>      /* In a statement-expression we want to return the last value.

>>>>>         For empty statement expression return void_node.  */

>>>>> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx

>>>>> *ctx, tree t,

>>>>>          local_target = NULL_TREE;

>>>>>          jump_target = &local_target;

>>>>>        }

>>>>> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>>> +  for (tree stmt : tsi_range (t))

>>>>>        {

>>>>> -      tree stmt = tsi_stmt (i);

>>>>>          /* We've found a continue, so skip everything until we reach

>>>>>         the label its jumping to.  */

>>>>>          if (continues (jump_target))

>>>>> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool

>>>>> want_rval, bool strict, bool now,

>>>>>          }

>>>>>        case STATEMENT_LIST:

>>>>> -      {

>>>>> -    tree_stmt_iterator i;

>>>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>>> -      {

>>>>> -        if (!RECUR (tsi_stmt (i), any))

>>>>> -          return false;

>>>>> -      }

>>>>> -    return true;

>>>>> -      }

>>>>> -      break;

>>>>> +      for (tree stmt : tsi_range (t))

>>>>> +    if (!RECUR (stmt, any))

>>>>> +      return false;

>>>>> +      return true;

>>>>>        case MODIFY_EXPR:

>>>>>          if (cxx_dialect < cxx14)

>>>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

>>>>> index dbd703a67cc..9b498f9d0b4 100644

>>>>> --- a/gcc/cp/coroutines.cc

>>>>> +++ b/gcc/cp/coroutines.cc

>>>>> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int

>>>>> *do_subtree, void *d)

>>>>>        return NULL_TREE; /* Just process the sub-trees.  */

>>>>>      else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>>>>        {

>>>>> -      tree_stmt_iterator i;

>>>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>>>>> +      for (tree &s : tsi_range (*stmt))

>>>>>        {

>>>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

>>>>> +      res = cp_walk_tree (&s, await_statement_expander,

>>>>>                      d, NULL);

>>>>>          if (res)

>>>>>            return res;

>>>>> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int

>>>>> *do_subtree, void *d)

>>>>>        }

>>>>>      else if (TREE_CODE (*stmt) == STATEMENT_LIST)

>>>>>        {

>>>>> -      tree_stmt_iterator i;

>>>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

>>>>> +      for (tree &s : tsi_range (*stmt))

>>>>>        {

>>>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

>>>>> +      res = cp_walk_tree (&s, await_statement_walker,

>>>>>                      d, NULL);

>>>>>          if (res)

>>>>>            return res;

>>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

>>>>> index 02c19f55548..f0fb0144706 100644

>>>>> --- a/gcc/cp/module.cc

>>>>> +++ b/gcc/cp/module.cc

>>>>> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

>>>>>          break;

>>>>>        case STATEMENT_LIST:

>>>>> -      for (tree_stmt_iterator iter = tsi_start (t);

>>>>> -       !tsi_end_p (iter); tsi_next (&iter))

>>>>> -    if (tree stmt = tsi_stmt (iter))

>>>>> +      for (tree stmt : tsi_range (t))

>>>>> +    if (stmt)

>>>>>          WT (stmt);

>>>>>          WT (NULL_TREE);

>>>>>          break;

>>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

>>>>> index 116bdd2e42a..ad140cfd586 100644

>>>>> --- a/gcc/cp/pt.c

>>>>> +++ b/gcc/cp/pt.c

>>>>> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t

>>>>> complain, tree in_decl,

>>>>>        {

>>>>>        case STATEMENT_LIST:

>>>>>          {

>>>>> -    tree_stmt_iterator i;

>>>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

>>>>> -      RECUR (tsi_stmt (i));

>>>>> +    for (tree stmt : tsi_range (t))

>>>>> +      RECUR (stmt);

>>>>>        break;

>>>>>          }

>>>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

>>>>> index 6224f49f189..2912efad9be 100644

>>>>> --- a/gcc/cp/semantics.c

>>>>> +++ b/gcc/cp/semantics.c

>>>>> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

>>>>>          set_cleanup_locs (CLEANUP_BODY (stmts), loc);

>>>>>        }

>>>>>      else if (TREE_CODE (stmts) == STATEMENT_LIST)

>>>>> -    for (tree_stmt_iterator i = tsi_start (stmts);

>>>>> -     !tsi_end_p (i); tsi_next (&i))

>>>>> -      set_cleanup_locs (tsi_stmt (i), loc);

>>>>> +    for (tree stmt : tsi_range (stmts))

>>>>> +      set_cleanup_locs (stmt, loc);

>>>>>    }

>>>>>    /* Finish a scope.  */

>>>>>

>>>>> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

>>>>> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

>>>>>

>>>>

>>>

>>

>
Dragan Mladjenovic via Gcc-patches May 28, 2021, 6:53 a.m. | #7
On Wed, May 26, 2021 at 3:56 PM Jason Merrill <jason@redhat.com> wrote:
>

> Ping.


The non-C++ parts are OK.

Richard.

> On 5/17/21 1:58 PM, Jason Merrill wrote:

> > On 5/17/21 3:56 AM, Richard Biener wrote:

> >> On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches

> >> <gcc-patches@gcc.gnu.org> wrote:

> >>>

> >>> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:

> >>>> Ping.

> >>>>

> >>>> On 5/1/21 12:29 PM, Jason Merrill wrote:

> >>>>> Like my recent patch to add ovl_range and lkp_range in the C++

> >>>>> front end,

> >>>>> this patch adds the tsi_range adaptor for using C++11 range-based

> >>>>> 'for' with

> >>>>> a STATEMENT_LIST, e.g.

> >>>>>

> >>>>>     for (tree stmt : tsi_range (stmt_list)) { ... }

> >>>>>

> >>>>> This also involves adding some operators to tree_stmt_iterator that

> >>>>> are

> >>>>> needed for range-for iterators, and should also be useful in code that

> >>>>> uses

> >>>>> the iterators directly.

> >>>>>

> >>>>> The patch updates the suitable loops in the C++ front end, but does

> >>>>> not

> >>>>> touch any loops elsewhere in the compiler.

> >>>

> >>> I like the modernization of the loops.

> >>

> >> The only worry I have (and why I stopped looking at range-for) is that

> >> this adds another style of looping over stmts without opening the

> >> possibility to remove another or even unify all of them.  That's because

> >> range-for isn't powerful enough w/o jumping through hoops and/or

> >> we cannot use what appearantly ranges<> was intended for (fix

> >> this limitation).

> >

> > The range-for enabled by my patch simplifies the common case of simple

> > iteration over elements; that seems worth doing to me even if it doesn't

> > replace all loops.  Just as FOR_EACH_VEC_ELT isn't suitable for all

> > loops over a vector.

> >

> >> That said, if some C++ literate could see if for example

> >> what gimple-iterator.h provides can be completely modernized

> >> then that would be great of course.

> >>

> >> There's stuff like reverse iteration

> >

> > This is typically done with the reverse_iterator<> adaptor, which we

> > could get from <iterator> or duplicate.  I didn't see enough reverse

> > iterations to make it seem worth the bother.

> >

> >> iteration skipping debug stmts,

> >

> > There you can move the condition into the loop:

> >

> > if (gimple_code (stmt) == GIMPLE_DEBUG)

> >   continue;

> >

> >> compares of iterators like gsi_one_before_end_p, etc.

> >

> > Certainly anything where you want to mess with the iterators directly

> > doesn't really translate to range-for.

> >

> >> Given my failed tries (but I'm a C++ illiterate) my TODO list now

> >> only contains turning the iterators into STL style ones, thus

> >> gsi_stmt (it) -> *it, gsi_next (&it) -> ++it, etc. - but even

> >> it != end_p looks a bit awkward there.

> >

> > Well, it < end_val is pretty typical for loops involving integer

> > iterators.  But you don't have to use that style if you'd rather not.

> > You could continue to use gsi_end_p, or just *it, since we know that *it

> > is NULL at the end of the sequence.

> >

> >>> I can't find anything terribly wrong with the iterator but let me

> >>> at least pick on some nits ;)

> >>>

> >>>>>

> >>>>> gcc/ChangeLog:

> >>>>>

> >>>>>      * tree-iterator.h (struct tree_stmt_iterator): Add operator++,

> >>>>>      operator--, operator*, operator==, and operator!=.

> >>>>>      (class tsi_range): New.

> >>>>>

> >>>>> gcc/cp/ChangeLog:

> >>>>>

> >>>>>      * constexpr.c (build_data_member_initialization): Use tsi_range.

> >>>>>      (build_constexpr_constructor_member_initializers): Likewise.

> >>>>>      (constexpr_fn_retval, cxx_eval_statement_list): Likewise.

> >>>>>      (potential_constant_expression_1): Likewise.

> >>>>>      * coroutines.cc (await_statement_expander): Likewise.

> >>>>>      (await_statement_walker): Likewise.

> >>>>>      * module.cc (trees_out::core_vals): Likewise.

> >>>>>      * pt.c (tsubst_expr): Likewise.

> >>>>>      * semantics.c (set_cleanup_locs): Likewise.

> >>>>> ---

> >>>>>    gcc/tree-iterator.h  | 28 +++++++++++++++++++++++-----

> >>>>>    gcc/cp/constexpr.c   | 42

> >>>>> ++++++++++++++----------------------------

> >>>>>    gcc/cp/coroutines.cc | 10 ++++------

> >>>>>    gcc/cp/module.cc     |  5 ++---

> >>>>>    gcc/cp/pt.c          |  5 ++---

> >>>>>    gcc/cp/semantics.c   |  5 ++---

> >>>>>    6 files changed, 47 insertions(+), 48 deletions(-)

> >>>>>

> >>>>> diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h

> >>>>> index 076fff8644c..f57456bb473 100644

> >>>>> --- a/gcc/tree-iterator.h

> >>>>> +++ b/gcc/tree-iterator.h

> >>>>> @@ -1,4 +1,4 @@

> >>>>> -/* Iterator routines for manipulating GENERIC tree statement list.

> >>>>> +/* Iterator routines for manipulating GENERIC tree statement list.

> >>>>> -*- C++ -*-

> >>>>>       Copyright (C) 2003-2021 Free Software Foundation, Inc.

> >>>>>       Contributed by Andrew MacLeod  <amacleod@redhat.com>

> >>>>> @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see

> >>>>>    struct tree_stmt_iterator {

> >>>>>      struct tree_statement_list_node *ptr;

> >>>>>      tree container;

> >>>

> >>> I assume the absence of ctors is intentional.  If so, I suggest

> >>> to add a comment explaing why.  Otherwise, I would provide one

> >>> (or as many as needed).

> >>>

> >>>>> +

> >>>>> +  bool operator== (tree_stmt_iterator b) const

> >>>>> +    { return b.ptr == ptr && b.container == container; }

> >>>>> +  bool operator!= (tree_stmt_iterator b) const { return !(*this ==

> >>>>> b); }

> >>>>> +  tree_stmt_iterator &operator++ () { ptr = ptr->next; return

> >>>>> *this; }

> >>>>> +  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return

> >>>>> *this; }

> >>>

> >>> I would suggest to add postincrement and postdecrement.

> >>>

> >>>>> +  tree &operator* () { return ptr->stmt; }

> >>>

> >>> Given the pervasive lack of const-safety in GCC and the by-value

> >>> semantics of the iterator this probably isn't worth it but maybe

> >>> add a const overload.  operator-> would probably never be used.

> >>>

> >>>>>    };

> >>>>>    static inline tree_stmt_iterator

> >>>>> @@ -71,27 +78,38 @@ tsi_one_before_end_p (tree_stmt_iterator i)

> >>>>>    static inline void

> >>>>>    tsi_next (tree_stmt_iterator *i)

> >>>>>    {

> >>>>> -  i->ptr = i->ptr->next;

> >>>>> +  ++(*i);

> >>>>>    }

> >>>>>    static inline void

> >>>>>    tsi_prev (tree_stmt_iterator *i)

> >>>>>    {

> >>>>> -  i->ptr = i->ptr->prev;

> >>>>> +  --(*i);

> >>>>>    }

> >>>>>    static inline tree *

> >>>>>    tsi_stmt_ptr (tree_stmt_iterator i)

> >>>>>    {

> >>>>> -  return &i.ptr->stmt;

> >>>>> +  return &(*i);

> >>>>>    }

> >>>>>    static inline tree

> >>>>>    tsi_stmt (tree_stmt_iterator i)

> >>>>>    {

> >>>>> -  return i.ptr->stmt;

> >>>>> +  return *i;

> >>>>>    }

> >>>>> +/* Make tree_stmt_iterator work as a C++ range, e.g.

> >>>>> +   for (tree stmt : tsi_range (stmt_list)) { ... }  */

> >>>>> +class tsi_range

> >>>>> +{

> >>>>> +  tree t;

> >>>>> + public:

> >>>>> +  tsi_range (tree t): t(t) { }

> >>>>> +  tree_stmt_iterator begin() { return tsi_start (t); }

> >>>>> +  tree_stmt_iterator end() { return { nullptr, t }; }

> >>>

> >>> Those member functions could be made const.

> >>>

> >>> Martin

> >>>

> >>>>> +};

> >>>>> +

> >>>>>    enum tsi_iterator_update

> >>>>>    {

> >>>>>      TSI_NEW_STMT,        /* Only valid when single statement is

> >>>>> added,

> >>>>> move

> >>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c

> >>>>> index 9481a5bfd3c..260b0122f59 100644

> >>>>> --- a/gcc/cp/constexpr.c

> >>>>> +++ b/gcc/cp/constexpr.c

> >>>>> @@ -330,12 +330,9 @@ build_data_member_initialization (tree t,

> >>>>> vec<constructor_elt, va_gc> **vec)

> >>>>>        return false;

> >>>>>      if (TREE_CODE (t) == STATEMENT_LIST)

> >>>>>        {

> >>>>> -      tree_stmt_iterator i;

> >>>>> -      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >>>>> -    {

> >>>>> -      if (! build_data_member_initialization (tsi_stmt (i), vec))

> >>>>> -        return false;

> >>>>> -    }

> >>>>> +      for (tree stmt : tsi_range (t))

> >>>>> +    if (! build_data_member_initialization (stmt, vec))

> >>>>> +      return false;

> >>>>>          return true;

> >>>>>        }

> >>>>>      if (TREE_CODE (t) == CLEANUP_STMT)

> >>>>> @@ -577,10 +574,9 @@ build_constexpr_constructor_member_initializers

> >>>>> (tree type, tree body)

> >>>>>        break;

> >>>>>          case STATEMENT_LIST:

> >>>>> -    for (tree_stmt_iterator i = tsi_start (body);

> >>>>> -         !tsi_end_p (i); tsi_next (&i))

> >>>>> +    for (tree stmt : tsi_range (body))

> >>>>>          {

> >>>>> -        body = tsi_stmt (i);

> >>>>> +        body = stmt;

> >>>>>            if (TREE_CODE (body) == BIND_EXPR)

> >>>>>              break;

> >>>>>          }

> >>>>> @@ -617,10 +613,9 @@ build_constexpr_constructor_member_initializers

> >>>>> (tree type, tree body)

> >>>>>        }

> >>>>>      else if (TREE_CODE (body) == STATEMENT_LIST)

> >>>>>        {

> >>>>> -      tree_stmt_iterator i;

> >>>>> -      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

> >>>>> +      for (tree stmt : tsi_range (body))

> >>>>>        {

> >>>>> -      ok = build_data_member_initialization (tsi_stmt (i), &vec);

> >>>>> +      ok = build_data_member_initialization (stmt, &vec);

> >>>>>          if (!ok)

> >>>>>            break;

> >>>>>        }

> >>>>> @@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)

> >>>>>        {

> >>>>>        case STATEMENT_LIST:

> >>>>>          {

> >>>>> -    tree_stmt_iterator i;

> >>>>>        tree expr = NULL_TREE;

> >>>>> -    for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))

> >>>>> +    for (tree stmt : tsi_range (body))

> >>>>>          {

> >>>>> -        tree s = constexpr_fn_retval (tsi_stmt (i));

> >>>>> +        tree s = constexpr_fn_retval (stmt);

> >>>>>            if (s == error_mark_node)

> >>>>>              return error_mark_node;

> >>>>>            else if (s == NULL_TREE)

> >>>>> @@ -5772,7 +5766,6 @@ cxx_eval_statement_list (const constexpr_ctx

> >>>>> *ctx, tree t,

> >>>>>                 bool *non_constant_p, bool *overflow_p,

> >>>>>                 tree *jump_target)

> >>>>>    {

> >>>>> -  tree_stmt_iterator i;

> >>>>>      tree local_target;

> >>>>>      /* In a statement-expression we want to return the last value.

> >>>>>         For empty statement expression return void_node.  */

> >>>>> @@ -5782,9 +5775,8 @@ cxx_eval_statement_list (const constexpr_ctx

> >>>>> *ctx, tree t,

> >>>>>          local_target = NULL_TREE;

> >>>>>          jump_target = &local_target;

> >>>>>        }

> >>>>> -  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >>>>> +  for (tree stmt : tsi_range (t))

> >>>>>        {

> >>>>> -      tree stmt = tsi_stmt (i);

> >>>>>          /* We've found a continue, so skip everything until we reach

> >>>>>         the label its jumping to.  */

> >>>>>          if (continues (jump_target))

> >>>>> @@ -8283,16 +8275,10 @@ potential_constant_expression_1 (tree t, bool

> >>>>> want_rval, bool strict, bool now,

> >>>>>          }

> >>>>>        case STATEMENT_LIST:

> >>>>> -      {

> >>>>> -    tree_stmt_iterator i;

> >>>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >>>>> -      {

> >>>>> -        if (!RECUR (tsi_stmt (i), any))

> >>>>> -          return false;

> >>>>> -      }

> >>>>> -    return true;

> >>>>> -      }

> >>>>> -      break;

> >>>>> +      for (tree stmt : tsi_range (t))

> >>>>> +    if (!RECUR (stmt, any))

> >>>>> +      return false;

> >>>>> +      return true;

> >>>>>        case MODIFY_EXPR:

> >>>>>          if (cxx_dialect < cxx14)

> >>>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> >>>>> index dbd703a67cc..9b498f9d0b4 100644

> >>>>> --- a/gcc/cp/coroutines.cc

> >>>>> +++ b/gcc/cp/coroutines.cc

> >>>>> @@ -1771,10 +1771,9 @@ await_statement_expander (tree *stmt, int

> >>>>> *do_subtree, void *d)

> >>>>>        return NULL_TREE; /* Just process the sub-trees.  */

> >>>>>      else if (TREE_CODE (*stmt) == STATEMENT_LIST)

> >>>>>        {

> >>>>> -      tree_stmt_iterator i;

> >>>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

> >>>>> +      for (tree &s : tsi_range (*stmt))

> >>>>>        {

> >>>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,

> >>>>> +      res = cp_walk_tree (&s, await_statement_expander,

> >>>>>                      d, NULL);

> >>>>>          if (res)

> >>>>>            return res;

> >>>>> @@ -3523,10 +3522,9 @@ await_statement_walker (tree *stmt, int

> >>>>> *do_subtree, void *d)

> >>>>>        }

> >>>>>      else if (TREE_CODE (*stmt) == STATEMENT_LIST)

> >>>>>        {

> >>>>> -      tree_stmt_iterator i;

> >>>>> -      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))

> >>>>> +      for (tree &s : tsi_range (*stmt))

> >>>>>        {

> >>>>> -      res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,

> >>>>> +      res = cp_walk_tree (&s, await_statement_walker,

> >>>>>                      d, NULL);

> >>>>>          if (res)

> >>>>>            return res;

> >>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc

> >>>>> index 02c19f55548..f0fb0144706 100644

> >>>>> --- a/gcc/cp/module.cc

> >>>>> +++ b/gcc/cp/module.cc

> >>>>> @@ -6094,9 +6094,8 @@ trees_out::core_vals (tree t)

> >>>>>          break;

> >>>>>        case STATEMENT_LIST:

> >>>>> -      for (tree_stmt_iterator iter = tsi_start (t);

> >>>>> -       !tsi_end_p (iter); tsi_next (&iter))

> >>>>> -    if (tree stmt = tsi_stmt (iter))

> >>>>> +      for (tree stmt : tsi_range (t))

> >>>>> +    if (stmt)

> >>>>>          WT (stmt);

> >>>>>          WT (NULL_TREE);

> >>>>>          break;

> >>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

> >>>>> index 116bdd2e42a..ad140cfd586 100644

> >>>>> --- a/gcc/cp/pt.c

> >>>>> +++ b/gcc/cp/pt.c

> >>>>> @@ -18234,9 +18234,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t

> >>>>> complain, tree in_decl,

> >>>>>        {

> >>>>>        case STATEMENT_LIST:

> >>>>>          {

> >>>>> -    tree_stmt_iterator i;

> >>>>> -    for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))

> >>>>> -      RECUR (tsi_stmt (i));

> >>>>> +    for (tree stmt : tsi_range (t))

> >>>>> +      RECUR (stmt);

> >>>>>        break;

> >>>>>          }

> >>>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c

> >>>>> index 6224f49f189..2912efad9be 100644

> >>>>> --- a/gcc/cp/semantics.c

> >>>>> +++ b/gcc/cp/semantics.c

> >>>>> @@ -613,9 +613,8 @@ set_cleanup_locs (tree stmts, location_t loc)

> >>>>>          set_cleanup_locs (CLEANUP_BODY (stmts), loc);

> >>>>>        }

> >>>>>      else if (TREE_CODE (stmts) == STATEMENT_LIST)

> >>>>> -    for (tree_stmt_iterator i = tsi_start (stmts);

> >>>>> -     !tsi_end_p (i); tsi_next (&i))

> >>>>> -      set_cleanup_locs (tsi_stmt (i), loc);

> >>>>> +    for (tree stmt : tsi_range (stmts))

> >>>>> +      set_cleanup_locs (stmt, loc);

> >>>>>    }

> >>>>>    /* Finish a scope.  */

> >>>>>

> >>>>> base-commit: 3c65858787dc52b65b26fa7018587c01510f442c

> >>>>> prerequisite-patch-id: 7ce9dff1f7d449f4a13fb882bf7b1a962a56de95

> >>>>>

> >>>>

> >>>

> >>

> >

>

Patch

diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h
index 076fff8644c..f57456bb473 100644
--- a/gcc/tree-iterator.h
+++ b/gcc/tree-iterator.h
@@ -1,4 +1,4 @@ 
-/* Iterator routines for manipulating GENERIC tree statement list.
+/* Iterator routines for manipulating GENERIC tree statement list. -*- C++ -*-
    Copyright (C) 2003-2021 Free Software Foundation, Inc.
    Contributed by Andrew MacLeod  <amacleod@redhat.com>
 
@@ -32,6 +32,13 @@  along with GCC; see the file COPYING3.  If not see
 struct tree_stmt_iterator {
   struct tree_statement_list_node *ptr;
   tree container;
+
+  bool operator== (tree_stmt_iterator b) const
+    { return b.ptr == ptr && b.container == container; }
+  bool operator!= (tree_stmt_iterator b) const { return !(*this == b); }
+  tree_stmt_iterator &operator++ () { ptr = ptr->next; return *this; }
+  tree_stmt_iterator &operator-- () { ptr = ptr->prev; return *this; }
+  tree &operator* () { return ptr->stmt; }
 };
 
 static inline tree_stmt_iterator
@@ -71,27 +78,38 @@  tsi_one_before_end_p (tree_stmt_iterator i)
 static inline void
 tsi_next (tree_stmt_iterator *i)
 {
-  i->ptr = i->ptr->next;
+  ++(*i);
 }
 
 static inline void
 tsi_prev (tree_stmt_iterator *i)
 {
-  i->ptr = i->ptr->prev;
+  --(*i);
 }
 
 static inline tree *
 tsi_stmt_ptr (tree_stmt_iterator i)
 {
-  return &i.ptr->stmt;
+  return &(*i);
 }
 
 static inline tree
 tsi_stmt (tree_stmt_iterator i)
 {
-  return i.ptr->stmt;
+  return *i;
 }
 
+/* Make tree_stmt_iterator work as a C++ range, e.g.
+   for (tree stmt : tsi_range (stmt_list)) { ... }  */
+class tsi_range
+{
+  tree t;
+ public:
+  tsi_range (tree t): t(t) { }
+  tree_stmt_iterator begin() { return tsi_start (t); }
+  tree_stmt_iterator end() { return { nullptr, t }; }
+};
+
 enum tsi_iterator_update
 {
   TSI_NEW_STMT,		/* Only valid when single statement is added, move
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 9481a5bfd3c..260b0122f59 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -330,12 +330,9 @@  build_data_member_initialization (tree t, vec<constructor_elt, va_gc> **vec)
     return false;
   if (TREE_CODE (t) == STATEMENT_LIST)
     {
-      tree_stmt_iterator i;
-      for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
-	{
-	  if (! build_data_member_initialization (tsi_stmt (i), vec))
-	    return false;
-	}
+      for (tree stmt : tsi_range (t))
+	if (! build_data_member_initialization (stmt, vec))
+	  return false;
       return true;
     }
   if (TREE_CODE (t) == CLEANUP_STMT)
@@ -577,10 +574,9 @@  build_constexpr_constructor_member_initializers (tree type, tree body)
 	break;
 
       case STATEMENT_LIST:
-	for (tree_stmt_iterator i = tsi_start (body);
-	     !tsi_end_p (i); tsi_next (&i))
+	for (tree stmt : tsi_range (body))
 	  {
-	    body = tsi_stmt (i);
+	    body = stmt;
 	    if (TREE_CODE (body) == BIND_EXPR)
 	      break;
 	  }
@@ -617,10 +613,9 @@  build_constexpr_constructor_member_initializers (tree type, tree body)
     }
   else if (TREE_CODE (body) == STATEMENT_LIST)
     {
-      tree_stmt_iterator i;
-      for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))
+      for (tree stmt : tsi_range (body))
 	{
-	  ok = build_data_member_initialization (tsi_stmt (i), &vec);
+	  ok = build_data_member_initialization (stmt, &vec);
 	  if (!ok)
 	    break;
 	}
@@ -675,11 +670,10 @@  constexpr_fn_retval (tree body)
     {
     case STATEMENT_LIST:
       {
-	tree_stmt_iterator i;
 	tree expr = NULL_TREE;
-	for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))
+	for (tree stmt : tsi_range (body))
 	  {
-	    tree s = constexpr_fn_retval (tsi_stmt (i));
+	    tree s = constexpr_fn_retval (stmt);
 	    if (s == error_mark_node)
 	      return error_mark_node;
 	    else if (s == NULL_TREE)
@@ -5772,7 +5766,6 @@  cxx_eval_statement_list (const constexpr_ctx *ctx, tree t,
 			 bool *non_constant_p, bool *overflow_p,
 			 tree *jump_target)
 {
-  tree_stmt_iterator i;
   tree local_target;
   /* In a statement-expression we want to return the last value.
      For empty statement expression return void_node.  */
@@ -5782,9 +5775,8 @@  cxx_eval_statement_list (const constexpr_ctx *ctx, tree t,
       local_target = NULL_TREE;
       jump_target = &local_target;
     }
-  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
+  for (tree stmt : tsi_range (t))
     {
-      tree stmt = tsi_stmt (i);
       /* We've found a continue, so skip everything until we reach
 	 the label its jumping to.  */
       if (continues (jump_target))
@@ -8283,16 +8275,10 @@  potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
       }
 
     case STATEMENT_LIST:
-      {
-	tree_stmt_iterator i;
-	for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
-	  {
-	    if (!RECUR (tsi_stmt (i), any))
-	      return false;
-	  }
-	return true;
-      }
-      break;
+      for (tree stmt : tsi_range (t))
+	if (!RECUR (stmt, any))
+	  return false;
+      return true;
 
     case MODIFY_EXPR:
       if (cxx_dialect < cxx14)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index dbd703a67cc..9b498f9d0b4 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1771,10 +1771,9 @@  await_statement_expander (tree *stmt, int *do_subtree, void *d)
     return NULL_TREE; /* Just process the sub-trees.  */
   else if (TREE_CODE (*stmt) == STATEMENT_LIST)
     {
-      tree_stmt_iterator i;
-      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))
+      for (tree &s : tsi_range (*stmt))
 	{
-	  res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_expander,
+	  res = cp_walk_tree (&s, await_statement_expander,
 			      d, NULL);
 	  if (res)
 	    return res;
@@ -3523,10 +3522,9 @@  await_statement_walker (tree *stmt, int *do_subtree, void *d)
     }
   else if (TREE_CODE (*stmt) == STATEMENT_LIST)
     {
-      tree_stmt_iterator i;
-      for (i = tsi_start (*stmt); !tsi_end_p (i); tsi_next (&i))
+      for (tree &s : tsi_range (*stmt))
 	{
-	  res = cp_walk_tree (tsi_stmt_ptr (i), await_statement_walker,
+	  res = cp_walk_tree (&s, await_statement_walker,
 			      d, NULL);
 	  if (res)
 	    return res;
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 02c19f55548..f0fb0144706 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -6094,9 +6094,8 @@  trees_out::core_vals (tree t)
       break;
 
     case STATEMENT_LIST:
-      for (tree_stmt_iterator iter = tsi_start (t);
-	   !tsi_end_p (iter); tsi_next (&iter))
-	if (tree stmt = tsi_stmt (iter))
+      for (tree stmt : tsi_range (t))
+	if (stmt)
 	  WT (stmt);
       WT (NULL_TREE);
       break;
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 116bdd2e42a..ad140cfd586 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18234,9 +18234,8 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
     {
     case STATEMENT_LIST:
       {
-	tree_stmt_iterator i;
-	for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
-	  RECUR (tsi_stmt (i));
+	for (tree stmt : tsi_range (t))
+	  RECUR (stmt);
 	break;
       }
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 6224f49f189..2912efad9be 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -613,9 +613,8 @@  set_cleanup_locs (tree stmts, location_t loc)
       set_cleanup_locs (CLEANUP_BODY (stmts), loc);
     }
   else if (TREE_CODE (stmts) == STATEMENT_LIST)
-    for (tree_stmt_iterator i = tsi_start (stmts);
-	 !tsi_end_p (i); tsi_next (&i))
-      set_cleanup_locs (tsi_stmt (i), loc);
+    for (tree stmt : tsi_range (stmts))
+      set_cleanup_locs (stmt, loc);
 }
 
 /* Finish a scope.  */