[v2] Add .clang-format style file

  Went with version >= 9.0 since it covers most of the major features
and should be pretty universally accessibly.

There are some issues, particularly indention of preprocessor
directives. Unfortunately there doesn't appear to be a switch for
a seperate 'IndentWidth' for preprocessor directives vs. normal
code so we are stuck either not indenting the directives or
over-indenting them. Chose to over-indent as it generally seems easier
to script halving all pre-processor indentations than counting the
nested depth and indenting from scratch.

Other than the pre-processor directives most of the changes it makes
are cleaning up inconsistencies that already existed.
---
 .clang-format | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 .clang-format
  

Looks good but I don't know I have the permission to add a Reviewed-by:
:)

To justify this for others, it helps to attach some examples
how clang-format formatted files match or differ from the existing ones.

Showing some patches also helps as the patch formatter helps users the
most:

   git diff -U0 --no-color 'HEAD^' -- | py3
   path/to/llvm-project/clang/tools/clang-format/clang-format-diff.py -i -p1

On 2022-03-30, Noah Goldstein via Libc-alpha wrote:
>Went with version >= 9.0 since it covers most of the major features
>and should be pretty universally accessibly.

9.0 => 11.0

>There are some issues, particularly indention of preprocessor
>directives. Unfortunately there doesn't appear to be a switch for
>a seperate 'IndentWidth' for preprocessor directives vs. normal
>code so we are stuck either not indenting the directives or
>over-indenting them. Chose to over-indent as it generally seems easier
>to script halving all pre-processor indentations than counting the
>nested depth and indenting from scratch.
>
>Other than the pre-processor directives most of the changes it makes
>are cleaning up inconsistencies that already existed.
>---
> .clang-format | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 163 insertions(+)
> create mode 100644 .clang-format
>
>diff --git a/.clang-format b/.clang-format
>new file mode 100644
>index 0000000000..0a8663a596
>--- /dev/null
>+++ b/.clang-format
>@@ -0,0 +1,163 @@
>+#  clang-format file for GLIBC

The first line seems unneeded. The filename self tells.

