| Message ID | 20220330170200.2867894-1-goldstein.w.n@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Add .clang-format style file | expand |
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
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 +# 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' +...