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