[PATCH] Fix hash_map::traverse overload

Matthias Kretz m.kretz@gsi.de
Tue Dec 7 08:38:57 GMT 2021


On Tuesday, 7 December 2021 08:47:56 CET Richard Biener wrote:
> On Tue, Dec 7, 2021 at 8:43 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> 
> >
> >
> > On Mon, Dec 6, 2021 at 11:47 AM Matthias Kretz <m.kretz@gsi.de> wrote:
> > 
> > >
> > >
> > > While reading the hash_map code I noticed this inconsistency.
> > > Bootstrapped and regtested on x86_64. OK for trunk?
> >
> >
> >
> > I've inspected two users of said overload and they return true.  Did you
> > look at the rest?  I assume that bootstrapping and testing with
> > asserting that the callback never returns false in that overload should
> > succeed?>
> >
> > That said, the inconsistency is bad - but how can we be sure we're not
> > relying on that?  I mean more than just bootstrapping and regtesting ;)

I could change the call back signature to return void. That would catch all 
uses, which I would modify to return void instead of true. If anyone then 
needs another overload which can break a correct overload can be added back.

> Btw, can you please amend the
> 
>  /* Call the call back on each pair of key and value with the passed in
>      arg.  */
> 
> comment to say how the return value influences iteration?  Maybe also
> note the traversal is unordered.

OK.

> Note hash-set.h has the same "problem" (a callback with a bool return
> but ignored result).  hash-table.h "properly" handles the return.

I'll take a look.

Matthias.

> 
> Richard.
> 
> 
> > Thanks,
> > Richard.
> >
> >
> >
> > >
> > >
> > > The hash_map::traverse overload taking a non-const Value pointer breaks
> > > if the callback returns false. The other overload should behave the
> > > same.
> > >
> > >
> > >
> > > Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
> > >
> > >
> > >
> > > gcc/ChangeLog:
> > >
> > >
> > >
> > >         * hash-map.h (hash_map::traverse): Let both overloads behave
> > >         the
> > >         same.
> > > 
> > > ---
> > > 
> > >  gcc/hash-map.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > >
> > >
> > >
> > > --
> > > ────────────────────────────────────────────────────────────────────────
> > > ──
> > > 
> > >  Dr. Matthias Kretz                          
> > >  https://mattkretz.github.io
> > >  GSI Helmholtz Centre for Heavy Ion Research              
> > >  https://gsi.de
> > >  stdₓ::simd
> > > 
> > > ────────────────────────────────────────────────────────────────────────
> > > ──


-- 
──────────────────────────────────────────────────────────────────────────
 Dr. Matthias Kretz                           https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
 stdₓ::simd
──────────────────────────────────────────────────────────────────────────


More information about the Gcc-patches mailing list