On Tue, Dec 7, 2021 at 9:38 AM Matthias Kretz <m.kr...@gsi.de> wrote: > > On Tuesday, 7 December 2021 08:47:56 CET Richard Biener wrote: > > On Tue, Dec 7, 2021 at 8:43 AM Richard Biener > > <richard.guent...@gmail.com> wrote: > > > > > > > > > > > On Mon, Dec 6, 2021 at 11:47 AM Matthias Kretz <m.kr...@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, I do like consistency between the container traversal APIs ... > > 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.kr...@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 > ──────────────────────────────────────────────────────────────────────────