>+#  Copyright (C) 2022 Free Software Foundation, Inc.
>+#  This file is part of the GNU C Library.
>+#
>+#  The GNU C Library is free software; you can redistribute it and/or
>+#  modify it under the terms of the GNU Lesser General Public
>+#  License as published by the Free Software Foundation; either
>+#  version 2.1 of the License, or (at your option) any later version.
>+#
>+#  The GNU C Library is distributed in the hope that it will be useful,
>+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
>+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+#  Lesser General Public License for more details.
>+#
>+#  You should have received a copy of the GNU Lesser General Public
>+#  License along with the GNU C Library; if not, see
>+#  <https://www.gnu.org/licenses/>.
>+#
>+#  Requires clang-format version >= 11.0
>+#
>+#  For more information, see:
>+#
>+#   Documentation/process/clang-format.rst
>+#   https://clang.llvm.org/docs/ClangFormat.html
>+#   https://clang.llvm.org/docs/ClangFormatStyleOptions.html
>+#
>+#  There are some known cases that this doesn't produce the desired
>+#  style (i.e Preprocessor Directives are over-indented and not
>+#  auto-commented). As a result, this is meant to be a utility to make
>+#  formatting easier, not a definitive standard.
>+---
>+# BasedOnStyle:  GNU
>+AccessModifierOffset: -2
>+AlignAfterOpenBracket: Align
>+AlignConsecutiveMacros: false
>+AlignConsecutiveAssignments: false
>+AlignConsecutiveBitFields: false
>+AlignConsecutiveDeclarations: false
>+AlignEscapedNewlines: Right
>+AlignOperands:   true
>+AlignTrailingComments: true
>+AllowAllArgumentsOnNextLine: true
>+AllowAllConstructorInitializersOnNextLine: true
>+AllowAllParametersOfDeclarationOnNextLine: true
>+AllowShortEnumsOnASingleLine: true
>+AllowShortBlocksOnASingleLine: false
>+AllowShortCaseLabelsOnASingleLine: false
>+AllowShortFunctionsOnASingleLine: All
>+AllowShortLambdasOnASingleLine: All
>+AllowShortIfStatementsOnASingleLine: Never
>+AllowShortLoopsOnASingleLine: false
>+AlwaysBreakAfterDefinitionReturnType: All
>+AlwaysBreakAfterReturnType: AllDefinitions
>+AlwaysBreakBeforeMultilineStrings: false
>+AlwaysBreakTemplateDeclarations: MultiLine
>+BinPackArguments: true
>+BinPackParameters: true
>+BraceWrapping:
>+  AfterCaseLabel:  true
>+  AfterClass:      true
>+  AfterControlStatement: true
>+  AfterEnum:       true
>+  AfterFunction:   true
>+  AfterNamespace:  true
>+  AfterStruct:     true
>+  AfterUnion:      true
>+  AfterExternBlock: true
>+  BeforeCatch:     true
>+  BeforeElse:      true
>+  BeforeWhile:     true
>+  IndentBraces:    true
>+  SplitEmptyFunction: true
>+  SplitEmptyRecord: true
>+  SplitEmptyNamespace: true
>+BreakBeforeBinaryOperators: All
>+BreakBeforeBraces: GNU
>+BreakBeforeInheritanceComma: false
>+BreakInheritanceList: BeforeColon
>+BreakBeforeTernaryOperators: true
>+BreakConstructorInitializersBeforeComma: false
>+BreakConstructorInitializers: BeforeColon
>+BreakStringLiterals: true
>+ColumnLimit:     79
>+CommentPragmas:  '^ IWYU pragma:'
>+CompactNamespaces: false
>+ConstructorInitializerAllOnOneLineOrOnePerLine: false
>+ConstructorInitializerIndentWidth: 4
>+ContinuationIndentWidth: 4
>+Cpp11BracedListStyle: false
>+DeriveLineEnding: true
>+DerivePointerAlignment: false
>+DisableFormat:   false
>+ExperimentalAutoDetectBinPacking: false
>+FixNamespaceComments: false
>+ForEachMacros:
>+  - foreach
>+  - Q_FOREACH
>+  - BOOST_FOREACH
>+IncludeBlocks:   Preserve
>+IncludeCategories:
>+  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
>+    Priority:        2
>+  - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
>+    Priority:        3
>+  - Regex:           '.*'
>+    Priority:        1
>+IncludeIsMainRegex: '(Test)?$'
>+IndentCaseLabels: false
>+IndentCaseBlocks: false
>+IndentGotoLabels: true
>+IndentWidth:     2
>+IndentPPDirectives: AfterHash
>+IndentExternBlock: AfterExternBlock
>+IndentWrappedFunctionNames: false
>+InsertTrailingCommas: None
>+KeepEmptyLinesAtTheStartOfBlocks: true
>+MacroBlockBegin: ''
>+MacroBlockEnd:   ''
>+MaxEmptyLinesToKeep: 1
>+NamespaceIndentation: None
>+PenaltyBreakAssignment: 2
>+PenaltyBreakBeforeFirstCallParameter: 19
>+PenaltyBreakComment: 300
>+PenaltyBreakFirstLessLess: 120
>+PenaltyBreakString: 1000
>+PenaltyBreakTemplateDeclaration: 10
>+PenaltyExcessCharacter: 1000000
>+PenaltyReturnTypeOnItsOwnLine: 60
>+PointerAlignment: Right
>+ReflowComments:  true
>+SortIncludes:    false
>+SortUsingDeclarations: true
>+SpaceAfterCStyleCast: true
>+SpaceAfterLogicalNot: false
>+SpaceAfterTemplateKeyword: true
>+SpaceBeforeAssignmentOperators: true
>+SpaceBeforeCpp11BracedList: false
>+SpaceBeforeCtorInitializerColon: true
>+SpaceBeforeInheritanceColon: true
>+SpaceBeforeParens: Always
>+SpaceBeforeRangeBasedForLoopColon: true
>+SpaceInEmptyBlock: false
>+SpaceInEmptyParentheses: false
>+SpacesBeforeTrailingComments: 1
>+SpacesInAngles:  false
>+SpacesInConditionalStatement: false
>+SpacesInContainerLiterals: true
>+SpacesInCStyleCastParentheses: false
>+SpacesInParentheses: false
>+SpacesInSquareBrackets: false
>+SpaceBeforeSquareBrackets: false
>+Standard:        Cpp03
>+StatementMacros:
>+  - Q_UNUSED
>+  - QT_REQUIRE_VERSION
>+TabWidth:        8
>+UseTab:          Never
>+ForEachMacros:
>+  - 'FOR_EACH_IMPL'
>+  - 'list_for_each'
>+  - 'list_for_each_prev'
>+  - 'list_for_each_prev_safe'
>+...
>-- 
>2.25.1
>
  

