On Wed, May 10, 2017 at 07:44:17AM +0100, Richard Sandiford wrote:
> tbsaunde+...@tbsaunde.org writes:
> > From: Trevor Saunders <tbsaunde+...@tbsaunde.org>
> >
> > This make the sbitmap version return true if the bit was previously
> > unset to make it similar to the bitmap version.
> >
> > gcc/ChangeLog:
> >
> > 2017-05-09 Trevor Saunders <tbsaunde+...@tbsaunde.org>
> >
> > * sbitmap.h (bitmap_set_bit): Return bool similar to bitmap
> > version of this function.
> > ---
> > gcc/sbitmap.h | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/sbitmap.h b/gcc/sbitmap.h
> > index cba0452cdb9..d4e3177d495 100644
> > --- a/gcc/sbitmap.h
> > +++ b/gcc/sbitmap.h
> > @@ -108,11 +108,14 @@ bitmap_bit_p (const_sbitmap map, int bitno)
> >
> > /* Set bit number BITNO in the sbitmap MAP. */
> >
> > -static inline void
> > +static inline bool
> > bitmap_set_bit (sbitmap map, int bitno)
> > {
> > - map->elms[bitno / SBITMAP_ELT_BITS]
> > - |= (SBITMAP_ELT_TYPE) 1 << (bitno) % SBITMAP_ELT_BITS;
> > + SBITMAP_ELT_TYPE &word = map->elms[bitno / SBITMAP_ELT_BITS];
> > + SBITMAP_ELT_TYPE mask = (SBITMAP_ELT_TYPE) 1 << (bitno) %
> > SBITMAP_ELT_BITS;
> > + bool ret = (word & mask) == 0;
> > + word |= mask;
> > + return ret;
> > }
>
> Indentation looks off (mabye it's a mailer thing?). Think the function
> comment should be updated too -- personally I can never remember whether
> true means "I just set it" or "it was already set" :-)
Sure, I can handle that.
> What's the current position on the use of references? IMO a pointer
> is clearer here.
Well, I generally think non const references aren't a great idea, so I'm
not really sure why I used one here. Anyway not a big deal so happy to
change that.
thanks
Trev
>
> Thanks,
> Richard
