Re: [PATCH v2] Add .clang-format style file

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Wed, Mar 30, 2022 at 2:47 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 30 Mar 2022, Noah Goldstein via Libc-alpha wrote:
>
> > clang-format cleans up excess whitespace before the first comment
> > [MaxEmptyLinesToKeep: 1] and realigns part of the comments:
> >
> > -
> >  /* A reader--writer lock that fulfills the POSIX requirements (but operations
> >     on this lock are not necessarily full barriers, as one may interpret the
> >     POSIX requirement about "synchronizing memory").  All critical sections are
> > @@ -71,18 +70,18 @@
> >     #1    0   0   0   0   Lock is idle (and in a read phase).
> >     #2    0   0   >0  0   Readers have acquired the lock.
> >     #3    0   1   0   0   Lock is not acquired; a writer will try to start a
> > - write phase.
> > +                         write phase.
>
> But the indentation is correct as-is.  The general rule in glibc is that
> any multiple of 8 blank columns at the start of a line is represented by
> the corresponding number of TAB characters.  There may be a case for using
> spaces instead of tabs, but that's not the current style (except maybe for
> some files shared with gnulib).

My mistake, fixed in v4.

>
> > It also fixes difficult to ready indentation:
> >
> >      {
> >        rnew = r - (1 << PTHREAD_RWLOCK_READER_SHIFT);
> >        /* If we are the last reader, we also need to unblock any readers
> > - that are waiting for a writer to go first (PTHREAD_RWLOCK_RWAITING)
> > - so that they can register while the writer is active.  */
> > +         that are waiting for a writer to go first (PTHREAD_RWLOCK_RWAITING)
> > +         so that they can register while the writer is active.  */
> >        if ((rnew >> PTHREAD_RWLOCK_READER_SHIFT) == 0)
> > - {
> > -   if ((rnew & PTHREAD_RWLOCK_WRLOCKED) != 0)
> > -     rnew |= PTHREAD_RWLOCK_WRPHASE;
> > -   rnew &= ~(unsigned int) PTHREAD_RWLOCK_RWAITING;
> > - }
> > +        {
> > +          if ((rnew & PTHREAD_RWLOCK_WRLOCKED) != 0)
> > +            rnew |= PTHREAD_RWLOCK_WRPHASE;
> > +          rnew &= ~(unsigned int) PTHREAD_RWLOCK_RWAITING;
> > +        }
>
> Again, the indentation looks fine as-is; it's using TABs.
>
> > aligns conditions nicely:
> >
> >        while ((r & PTHREAD_RWLOCK_WRPHASE) == 0
> > -      && (r & PTHREAD_RWLOCK_WRLOCKED) != 0
> > -      && (r >> PTHREAD_RWLOCK_READER_SHIFT) > 0)
> >
> > +             && (r & PTHREAD_RWLOCK_WRLOCKED) != 0
> > +             && (r >> PTHREAD_RWLOCK_READER_SHIFT) > 0)
>
> Likewise, the indentation is already correct, using TABs.
>
> > and cleans up some obvious style mistakes
> >
> > - done:
> > +done:
>
> I don't think that's a style mistake either.  Emacs indents a label in the
> outermost block of a function like that (one-column indent rather than
> zero columns), and I think it's deliberate to support tools such as "diff
> -p", where it's desirable that, at any point within a function, the
> previous line starting with a letter is the line with the function name,
> not a line with a label somewhere within the function.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com