On Wed, Mar 30, 2022 at 1:17 PM Fangrui Song <maskray@google.com> wrote:
>
> Looks good but I don't know I have the permission to add a Reviewed-by:
> :)


Think it's fine, not like it can break anything. You've certainly
improved the patch.
Unless objection I'll add before I push
>
> To justify this for others, it helps to attach some examples
> how clang-format formatted files match or differ from the existing ones.
>
> Showing some patches also helps as the patch formatter helps users the
> most:
>
>    git diff -U0 --no-color 'HEAD^' -- | py3
>    path/to/llvm-project/clang/tools/clang-format/clang-format-diff.py -i -p1

Added a comment. If desirable we can add a script for it later.

Here are some diff examples:

Attached are four examples reformatted with clang-format. They
highlight some of the pros/cons of the new automated style.

pthread_rwlock_common.c:

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.

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;
+        }

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)

and cleans up some obvious style mistakes

- done:
+done:


pthread_self.c:

This diff highlights a drawback:

-libc_hidden_def (__pthread_self)
-weak_alias (__pthread_self, pthread_self)
+libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)

clang-format doesn't do so well with macros and without a semi-colon
breaking the line will concatenate the two lines which is undesirable
in this case (and the many other equivilent cases in glibc).

pthread_yield.c:

-#if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_2, GLIBC_2_34)
+#if OTHER_SHLIB_COMPAT(libpthread, GLIBC_2_2, GLIBC_2_34)

This is another issue in clang-format. Inside in #ifdef the
[SpaceBeforeParens: Always] configuration fails.

bench_memcpy.c:

Finally there is the issue of [IndentWidth: 2] applying to macros as
well as code

 #ifndef MEMCPY_RESULT
-# define MEMCPY_RESULT(dst, len) dst
-# define MIN_PAGE_SIZE 131072
-# define TEST_MAIN
-# define TEST_NAME "memcpy"
-# include "bench-string.h"
+#  define MEMCPY_RESULT(dst, len) dst
+#  define MIN_PAGE_SIZE 131072
+#  define TEST_MAIN
+#  define TEST_NAME "memcpy"
+#  include "bench-string.h"


