Re: [PATCH RFA] tree-iterator: C++11 range-for and tree_stmt_iterator
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
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
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
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
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
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
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
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)