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

2021-05-28 Thread Richard Biener via Gcc-patches
On Wed, May 26, 2021 at 3:56 PM Jason Merrill  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
> >>  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  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, 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  
> > @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see
> >struct tree_stmt_iterator {
> >  struct 

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

2021-05-26 Thread Jason Merrill via Gcc-patches

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
 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  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, 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  
@@ -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 ++ () { ptr = ptr->next; return 
*this; }
+  tree_stmt_iterator  () { ptr = ptr->prev; return 
*this; }


I would suggest to add postincrement and postdecrement.


+  tree * () { 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-> 

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

2021-05-17 Thread Jason Merrill via Gcc-patches

On 5/17/21 3:56 AM, Richard Biener wrote:

On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches
 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  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, 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  
@@ -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 ++ () { ptr = ptr->next; return *this; }
+  tree_stmt_iterator  () { ptr = ptr->prev; return *this; }


I would suggest to add postincrement and postdecrement.


+  tree * () { 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 

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

2021-05-17 Thread Richard Biener via Gcc-patches
On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches
 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, 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  
> >> @@ -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 ++ () { ptr = ptr->next; return *this; }
> >> +  tree_stmt_iterator  () { ptr = ptr->prev; return *this; }
>
> I would suggest to add postincrement and postdecrement.
>
> >> +  tree * () { 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 >stmt;
> >> +  return &(*i);
> >>   }
> >>   static inline tree
> >>   tsi_stmt (tree_stmt_iterator i)
> >>   {
> >> -  return i.ptr->stmt;
> >> +  return *i;
> >>   }
> 

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

2021-05-13 Thread Jason Merrill via Gcc-patches

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  
@@ -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 ++ () { ptr = ptr->next; return *this; }
+  tree_stmt_iterator  () { ptr = ptr->prev; return *this; }


I would suggest to add postincrement and postdecrement.


+  tree * () { 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 >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 **vec)

  return false;
    if (TREE_CODE (t) == STATEMENT_LIST)
  {
-  tree_stmt_iterator i;
-  for (i = tsi_start (t); !tsi_end_p (i); tsi_next ())
-    {
-  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 ())
+    for 

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

2021-05-13 Thread Martin Sebor via Gcc-patches

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  
@@ -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 ++ () { ptr = ptr->next; return *this; }
+  tree_stmt_iterator  () { ptr = ptr->prev; return *this; }


I would suggest to add postincrement and postdecrement.


+  tree * () { 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 >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 **vec)

  return false;
    if (TREE_CODE (t) == STATEMENT_LIST)
  {
-  tree_stmt_iterator i;
-  for (i = tsi_start (t); !tsi_end_p (i); tsi_next ())
-    {
-  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 ())
+    for (tree stmt : tsi_range (body))
    {
-  

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

2021-05-13 Thread Jason Merrill via Gcc-patches

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  
  
@@ -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 ++ () { ptr = ptr->next; return *this; }
+  tree_stmt_iterator  () { ptr = ptr->prev; return *this; }
+  tree * () { 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 >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 **vec)
  return false;
if (TREE_CODE (t) == STATEMENT_LIST)
  {
-  tree_stmt_iterator i;
-  for (i = tsi_start (t); !tsi_end_p (i); tsi_next ())
-   {
- 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 ())
+   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 ())
+  for (tree stmt : tsi_range (body))
{
- ok = build_data_member_initialization (tsi_stmt (i), );
+ ok = build_data_member_initialization (stmt, );
 

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

2021-05-01 Thread Jason Merrill via Gcc-patches
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  
 
@@ -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 ++ () { ptr = ptr->next; return *this; }
+  tree_stmt_iterator  () { ptr = ptr->prev; return *this; }
+  tree * () { 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 >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 **vec)
 return false;
   if (TREE_CODE (t) == STATEMENT_LIST)
 {
-  tree_stmt_iterator i;
-  for (i = tsi_start (t); !tsi_end_p (i); tsi_next ())
-   {
- 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 ())
+   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 ())
+  for (tree stmt : tsi_range (body))
{
- ok = build_data_member_initialization (tsi_stmt (i), );
+ ok = build_data_member_initialization (stmt, );
  if (!ok)
break;
}
@@ -675,11 +670,10 @@ constexpr_fn_retval (tree body)