There are active issues with clang-format to allow a seperate
IndentWidth for PP directives
(https://github.com/llvm/llvm-project/issues/42264) but until that is
fixed and we upgrade to a new version we are stuck either not
indenting or over-indenting.
>
> On 2022-03-30, Noah Goldstein via Libc-alpha wrote:
> >Went with version >= 9.0 since it covers most of the major features
> >and should be pretty universally accessibly.
>
> 9.0 => 11.0

Fixed in v2:
>
> >There are some issues, particularly indention of preprocessor
> >directives. Unfortunately there doesn't appear to be a switch for
> >a seperate 'IndentWidth' for preprocessor directives vs. normal
> >code so we are stuck either not indenting the directives or
> >over-indenting them. Chose to over-indent as it generally seems easier
> >to script halving all pre-processor indentations than counting the
> >nested depth and indenting from scratch.
> >
> >Other than the pre-processor directives most of the changes it makes
> >are cleaning up inconsistencies that already existed.
> >---
> > .clang-format | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 163 insertions(+)
> > create mode 100644 .clang-format
> >
> >diff --git a/.clang-format b/.clang-format
> >new file mode 100644
> >index 0000000000..0a8663a596
> >--- /dev/null
> >+++ b/.clang-format
> >@@ -0,0 +1,163 @@
> >+#  clang-format file for GLIBC
>
> The first line seems unneeded. The filename self tells.
>
> >+#  Copyright (C) 2022 Free Software Foundation, Inc.
> >+#  This file is part of the GNU C Library.
> >+#
> >+#  The GNU C Library is free software; you can redistribute it and/or
> >+#  modify it under the terms of the GNU Lesser General Public
> >+#  License as published by the Free Software Foundation; either
> >+#  version 2.1 of the License, or (at your option) any later version.
> >+#
> >+#  The GNU C Library is distributed in the hope that it will be useful,
> >+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >+#  Lesser General Public License for more details.
> >+#
> >+#  You should have received a copy of the GNU Lesser General Public
> >+#  License along with the GNU C Library; if not, see
> >+#  <https://www.gnu.org/licenses/>.
> >+#
> >+#  Requires clang-format version >= 11.0
> >+#
> >+#  For more information, see:
> >+#
> >+#   Documentation/process/clang-format.rst
> >+#   https://clang.llvm.org/docs/ClangFormat.html
> >+#   https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> >+#
> >+#  There are some known cases that this doesn't produce the desired
> >+#  style (i.e Preprocessor Directives are over-indented and not
> >+#  auto-commented). As a result, this is meant to be a utility to make
> >+#  formatting easier, not a definitive standard.
> >+---
> >+# BasedOnStyle:  GNU
> >+AccessModifierOffset: -2
> >+AlignAfterOpenBracket: Align
> >+AlignConsecutiveMacros: false
> >+AlignConsecutiveAssignments: false
> >+AlignConsecutiveBitFields: false
> >+AlignConsecutiveDeclarations: false
> >+AlignEscapedNewlines: Right
> >+AlignOperands:   true
> >+AlignTrailingComments: true
> >+AllowAllArgumentsOnNextLine: true
> >+AllowAllConstructorInitializersOnNextLine: true
> >+AllowAllParametersOfDeclarationOnNextLine: true
> >+AllowShortEnumsOnASingleLine: true
> >+AllowShortBlocksOnASingleLine: false
> >+AllowShortCaseLabelsOnASingleLine: false
> >+AllowShortFunctionsOnASingleLine: All
> >+AllowShortLambdasOnASingleLine: All
> >+AllowShortIfStatementsOnASingleLine: Never
> >+AllowShortLoopsOnASingleLine: false
> >+AlwaysBreakAfterDefinitionReturnType: All
> >+AlwaysBreakAfterReturnType: AllDefinitions
> >+AlwaysBreakBeforeMultilineStrings: false
> >+AlwaysBreakTemplateDeclarations: MultiLine
> >+BinPackArguments: true
> >+BinPackParameters: true
> >+BraceWrapping:
> >+  AfterCaseLabel:  true
> >+  AfterClass:      true
> >+  AfterControlStatement: true
> >+  AfterEnum:       true
> >+  AfterFunction:   true
> >+  AfterNamespace:  true
> >+  AfterStruct:     true
> >+  AfterUnion:      true
> >+  AfterExternBlock: true
> >+  BeforeCatch:     true
> >+  BeforeElse:      true
> >+  BeforeWhile:     true
> >+  IndentBraces:    true
> >+  SplitEmptyFunction: true
> >+  SplitEmptyRecord: true
> >+  SplitEmptyNamespace: true
> >+BreakBeforeBinaryOperators: All
> >+BreakBeforeBraces: GNU
> >+BreakBeforeInheritanceComma: false
> >+BreakInheritanceList: BeforeColon
> >+BreakBeforeTernaryOperators: true
> >+BreakConstructorInitializersBeforeComma: false
> >+BreakConstructorInitializers: BeforeColon
> >+BreakStringLiterals: true
> >+ColumnLimit:     79
> >+CommentPragmas:  '^ IWYU pragma:'
> >+CompactNamespaces: false
> >+ConstructorInitializerAllOnOneLineOrOnePerLine: false
> >+ConstructorInitializerIndentWidth: 4
> >+ContinuationIndentWidth: 4
> >+Cpp11BracedListStyle: false
> >+DeriveLineEnding: true
> >+DerivePointerAlignment: false
> >+DisableFormat:   false
> >+ExperimentalAutoDetectBinPacking: false
> >+FixNamespaceComments: false
> >+ForEachMacros:
> >+  - foreach
> >+  - Q_FOREACH
> >+  - BOOST_FOREACH
> >+IncludeBlocks:   Preserve
> >+IncludeCategories:
> >+  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
> >+    Priority:        2
> >+  - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
> >+    Priority:        3
> >+  - Regex:           '.*'
> >+    Priority:        1
> >+IncludeIsMainRegex: '(Test)?$'
> >+IndentCaseLabels: false
> >+IndentCaseBlocks: false
> >+IndentGotoLabels: true
> >+IndentWidth:     2
> >+IndentPPDirectives: AfterHash
> >+IndentExternBlock: AfterExternBlock
> >+IndentWrappedFunctionNames: false
> >+InsertTrailingCommas: None
> >+KeepEmptyLinesAtTheStartOfBlocks: true
> >+MacroBlockBegin: ''
> >+MacroBlockEnd:   ''
> >+MaxEmptyLinesToKeep: 1
> >+NamespaceIndentation: None
> >+PenaltyBreakAssignment: 2
> >+PenaltyBreakBeforeFirstCallParameter: 19
> >+PenaltyBreakComment: 300
> >+PenaltyBreakFirstLessLess: 120
> >+PenaltyBreakString: 1000
> >+PenaltyBreakTemplateDeclaration: 10
> >+PenaltyExcessCharacter: 1000000
> >+PenaltyReturnTypeOnItsOwnLine: 60
> >+PointerAlignment: Right
> >+ReflowComments:  true
> >+SortIncludes:    false
> >+SortUsingDeclarations: true
> >+SpaceAfterCStyleCast: true
> >+SpaceAfterLogicalNot: false
> >+SpaceAfterTemplateKeyword: true
> >+SpaceBeforeAssignmentOperators: true
> >+SpaceBeforeCpp11BracedList: false
> >+SpaceBeforeCtorInitializerColon: true
> >+SpaceBeforeInheritanceColon: true
> >+SpaceBeforeParens: Always
> >+SpaceBeforeRangeBasedForLoopColon: true
> >+SpaceInEmptyBlock: false
> >+SpaceInEmptyParentheses: false
> >+SpacesBeforeTrailingComments: 1
> >+SpacesInAngles:  false
> >+SpacesInConditionalStatement: false
> >+SpacesInContainerLiterals: true
> >+SpacesInCStyleCastParentheses: false
> >+SpacesInParentheses: false
> >+SpacesInSquareBrackets: false
> >+SpaceBeforeSquareBrackets: false
> >+Standard:        Cpp03
> >+StatementMacros:
> >+  - Q_UNUSED
> >+  - QT_REQUIRE_VERSION
> >+TabWidth:        8
> >+UseTab:          Never
> >+ForEachMacros:
> >+  - 'FOR_EACH_IMPL'
> >+  - 'list_for_each'
> >+  - 'list_for_each_prev'
> >+  - 'list_for_each_prev_safe'
> >+...
> >--
> >2.25.1
> >
  

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).

> 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.
  

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
  

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).
>
> > 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.

What's the switch for that in emacs?

But I still see it getting the correct function with done indented zero-columns.

>
> --
> Joseph S. Myers
> joseph@codesourcery.com
  

On Wed, 30 Mar 2022, Noah Goldstein via Libc-alpha wrote:

> > > 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.
> 
> What's the switch for that in emacs?

This is what emacs does by default; I don't know what settings there are 
to configure it.
  

On Mär 30 2022, Noah Goldstein via Libc-alpha wrote:

> What's the switch for that in emacs?

c-label-minimum-indentation