* [PATCH v1] Add .clang-format style file
@ 2022-03-28 19:12 Noah Goldstein
2022-03-30 0:26 ` Fangrui Song
` (4 more replies)
0 siblings, 5 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-03-28 19:12 UTC (permalink / raw)
To: libc-alpha
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 | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 164 insertions(+)
create mode 100644 .clang-format
diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..121b475dbd
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,164 @@
+# 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 >= 9.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 # clang-format >= 11.0
+AlignConsecutiveDeclarations: false
+AlignEscapedNewlines: Right
+AlignOperands: true
+AlignTrailingComments: true
+AllowAllArgumentsOnNextLine: true
+AllowAllConstructorInitializersOnNextLine: true
+AllowAllParametersOfDeclarationOnNextLine: true
+#AllowShortEnumsOnASingleLine: true # clang-format >= 11.0
+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 # clang-format >= 11.0
+ 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 # clang-format >= 10.0
+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 # clang-format >= 11.0
+#IndentGotoLabels: true # clang-format >= 10.0
+IndentWidth: 2
+IndentPPDirectives: AfterHash
+#IndentExternBlock: AfterExternBlock # clang-format >= 11.0
+IndentWrappedFunctionNames: false
+#InsertTrailingCommas: None # clang-format >= 11.0
+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 # clang-format >= 10.0
+SpaceInEmptyParentheses: false
+SpacesBeforeTrailingComments: 1
+SpacesInAngles: false
+#SpacesInConditionalStatement: false # clang-format >= 10.0
+SpacesInContainerLiterals: true
+SpacesInCStyleCastParentheses: false
+SpacesInParentheses: false
+SpacesInSquareBrackets: false
+#SpaceBeforeSquareBrackets: false # clang-format >= 10.0
+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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] Add .clang-format style file
2022-03-28 19:12 [PATCH v1] Add .clang-format style file Noah Goldstein
@ 2022-03-30 0:26 ` Fangrui Song
2022-03-30 1:33 ` Noah Goldstein
2022-03-30 19:27 ` [PATCH v3] " Noah Goldstein
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Fangrui Song @ 2022-03-30 0:26 UTC (permalink / raw)
To: Noah Goldstein; +Cc: libc-alpha
On 2022-03-28, 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.
Why not 10.0? clang-format 10.0.0 has been there for two years. Using
it will remove some clang-format >= 10.0 below. Related:
Linux kernel now requires Clang >= 11.0.
Since this patch introduces new tool, we can require higher versions.
Installing new clang-format isn't so difficult for old systems. There
are prebuilt packages even if the distro stops upgrading them.
>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.
https://github.com/llvm/llvm-project/issues/42264
"Feature request: Two different indentation width options, one for code, one for pre-processor directives" :(
>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.
Yeah, I picked over-indent for compiler-rt sanitizers as well
https://reviews.llvm.org/D100238
>Other than the pre-processor directives most of the changes it makes
>are cleaning up inconsistencies that already existed.
>---
> .clang-format | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 164 insertions(+)
> create mode 100644 .clang-format
>
>diff --git a/.clang-format b/.clang-format
>new file mode 100644
>index 0000000000..121b475dbd
>--- /dev/null
>+++ b/.clang-format
>@@ -0,0 +1,164 @@
>+# 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 >= 9.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 # clang-format >= 11.0
>+AlignConsecutiveDeclarations: false
>+AlignEscapedNewlines: Right
>+AlignOperands: true
>+AlignTrailingComments: true
>+AllowAllArgumentsOnNextLine: true
>+AllowAllConstructorInitializersOnNextLine: true
>+AllowAllParametersOfDeclarationOnNextLine: true
>+#AllowShortEnumsOnASingleLine: true # clang-format >= 11.0
>+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 # clang-format >= 11.0
>+ 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 # clang-format >= 10.0
>+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 # clang-format >= 11.0
>+#IndentGotoLabels: true # clang-format >= 10.0
>+IndentWidth: 2
>+IndentPPDirectives: AfterHash
>+#IndentExternBlock: AfterExternBlock # clang-format >= 11.0
>+IndentWrappedFunctionNames: false
>+#InsertTrailingCommas: None # clang-format >= 11.0
>+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 # clang-format >= 10.0
>+SpaceInEmptyParentheses: false
>+SpacesBeforeTrailingComments: 1
>+SpacesInAngles: false
>+#SpacesInConditionalStatement: false # clang-format >= 10.0
>+SpacesInContainerLiterals: true
>+SpacesInCStyleCastParentheses: false
>+SpacesInParentheses: false
>+SpacesInSquareBrackets: false
>+#SpaceBeforeSquareBrackets: false # clang-format >= 10.0
>+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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] Add .clang-format style file
2022-03-30 0:26 ` Fangrui Song
@ 2022-03-30 1:33 ` Noah Goldstein
2022-03-30 17:02 ` [PATCH v2] " Noah Goldstein
0 siblings, 1 reply; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 1:33 UTC (permalink / raw)
To: Fangrui Song; +Cc: GNU C Library
On Tue, Mar 29, 2022 at 7:27 PM Fangrui Song <maskray@google.com> wrote:
>
> On 2022-03-28, 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.
>
> Why not 10.0? clang-format 10.0.0 has been there for two years. Using
> it will remove some clang-format >= 10.0 below. Related:
> Linux kernel now requires Clang >= 11.0.
> Since this patch introduces new tool, we can require higher versions.
> Installing new clang-format isn't so difficult for old systems. There
> are prebuilt packages even if the distro stops upgrading them.
Fair enough. If linux is on 11.0+ that seems to be enough of a precedent.
Will post v2 with 11.0+ tomorrow so if anyone has a reason to stick with
9.0 they can comment.
>
> >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.
>
> https://github.com/llvm/llvm-project/issues/42264
> "Feature request: Two different indentation width options, one for code, one for pre-processor directives" :(
>
> >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.
>
> Yeah, I picked over-indent for compiler-rt sanitizers as well
> https://reviews.llvm.org/D100238
>
> >Other than the pre-processor directives most of the changes it makes
> >are cleaning up inconsistencies that already existed.
> >---
> > .clang-format | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 164 insertions(+)
> > create mode 100644 .clang-format
> >
> >diff --git a/.clang-format b/.clang-format
> >new file mode 100644
> >index 0000000000..121b475dbd
> >--- /dev/null
> >+++ b/.clang-format
> >@@ -0,0 +1,164 @@
> >+# 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 >= 9.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 # clang-format >= 11.0
> >+AlignConsecutiveDeclarations: false
> >+AlignEscapedNewlines: Right
> >+AlignOperands: true
> >+AlignTrailingComments: true
> >+AllowAllArgumentsOnNextLine: true
> >+AllowAllConstructorInitializersOnNextLine: true
> >+AllowAllParametersOfDeclarationOnNextLine: true
> >+#AllowShortEnumsOnASingleLine: true # clang-format >= 11.0
> >+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 # clang-format >= 11.0
> >+ 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 # clang-format >= 10.0
> >+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 # clang-format >= 11.0
> >+#IndentGotoLabels: true # clang-format >= 10.0
> >+IndentWidth: 2
> >+IndentPPDirectives: AfterHash
> >+#IndentExternBlock: AfterExternBlock # clang-format >= 11.0
> >+IndentWrappedFunctionNames: false
> >+#InsertTrailingCommas: None # clang-format >= 11.0
> >+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 # clang-format >= 10.0
> >+SpaceInEmptyParentheses: false
> >+SpacesBeforeTrailingComments: 1
> >+SpacesInAngles: false
> >+#SpacesInConditionalStatement: false # clang-format >= 10.0
> >+SpacesInContainerLiterals: true
> >+SpacesInCStyleCastParentheses: false
> >+SpacesInParentheses: false
> >+SpacesInSquareBrackets: false
> >+#SpaceBeforeSquareBrackets: false # clang-format >= 10.0
> >+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
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] Add .clang-format style file
2022-03-30 1:33 ` Noah Goldstein
@ 2022-03-30 17:02 ` Noah Goldstein
2022-03-30 18:17 ` Fangrui Song
0 siblings, 1 reply; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 17:02 UTC (permalink / raw)
To: libc-alpha
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
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'
+...
--
2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 17:02 ` [PATCH v2] " Noah Goldstein
@ 2022-03-30 18:17 ` Fangrui Song
2022-03-30 19:30 ` Noah Goldstein
0 siblings, 1 reply; 24+ messages in thread
From: Fangrui Song @ 2022-03-30 18:17 UTC (permalink / raw)
To: Noah Goldstein; +Cc: libc-alpha
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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] Add .clang-format style file
2022-03-28 19:12 [PATCH v1] Add .clang-format style file Noah Goldstein
2022-03-30 0:26 ` Fangrui Song
@ 2022-03-30 19:27 ` Noah Goldstein
2022-03-30 19:59 ` Fangrui Song
2022-03-30 20:01 ` Florian Weimer
2022-03-30 20:10 ` [PATCH v4] " Noah Goldstein
` (2 subsequent siblings)
4 siblings, 2 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 19:27 UTC (permalink / raw)
To: libc-alpha
Went with version >= 11.0 since it covers most of the major features
and should be pretty universally accessibly.
There are some issues:
1. 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. i.e:
Desired:
```
#ifndef A
# define B
#endif
```
Options:
```
#ifndef A
# define B /* Two spaces instead of one. */
#endif
#ifndef C
#define D /* No spaces. */
#endif
```
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.
2. concatenation of lines missing semi-colons:
Throughout glibc there are macros used to setup aliasing that are
outside of functions and don't end in semi-colons i.e:
```
libc_hidden_def (__pthread_self)
weak_alias (__pthread_self, pthread_self)
```
clang-format reformats lines like these to:
```
libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
```
which is generally undesirable.
Other than those two big concerns there are certainly some questions
diffs but for the most part it creates a easy to read and consistent
style.
---
.clang-format | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 175 insertions(+)
create mode 100644 .clang-format
diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..fcb344ef70
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,175 @@
+# 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.
+#
+# To format the current git diff inplace (-i) the follow command can
+# be used:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
+#
+# To just view the diff clang-format would generate:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
+#
+# NB: clang-format-diff, along with other clang-format related tools,
+# can be found at: /path/to/llvm-project/clang/tools/clang-format/
+#
+#
+---
+# 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 18:17 ` Fangrui Song
@ 2022-03-30 19:30 ` Noah Goldstein
2022-03-30 19:47 ` Joseph Myers
0 siblings, 1 reply; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 19:30 UTC (permalink / raw)
To: Fangrui Song; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 10996 bytes --]
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
> >
[-- Attachment #2: v1-0004-clang-format-bench-memcpy.c.patch --]
[-- Type: text/x-patch, Size: 1304 bytes --]
From 1cb19acfef965d98b7cb4873d155fa2fb4fe166f Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n@gmail.com>
Date: Wed, 30 Mar 2022 14:04:01 -0500
Subject: [PATCH v1 4/4] clang-format bench-memcpy.c
---
benchtests/bench-memcpy.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/benchtests/bench-memcpy.c b/benchtests/bench-memcpy.c
index 4194d5add1..2f55fa91df 100644
--- a/benchtests/bench-memcpy.c
+++ b/benchtests/bench-memcpy.c
@@ -17,11 +17,11 @@
<https://www.gnu.org/licenses/>. */
#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"
void *generic_memcpy (void *, const void *, size_t);
@@ -30,13 +30,13 @@ IMPL (generic_memcpy, 0)
#endif
-# include "json-lib.h"
+#include "json-lib.h"
typedef void *(*proto_t) (void *, const void *, size_t);
static void
do_one_test (json_ctx_t *json_ctx, impl_t *impl, char *dst, const char *src,
- size_t len)
+ size_t len)
{
size_t i, iters = INNER_LOOP_ITERS;
timing_t start, stop, cur;
--
2.25.1
[-- Attachment #3: v1-0002-clang-format-pthread_self.c.patch --]
[-- Type: text/x-patch, Size: 680 bytes --]
From a905b3f269a07600e1c07308a1b24e51eb9b61f4 Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n@gmail.com>
Date: Wed, 30 Mar 2022 14:02:02 -0500
Subject: [PATCH v1 2/4] clang-format pthread_self.c
---
nptl/pthread_self.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/nptl/pthread_self.c b/nptl/pthread_self.c
index dac847f656..c9537a358e 100644
--- a/nptl/pthread_self.c
+++ b/nptl/pthread_self.c
@@ -23,5 +23,4 @@ __pthread_self (void)
{
return (pthread_t) THREAD_SELF;
}
-libc_hidden_def (__pthread_self)
-weak_alias (__pthread_self, pthread_self)
+libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
--
2.25.1
[-- Attachment #4: v1-0003-clang-format-pthread_yield.c.patch --]
[-- Type: text/x-patch, Size: 698 bytes --]
From 782cdf85ed53a5d1adf14654682fc77221a2d4cd Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n@gmail.com>
Date: Wed, 30 Mar 2022 14:02:22 -0500
Subject: [PATCH v1 3/4] clang-format pthread_yield.c
---
nptl/pthread_yield.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nptl/pthread_yield.c b/nptl/pthread_yield.c
index e80227028c..f299eacc49 100644
--- a/nptl/pthread_yield.c
+++ b/nptl/pthread_yield.c
@@ -19,7 +19,7 @@
#include <sched.h>
#include <shlib-compat.h>
-#if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_2, GLIBC_2_34)
+#if OTHER_SHLIB_COMPAT(libpthread, GLIBC_2_2, GLIBC_2_34)
int attribute_compat_text_section
__pthread_yield (void)
{
--
2.25.1
[-- Attachment #5: v1-0001-clang-format-pthread_rwlock_common.c.patch --]
[-- Type: text/x-patch, Size: 55762 bytes --]
From d717623535955e121873b7c39db3c36cd63d7d5f Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n@gmail.com>
Date: Wed, 30 Mar 2022 14:01:41 -0500
Subject: [PATCH v1 1/4] clang-format pthread_rwlock_common.c
---
nptl/pthread_rwlock_common.c | 966 ++++++++++++++++++-----------------
1 file changed, 487 insertions(+), 479 deletions(-)
diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
index bdc55d9592..6474d9ec65 100644
--- a/nptl/pthread_rwlock_common.c
+++ b/nptl/pthread_rwlock_common.c
@@ -26,7 +26,6 @@
#include <futex-internal.h>
#include <time.h>
-
/* 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.
#4 0 1 >0 0 Readers have acquired the lock; a writer is waiting
- and explicit hand-over to the writer is required.
+ and explicit hand-over to the writer is required.
#4a 0 1 >0 1 Same as #4 except that there are further readers
- waiting because the writer is to be preferred.
+ waiting because the writer is to be preferred.
#5 1 0 0 0 Lock is idle (and in a write phase).
#6 1 0 >0 0 Write phase; readers will try to start a read phase
- (requires explicit hand-over to all readers that
- do not start the read phase).
+ (requires explicit hand-over to all readers that
+ do not start the read phase).
#7 1 1 0 0 Lock is acquired by a writer.
#8 1 1 >0 0 Lock acquired by a writer and readers are waiting;
- explicit hand-over to the readers is required.
+ explicit hand-over to the readers is required.
WP (PTHREAD_RWLOCK_WRPHASE) is true if the lock is in a write phase, so
potentially acquired by a primary writer.
@@ -214,7 +213,6 @@
deciding when to use elision so that enabling it would lead to consistently
better performance. */
-
static int
__pthread_rwlock_get_private (pthread_rwlock_t *rwlock)
{
@@ -235,48 +233,48 @@ __pthread_rwlock_rdunlock (pthread_rwlock_t *rwlock)
{
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;
+ }
/* We need release MO here for three reasons. First, so that we
- synchronize with subsequent writers. Second, we might have been the
- first reader and set __wrphase_futex to 0, so we need to synchronize
- with the last reader that will set it to 1 (note that we will always
- change __readers before the last reader, or we are the last reader).
- Third, a writer that takes part in explicit hand-over needs to see
- the first reader's store to __wrphase_futex (or a later value) if
- the writer observes that a write phase has been started. */
- if (atomic_compare_exchange_weak_release (&rwlock->__data.__readers,
- &r, rnew))
- break;
+ synchronize with subsequent writers. Second, we might have been the
+ first reader and set __wrphase_futex to 0, so we need to synchronize
+ with the last reader that will set it to 1 (note that we will always
+ change __readers before the last reader, or we are the last reader).
+ Third, a writer that takes part in explicit hand-over needs to see
+ the first reader's store to __wrphase_futex (or a later value) if
+ the writer observes that a write phase has been started. */
+ if (atomic_compare_exchange_weak_release (&rwlock->__data.__readers, &r,
+ rnew))
+ break;
/* TODO Back-off. */
}
if ((rnew & PTHREAD_RWLOCK_WRPHASE) != 0)
{
/* We need to do explicit hand-over. We need the acquire MO fence so
- that our modification of _wrphase_futex happens after a store by
- another reader that started a read phase. Relaxed MO is sufficient
- for the modification of __wrphase_futex because it is just used
- to delay acquisition by a writer until all threads are unblocked
- irrespective of whether they are looking at __readers or
- __wrphase_futex; any other synchronizes-with relations that are
- necessary are established through __readers. */
+ that our modification of _wrphase_futex happens after a store by
+ another reader that started a read phase. Relaxed MO is sufficient
+ for the modification of __wrphase_futex because it is just used
+ to delay acquisition by a writer until all threads are unblocked
+ irrespective of whether they are looking at __readers or
+ __wrphase_futex; any other synchronizes-with relations that are
+ necessary are established through __readers. */
atomic_thread_fence_acquire ();
if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 1)
- & PTHREAD_RWLOCK_FUTEX_USED) != 0)
- futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
+ & PTHREAD_RWLOCK_FUTEX_USED)
+ != 0)
+ futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
}
/* Also wake up waiting readers if we did reset the RWAITING flag. */
if ((r & PTHREAD_RWLOCK_RWAITING) != (rnew & PTHREAD_RWLOCK_RWAITING))
futex_wake (&rwlock->__data.__readers, INT_MAX, private);
}
-
static __always_inline int
__pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
const struct __timespec64 *abstime)
@@ -289,14 +287,15 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
requires that "the validity of the abstime parameter need not be checked
if the lock can be immediately acquired" (i.e., we need not but may check
it). */
- if (abstime && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid)
- || ! valid_nanoseconds (abstime->tv_nsec)))
+ if (abstime
+ && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid)
+ || !valid_nanoseconds (abstime->tv_nsec)))
return EINVAL;
/* Make sure we are not holding the rwlock as a writer. This is a deadlock
situation we recognize and report. */
if (__glibc_unlikely (atomic_load_relaxed (&rwlock->__data.__cur_writer)
- == THREAD_GETMEM (THREAD_SELF, tid)))
+ == THREAD_GETMEM (THREAD_SELF, tid)))
return EDEADLK;
/* If we prefer writers, recursive rdlock is disallowed, we are in a read
@@ -311,47 +310,47 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
{
r = atomic_load_relaxed (&rwlock->__data.__readers);
while ((r & PTHREAD_RWLOCK_WRPHASE) == 0
- && (r & PTHREAD_RWLOCK_WRLOCKED) != 0
- && (r >> PTHREAD_RWLOCK_READER_SHIFT) > 0)
- {
- /* TODO Spin first. */
- /* Try setting the flag signaling that we are waiting without having
- incremented the number of readers. Relaxed MO is fine because
- this is just about waiting for a state change in __readers. */
- if (atomic_compare_exchange_weak_relaxed
- (&rwlock->__data.__readers, &r, r | PTHREAD_RWLOCK_RWAITING))
- {
- /* Wait for as long as the flag is set. An ABA situation is
- harmless because the flag is just about the state of
- __readers, and all threads set the flag under the same
- conditions. */
- while (((r = atomic_load_relaxed (&rwlock->__data.__readers))
- & PTHREAD_RWLOCK_RWAITING) != 0)
- {
- int private = __pthread_rwlock_get_private (rwlock);
- int err = __futex_abstimed_wait64 (&rwlock->__data.__readers,
- r, clockid, abstime,
- private);
- /* We ignore EAGAIN and EINTR. On time-outs, we can just
- return because we don't need to clean up anything. */
- if (err == ETIMEDOUT || err == EOVERFLOW)
- return err;
- }
- /* It makes sense to not break out of the outer loop here
- because we might be in the same situation again. */
- }
- else
- {
- /* TODO Back-off. */
- }
- }
+ && (r & PTHREAD_RWLOCK_WRLOCKED) != 0
+ && (r >> PTHREAD_RWLOCK_READER_SHIFT) > 0)
+ {
+ /* TODO Spin first. */
+ /* Try setting the flag signaling that we are waiting without having
+ incremented the number of readers. Relaxed MO is fine because
+ this is just about waiting for a state change in __readers. */
+ if (atomic_compare_exchange_weak_relaxed (
+ &rwlock->__data.__readers, &r, r | PTHREAD_RWLOCK_RWAITING))
+ {
+ /* Wait for as long as the flag is set. An ABA situation is
+ harmless because the flag is just about the state of
+ __readers, and all threads set the flag under the same
+ conditions. */
+ while (((r = atomic_load_relaxed (&rwlock->__data.__readers))
+ & PTHREAD_RWLOCK_RWAITING)
+ != 0)
+ {
+ int private = __pthread_rwlock_get_private (rwlock);
+ int err = __futex_abstimed_wait64 (
+ &rwlock->__data.__readers, r, clockid, abstime, private);
+ /* We ignore EAGAIN and EINTR. On time-outs, we can just
+ return because we don't need to clean up anything. */
+ if (err == ETIMEDOUT || err == EOVERFLOW)
+ return err;
+ }
+ /* It makes sense to not break out of the outer loop here
+ because we might be in the same situation again. */
+ }
+ else
+ {
+ /* TODO Back-off. */
+ }
+ }
}
/* Register as a reader, using an add-and-fetch so that R can be used as
expected value for future operations. Acquire MO so we synchronize with
prior writers as well as the last reader of the previous read phase (see
below). */
r = (atomic_fetch_add_acquire (&rwlock->__data.__readers,
- (1 << PTHREAD_RWLOCK_READER_SHIFT))
+ (1 << PTHREAD_RWLOCK_READER_SHIFT))
+ (1 << PTHREAD_RWLOCK_READER_SHIFT));
/* Check whether there is an overflow in the number of readers. We assume
@@ -370,12 +369,12 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
while (__glibc_unlikely (r >= PTHREAD_RWLOCK_READER_OVERFLOW))
{
/* Relaxed MO is okay because we just want to undo our registration and
- cannot have changed the rwlock state substantially if the CAS
- succeeds. */
- if (atomic_compare_exchange_weak_relaxed
- (&rwlock->__data.__readers,
- &r, r - (1 << PTHREAD_RWLOCK_READER_SHIFT)))
- return EAGAIN;
+ cannot have changed the rwlock state substantially if the CAS
+ succeeds. */
+ if (atomic_compare_exchange_weak_relaxed (
+ &rwlock->__data.__readers, &r,
+ r - (1 << PTHREAD_RWLOCK_READER_SHIFT)))
+ return EAGAIN;
}
/* We have registered as a reader, so if we are in a read phase, we have
@@ -393,35 +392,36 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
for explicit hand-over of the read phase; the only exception is if we
can start a read phase if there is no primary writer currently. */
while ((r & PTHREAD_RWLOCK_WRPHASE) != 0
- && (r & PTHREAD_RWLOCK_WRLOCKED) == 0)
+ && (r & PTHREAD_RWLOCK_WRLOCKED) == 0)
{
/* Try to enter a read phase: If the CAS below succeeds, we have
- ownership; if it fails, we will simply retry and reassess the
- situation.
- Acquire MO so we synchronize with prior writers. */
+ ownership; if it fails, we will simply retry and reassess the
+ situation.
+ Acquire MO so we synchronize with prior writers. */
if (atomic_compare_exchange_weak_acquire (&rwlock->__data.__readers, &r,
- r ^ PTHREAD_RWLOCK_WRPHASE))
- {
- /* We started the read phase, so we are also responsible for
- updating the write-phase futex. Relaxed MO is sufficient.
- We have to do the same steps as a writer would when handing
- over the read phase to us because other readers cannot
- distinguish between us and the writer; this includes
- explicit hand-over and potentially having to wake other readers
- (but we can pretend to do the setting and unsetting of WRLOCKED
- atomically, and thus can skip this step). */
- if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0)
- & PTHREAD_RWLOCK_FUTEX_USED) != 0)
- {
- int private = __pthread_rwlock_get_private (rwlock);
- futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
- }
- return 0;
- }
+ r ^ PTHREAD_RWLOCK_WRPHASE))
+ {
+ /* We started the read phase, so we are also responsible for
+ updating the write-phase futex. Relaxed MO is sufficient.
+ We have to do the same steps as a writer would when handing
+ over the read phase to us because other readers cannot
+ distinguish between us and the writer; this includes
+ explicit hand-over and potentially having to wake other readers
+ (but we can pretend to do the setting and unsetting of WRLOCKED
+ atomically, and thus can skip this step). */
+ if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0)
+ & PTHREAD_RWLOCK_FUTEX_USED)
+ != 0)
+ {
+ int private = __pthread_rwlock_get_private (rwlock);
+ futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
+ }
+ return 0;
+ }
else
- {
- /* TODO Back off before retrying. Also see above. */
- }
+ {
+ /* TODO Back off before retrying. Also see above. */
+ }
}
/* We were in a write phase but did not install the read phase. We cannot
@@ -449,80 +449,81 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
for (;;)
{
while (((wpf = atomic_load_relaxed (&rwlock->__data.__wrphase_futex))
- | PTHREAD_RWLOCK_FUTEX_USED) == (1 | PTHREAD_RWLOCK_FUTEX_USED))
- {
- int private = __pthread_rwlock_get_private (rwlock);
- if (((wpf & PTHREAD_RWLOCK_FUTEX_USED) == 0)
- && (!atomic_compare_exchange_weak_relaxed
- (&rwlock->__data.__wrphase_futex,
- &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED)))
- continue;
- int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
- 1 | PTHREAD_RWLOCK_FUTEX_USED,
- clockid, abstime, private);
- if (err == ETIMEDOUT || err == EOVERFLOW)
- {
- /* If we timed out, we need to unregister. If no read phase
- has been installed while we waited, we can just decrement
- the number of readers. Otherwise, we just acquire the
- lock, which is allowed because we give no precise timing
- guarantees, and because the timeout is only required to
- be in effect if we would have had to wait for other
- threads (e.g., if futex_wait would time-out immediately
- because the given absolute time is in the past). */
- r = atomic_load_relaxed (&rwlock->__data.__readers);
- while ((r & PTHREAD_RWLOCK_WRPHASE) != 0)
- {
- /* We don't need to make anything else visible to
- others besides unregistering, so relaxed MO is
- sufficient. */
- if (atomic_compare_exchange_weak_relaxed
- (&rwlock->__data.__readers, &r,
- r - (1 << PTHREAD_RWLOCK_READER_SHIFT)))
- return err;
- /* TODO Back-off. */
- }
- /* Use the acquire MO fence to mirror the steps taken in the
- non-timeout case. Note that the read can happen both
- in the atomic_load above as well as in the failure case
- of the CAS operation. */
- atomic_thread_fence_acquire ();
- /* We still need to wait for explicit hand-over, but we must
- not use futex_wait anymore because we would just time out
- in this case and thus make the spin-waiting we need
- unnecessarily expensive. */
- while ((atomic_load_relaxed (&rwlock->__data.__wrphase_futex)
- | PTHREAD_RWLOCK_FUTEX_USED)
- == (1 | PTHREAD_RWLOCK_FUTEX_USED))
- {
- /* TODO Back-off? */
- }
- ready = true;
- break;
- }
- /* If we got interrupted (EINTR) or the futex word does not have the
- expected value (EAGAIN), retry. */
- }
+ | PTHREAD_RWLOCK_FUTEX_USED)
+ == (1 | PTHREAD_RWLOCK_FUTEX_USED))
+ {
+ int private = __pthread_rwlock_get_private (rwlock);
+ if (((wpf & PTHREAD_RWLOCK_FUTEX_USED) == 0)
+ && (!atomic_compare_exchange_weak_relaxed (
+ &rwlock->__data.__wrphase_futex, &wpf,
+ wpf | PTHREAD_RWLOCK_FUTEX_USED)))
+ continue;
+ int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
+ 1 | PTHREAD_RWLOCK_FUTEX_USED,
+ clockid, abstime, private);
+ if (err == ETIMEDOUT || err == EOVERFLOW)
+ {
+ /* If we timed out, we need to unregister. If no read phase
+ has been installed while we waited, we can just decrement
+ the number of readers. Otherwise, we just acquire the
+ lock, which is allowed because we give no precise timing
+ guarantees, and because the timeout is only required to
+ be in effect if we would have had to wait for other
+ threads (e.g., if futex_wait would time-out immediately
+ because the given absolute time is in the past). */
+ r = atomic_load_relaxed (&rwlock->__data.__readers);
+ while ((r & PTHREAD_RWLOCK_WRPHASE) != 0)
+ {
+ /* We don't need to make anything else visible to
+ others besides unregistering, so relaxed MO is
+ sufficient. */
+ if (atomic_compare_exchange_weak_relaxed (
+ &rwlock->__data.__readers, &r,
+ r - (1 << PTHREAD_RWLOCK_READER_SHIFT)))
+ return err;
+ /* TODO Back-off. */
+ }
+ /* Use the acquire MO fence to mirror the steps taken in the
+ non-timeout case. Note that the read can happen both
+ in the atomic_load above as well as in the failure case
+ of the CAS operation. */
+ atomic_thread_fence_acquire ();
+ /* We still need to wait for explicit hand-over, but we must
+ not use futex_wait anymore because we would just time out
+ in this case and thus make the spin-waiting we need
+ unnecessarily expensive. */
+ while ((atomic_load_relaxed (&rwlock->__data.__wrphase_futex)
+ | PTHREAD_RWLOCK_FUTEX_USED)
+ == (1 | PTHREAD_RWLOCK_FUTEX_USED))
+ {
+ /* TODO Back-off? */
+ }
+ ready = true;
+ break;
+ }
+ /* If we got interrupted (EINTR) or the futex word does not have the
+ expected value (EAGAIN), retry. */
+ }
if (ready)
- /* See below. */
- break;
+ /* See below. */
+ break;
/* We need acquire MO here so that we synchronize with the lock
- release of the writer, and so that we observe a recent value of
- __wrphase_futex (see below). */
+ release of the writer, and so that we observe a recent value of
+ __wrphase_futex (see below). */
if ((atomic_load_acquire (&rwlock->__data.__readers)
- & PTHREAD_RWLOCK_WRPHASE) == 0)
- /* We are in a read phase now, so the least recent modification of
- __wrphase_futex we can read from is the store by the writer
- with value 1. Thus, only now we can assume that if we observe
- a value of 0, explicit hand-over is finished. Retry the loop
- above one more time. */
- ready = true;
+ & PTHREAD_RWLOCK_WRPHASE)
+ == 0)
+ /* We are in a read phase now, so the least recent modification of
+ __wrphase_futex we can read from is the store by the writer
+ with value 1. Thus, only now we can assume that if we observe
+ a value of 0, explicit hand-over is finished. Retry the loop
+ above one more time. */
+ ready = true;
}
return 0;
}
-
static __always_inline void
__pthread_rwlock_wrunlock (pthread_rwlock_t *rwlock)
{
@@ -532,25 +533,27 @@ __pthread_rwlock_wrunlock (pthread_rwlock_t *rwlock)
/* Disable waiting by writers. We will wake up after we decided how to
proceed. */
bool wake_writers
- = ((atomic_exchange_relaxed (&rwlock->__data.__writers_futex, 0)
- & PTHREAD_RWLOCK_FUTEX_USED) != 0);
+ = ((atomic_exchange_relaxed (&rwlock->__data.__writers_futex, 0)
+ & PTHREAD_RWLOCK_FUTEX_USED)
+ != 0);
if (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP)
{
/* First, try to hand over to another writer. */
unsigned int w = atomic_load_relaxed (&rwlock->__data.__writers);
while (w != 0)
- {
- /* Release MO so that another writer that gets WRLOCKED from us will
- synchronize with us and thus can take over our view of
- __readers (including, for example, whether we are in a write
- phase or not). */
- if (atomic_compare_exchange_weak_release
- (&rwlock->__data.__writers, &w, w | PTHREAD_RWLOCK_WRHANDOVER))
- /* Another writer will take over. */
- goto done;
- /* TODO Back-off. */
- }
+ {
+ /* Release MO so that another writer that gets WRLOCKED from us will
+ synchronize with us and thus can take over our view of
+ __readers (including, for example, whether we are in a write
+ phase or not). */
+ if (atomic_compare_exchange_weak_release (
+ &rwlock->__data.__writers, &w,
+ w | PTHREAD_RWLOCK_WRHANDOVER))
+ /* Another writer will take over. */
+ goto done;
+ /* TODO Back-off. */
+ }
}
/* We have done everything we needed to do to prefer writers, so now we
@@ -558,32 +561,32 @@ __pthread_rwlock_wrunlock (pthread_rwlock_t *rwlock)
stay in a write phase. See pthread_rwlock_rdunlock for more details. */
unsigned int r = atomic_load_relaxed (&rwlock->__data.__readers);
/* Release MO so that subsequent readers or writers synchronize with us. */
- while (!atomic_compare_exchange_weak_release
- (&rwlock->__data.__readers, &r,
- ((r ^ PTHREAD_RWLOCK_WRLOCKED)
- ^ ((r >> PTHREAD_RWLOCK_READER_SHIFT) == 0 ? 0
- : PTHREAD_RWLOCK_WRPHASE))))
+ while (!atomic_compare_exchange_weak_release (
+ &rwlock->__data.__readers, &r,
+ ((r ^ PTHREAD_RWLOCK_WRLOCKED)
+ ^ ((r >> PTHREAD_RWLOCK_READER_SHIFT) == 0 ? 0
+ : PTHREAD_RWLOCK_WRPHASE))))
{
/* TODO Back-off. */
}
if ((r >> PTHREAD_RWLOCK_READER_SHIFT) != 0)
{
/* We must hand over explicitly through __wrphase_futex. Relaxed MO is
- sufficient because it is just used to delay acquisition by a writer;
- any other synchronizes-with relations that are necessary are
- established through __readers. */
+ sufficient because it is just used to delay acquisition by a writer;
+ any other synchronizes-with relations that are necessary are
+ established through __readers. */
if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0)
- & PTHREAD_RWLOCK_FUTEX_USED) != 0)
- futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
+ & PTHREAD_RWLOCK_FUTEX_USED)
+ != 0)
+ futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
}
- done:
+done:
/* We released WRLOCKED in some way, so wake a writer. */
if (wake_writers)
futex_wake (&rwlock->__data.__writers_futex, 1, private);
}
-
static __always_inline int
__pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
const struct __timespec64 *abstime)
@@ -594,14 +597,15 @@ __pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
requires that "the validity of the abstime parameter need not be checked
if the lock can be immediately acquired" (i.e., we need not but may check
it). */
- if (abstime && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid)
- || ! valid_nanoseconds (abstime->tv_nsec)))
+ if (abstime
+ && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid)
+ || !valid_nanoseconds (abstime->tv_nsec)))
return EINVAL;
/* Make sure we are not holding the rwlock as a writer. This is a deadlock
situation we recognize and report. */
if (__glibc_unlikely (atomic_load_relaxed (&rwlock->__data.__cur_writer)
- == THREAD_GETMEM (THREAD_SELF, tid)))
+ == THREAD_GETMEM (THREAD_SELF, tid)))
return EDEADLK;
/* First we try to acquire the role of primary writer by setting WRLOCKED;
@@ -620,154 +624,155 @@ __pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
this could be less scalable if readers arrive and leave frequently. */
bool may_share_futex_used_flag = false;
unsigned int r = atomic_fetch_or_acquire (&rwlock->__data.__readers,
- PTHREAD_RWLOCK_WRLOCKED);
+ PTHREAD_RWLOCK_WRLOCKED);
if (__glibc_unlikely ((r & PTHREAD_RWLOCK_WRLOCKED) != 0))
{
/* There is another primary writer. */
bool prefer_writer
- = (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP);
+ = (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP);
if (prefer_writer)
- {
- /* We register as a waiting writer, so that we can make use of
- writer--writer hand-over. Relaxed MO is fine because we just
- want to register. We assume that the maximum number of threads
- is less than the capacity in __writers. */
- atomic_fetch_add_relaxed (&rwlock->__data.__writers, 1);
- }
+ {
+ /* We register as a waiting writer, so that we can make use of
+ writer--writer hand-over. Relaxed MO is fine because we just
+ want to register. We assume that the maximum number of threads
+ is less than the capacity in __writers. */
+ atomic_fetch_add_relaxed (&rwlock->__data.__writers, 1);
+ }
for (;;)
- {
- /* TODO Spin until WRLOCKED is 0 before trying the CAS below.
- But pay attention to not delay trying writer--writer hand-over
- for too long (which we must try eventually anyway). */
- if ((r & PTHREAD_RWLOCK_WRLOCKED) == 0)
- {
- /* Try to become the primary writer or retry. Acquire MO as in
- the fetch_or above. */
- if (atomic_compare_exchange_weak_acquire
- (&rwlock->__data.__readers, &r, r | PTHREAD_RWLOCK_WRLOCKED))
- {
- if (prefer_writer)
- {
- /* Unregister as a waiting writer. Note that because we
- acquired WRLOCKED, WRHANDOVER will not be set.
- Acquire MO on the CAS above ensures that
- unregistering happens after the previous writer;
- this sorts the accesses to __writers by all
- primary writers in a useful way (e.g., any other
- primary writer acquiring after us or getting it from
- us through WRHANDOVER will see both our changes to
- __writers).
- ??? Perhaps this is not strictly necessary for
- reasons we do not yet know of. */
- atomic_fetch_add_relaxed (&rwlock->__data.__writers, -1);
- }
- break;
- }
- /* Retry if the CAS fails (r will have been updated). */
- continue;
- }
- /* If writer--writer hand-over is available, try to become the
- primary writer this way by grabbing the WRHANDOVER token. If we
- succeed, we own WRLOCKED. */
- if (prefer_writer)
- {
- unsigned int w = atomic_load_relaxed (&rwlock->__data.__writers);
- if ((w & PTHREAD_RWLOCK_WRHANDOVER) != 0)
- {
- /* Acquire MO is required here so that we synchronize with
- the writer that handed over WRLOCKED. We also need this
- for the reload of __readers below because our view of
- __readers must be at least as recent as the view of the
- writer that handed over WRLOCKED; we must avoid an ABA
- through WRHANDOVER, which could, for example, lead to us
- assuming we are still in a write phase when in fact we
- are not. */
- if (atomic_compare_exchange_weak_acquire
- (&rwlock->__data.__writers,
- &w, (w - PTHREAD_RWLOCK_WRHANDOVER - 1)))
- {
- /* Reload so our view is consistent with the view of
- the previous owner of WRLOCKED. See above. */
- r = atomic_load_relaxed (&rwlock->__data.__readers);
- break;
- }
- /* We do not need to reload __readers here. We should try
- to perform writer--writer hand-over if possible; if it
- is not possible anymore, we will reload __readers
- elsewhere in this loop. */
- continue;
- }
- }
- /* We did not acquire WRLOCKED nor were able to use writer--writer
- hand-over, so we block on __writers_futex. */
- int private = __pthread_rwlock_get_private (rwlock);
- unsigned int wf
- = atomic_load_relaxed (&rwlock->__data.__writers_futex);
- if (((wf & ~(unsigned int) PTHREAD_RWLOCK_FUTEX_USED) != 1)
- || ((wf != (1 | PTHREAD_RWLOCK_FUTEX_USED))
- && (!atomic_compare_exchange_weak_relaxed
- (&rwlock->__data.__writers_futex, &wf,
- 1 | PTHREAD_RWLOCK_FUTEX_USED))))
- {
- /* If we cannot block on __writers_futex because there is no
- primary writer, or we cannot set PTHREAD_RWLOCK_FUTEX_USED,
- we retry. We must reload __readers here in case we cannot
- block on __writers_futex so that we can become the primary
- writer and are not stuck in a loop that just continuously
- fails to block on __writers_futex. */
- r = atomic_load_relaxed (&rwlock->__data.__readers);
- continue;
- }
- /* We set the flag that signals that the futex is used, or we could
- have set it if we had been faster than other waiters. As a
- result, we may share the flag with an unknown number of other
- writers. Therefore, we must keep this flag set when we acquire
- the lock. We do not need to do this when we do not reach this
- point here because then we are not part of the group that may
- share the flag, and another writer will wake one of the writers
- in this group. */
- may_share_futex_used_flag = true;
- int err = __futex_abstimed_wait64 (&rwlock->__data.__writers_futex,
- 1 | PTHREAD_RWLOCK_FUTEX_USED,
- clockid, abstime, private);
- if (err == ETIMEDOUT || err == EOVERFLOW)
- {
- if (prefer_writer)
- {
- /* We need to unregister as a waiting writer. If we are the
- last writer and writer--writer hand-over is available,
- we must make use of it because nobody else will reset
- WRLOCKED otherwise. (If we use it, we simply pretend
- that this happened before the timeout; see
- pthread_rwlock_rdlock_full for the full reasoning.)
- Also see the similar code above. */
- unsigned int w
- = atomic_load_relaxed (&rwlock->__data.__writers);
- while (!atomic_compare_exchange_weak_acquire
- (&rwlock->__data.__writers, &w,
- (w == PTHREAD_RWLOCK_WRHANDOVER + 1 ? 0 : w - 1)))
- {
- /* TODO Back-off. */
- }
- if (w == PTHREAD_RWLOCK_WRHANDOVER + 1)
- {
- /* We must continue as primary writer. See above. */
- r = atomic_load_relaxed (&rwlock->__data.__readers);
- break;
- }
- }
- /* We cleaned up and cannot have stolen another waiting writer's
- futex wake-up, so just return. */
- return err;
- }
- /* If we got interrupted (EINTR) or the futex word does not have the
- expected value (EAGAIN), retry after reloading __readers. */
- r = atomic_load_relaxed (&rwlock->__data.__readers);
- }
+ {
+ /* TODO Spin until WRLOCKED is 0 before trying the CAS below.
+ But pay attention to not delay trying writer--writer hand-over
+ for too long (which we must try eventually anyway). */
+ if ((r & PTHREAD_RWLOCK_WRLOCKED) == 0)
+ {
+ /* Try to become the primary writer or retry. Acquire MO as in
+ the fetch_or above. */
+ if (atomic_compare_exchange_weak_acquire (
+ &rwlock->__data.__readers, &r,
+ r | PTHREAD_RWLOCK_WRLOCKED))
+ {
+ if (prefer_writer)
+ {
+ /* Unregister as a waiting writer. Note that because we
+ acquired WRLOCKED, WRHANDOVER will not be set.
+ Acquire MO on the CAS above ensures that
+ unregistering happens after the previous writer;
+ this sorts the accesses to __writers by all
+ primary writers in a useful way (e.g., any other
+ primary writer acquiring after us or getting it from
+ us through WRHANDOVER will see both our changes to
+ __writers).
+ ??? Perhaps this is not strictly necessary for
+ reasons we do not yet know of. */
+ atomic_fetch_add_relaxed (&rwlock->__data.__writers, -1);
+ }
+ break;
+ }
+ /* Retry if the CAS fails (r will have been updated). */
+ continue;
+ }
+ /* If writer--writer hand-over is available, try to become the
+ primary writer this way by grabbing the WRHANDOVER token. If we
+ succeed, we own WRLOCKED. */
+ if (prefer_writer)
+ {
+ unsigned int w = atomic_load_relaxed (&rwlock->__data.__writers);
+ if ((w & PTHREAD_RWLOCK_WRHANDOVER) != 0)
+ {
+ /* Acquire MO is required here so that we synchronize with
+ the writer that handed over WRLOCKED. We also need this
+ for the reload of __readers below because our view of
+ __readers must be at least as recent as the view of the
+ writer that handed over WRLOCKED; we must avoid an ABA
+ through WRHANDOVER, which could, for example, lead to us
+ assuming we are still in a write phase when in fact we
+ are not. */
+ if (atomic_compare_exchange_weak_acquire (
+ &rwlock->__data.__writers, &w,
+ (w - PTHREAD_RWLOCK_WRHANDOVER - 1)))
+ {
+ /* Reload so our view is consistent with the view of
+ the previous owner of WRLOCKED. See above. */
+ r = atomic_load_relaxed (&rwlock->__data.__readers);
+ break;
+ }
+ /* We do not need to reload __readers here. We should try
+ to perform writer--writer hand-over if possible; if it
+ is not possible anymore, we will reload __readers
+ elsewhere in this loop. */
+ continue;
+ }
+ }
+ /* We did not acquire WRLOCKED nor were able to use writer--writer
+ hand-over, so we block on __writers_futex. */
+ int private = __pthread_rwlock_get_private (rwlock);
+ unsigned int wf
+ = atomic_load_relaxed (&rwlock->__data.__writers_futex);
+ if (((wf & ~(unsigned int) PTHREAD_RWLOCK_FUTEX_USED) != 1)
+ || ((wf != (1 | PTHREAD_RWLOCK_FUTEX_USED))
+ && (!atomic_compare_exchange_weak_relaxed (
+ &rwlock->__data.__writers_futex, &wf,
+ 1 | PTHREAD_RWLOCK_FUTEX_USED))))
+ {
+ /* If we cannot block on __writers_futex because there is no
+ primary writer, or we cannot set PTHREAD_RWLOCK_FUTEX_USED,
+ we retry. We must reload __readers here in case we cannot
+ block on __writers_futex so that we can become the primary
+ writer and are not stuck in a loop that just continuously
+ fails to block on __writers_futex. */
+ r = atomic_load_relaxed (&rwlock->__data.__readers);
+ continue;
+ }
+ /* We set the flag that signals that the futex is used, or we could
+ have set it if we had been faster than other waiters. As a
+ result, we may share the flag with an unknown number of other
+ writers. Therefore, we must keep this flag set when we acquire
+ the lock. We do not need to do this when we do not reach this
+ point here because then we are not part of the group that may
+ share the flag, and another writer will wake one of the writers
+ in this group. */
+ may_share_futex_used_flag = true;
+ int err = __futex_abstimed_wait64 (&rwlock->__data.__writers_futex,
+ 1 | PTHREAD_RWLOCK_FUTEX_USED,
+ clockid, abstime, private);
+ if (err == ETIMEDOUT || err == EOVERFLOW)
+ {
+ if (prefer_writer)
+ {
+ /* We need to unregister as a waiting writer. If we are the
+ last writer and writer--writer hand-over is available,
+ we must make use of it because nobody else will reset
+ WRLOCKED otherwise. (If we use it, we simply pretend
+ that this happened before the timeout; see
+ pthread_rwlock_rdlock_full for the full reasoning.)
+ Also see the similar code above. */
+ unsigned int w
+ = atomic_load_relaxed (&rwlock->__data.__writers);
+ while (!atomic_compare_exchange_weak_acquire (
+ &rwlock->__data.__writers, &w,
+ (w == PTHREAD_RWLOCK_WRHANDOVER + 1 ? 0 : w - 1)))
+ {
+ /* TODO Back-off. */
+ }
+ if (w == PTHREAD_RWLOCK_WRHANDOVER + 1)
+ {
+ /* We must continue as primary writer. See above. */
+ r = atomic_load_relaxed (&rwlock->__data.__readers);
+ break;
+ }
+ }
+ /* We cleaned up and cannot have stolen another waiting writer's
+ futex wake-up, so just return. */
+ return err;
+ }
+ /* If we got interrupted (EINTR) or the futex word does not have the
+ expected value (EAGAIN), retry after reloading __readers. */
+ r = atomic_load_relaxed (&rwlock->__data.__readers);
+ }
/* Our snapshot of __readers is up-to-date at this point because we
- either set WRLOCKED using a CAS (and update r accordingly below,
- which was used as expected value for the CAS) or got WRLOCKED from
- another writer whose snapshot of __readers we inherit. */
+ either set WRLOCKED using a CAS (and update r accordingly below,
+ which was used as expected value for the CAS) or got WRLOCKED from
+ another writer whose snapshot of __readers we inherit. */
r |= PTHREAD_RWLOCK_WRLOCKED;
}
@@ -775,9 +780,9 @@ __pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
MO is sufficient for futex words; acquire MO on the previous
modifications of __readers ensures that this store happens after the
store of value 0 by the previous primary writer. */
- atomic_store_relaxed (&rwlock->__data.__writers_futex,
- 1 | (may_share_futex_used_flag
- ? PTHREAD_RWLOCK_FUTEX_USED : 0));
+ atomic_store_relaxed (
+ &rwlock->__data.__writers_futex,
+ 1 | (may_share_futex_used_flag ? PTHREAD_RWLOCK_FUTEX_USED : 0));
/* If we are in a write phase, we have acquired the lock. */
if ((r & PTHREAD_RWLOCK_WRPHASE) != 0)
@@ -786,23 +791,23 @@ __pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
/* If we are in a read phase and there are no readers, try to start a write
phase. */
while ((r & PTHREAD_RWLOCK_WRPHASE) == 0
- && (r >> PTHREAD_RWLOCK_READER_SHIFT) == 0)
+ && (r >> PTHREAD_RWLOCK_READER_SHIFT) == 0)
{
/* Acquire MO so that we synchronize with prior writers and do
- not interfere with their updates to __writers_futex, as well
- as regarding prior readers and their updates to __wrphase_futex,
- respectively. */
- if (atomic_compare_exchange_weak_acquire (&rwlock->__data.__readers,
- &r, r | PTHREAD_RWLOCK_WRPHASE))
- {
- /* We have started a write phase, so need to enable readers to wait.
- See the similar case in __pthread_rwlock_rdlock_full. Unlike in
- that similar case, we are the (only) primary writer and so do
- not need to wake another writer. */
- atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 1);
-
- goto done;
- }
+ not interfere with their updates to __writers_futex, as well
+ as regarding prior readers and their updates to __wrphase_futex,
+ respectively. */
+ if (atomic_compare_exchange_weak_acquire (&rwlock->__data.__readers, &r,
+ r | PTHREAD_RWLOCK_WRPHASE))
+ {
+ /* We have started a write phase, so need to enable readers to wait.
+ See the similar case in __pthread_rwlock_rdlock_full. Unlike in
+ that similar case, we are the (only) primary writer and so do
+ not need to wake another writer. */
+ atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 1);
+
+ goto done;
+ }
/* TODO Back-off. */
}
@@ -818,133 +823,136 @@ __pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
for (;;)
{
while (((wpf = atomic_load_relaxed (&rwlock->__data.__wrphase_futex))
- | PTHREAD_RWLOCK_FUTEX_USED) == PTHREAD_RWLOCK_FUTEX_USED)
- {
- int private = __pthread_rwlock_get_private (rwlock);
- if ((wpf & PTHREAD_RWLOCK_FUTEX_USED) == 0
- && (!atomic_compare_exchange_weak_relaxed
- (&rwlock->__data.__wrphase_futex, &wpf,
- PTHREAD_RWLOCK_FUTEX_USED)))
- continue;
- int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
- PTHREAD_RWLOCK_FUTEX_USED,
- clockid, abstime, private);
- if (err == ETIMEDOUT || err == EOVERFLOW)
- {
- if (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP)
- {
- /* We try writer--writer hand-over. */
- unsigned int w
- = atomic_load_relaxed (&rwlock->__data.__writers);
- if (w != 0)
- {
- /* We are about to hand over WRLOCKED, so we must
- release __writers_futex too; otherwise, we'd have
- a pending store, which could at least prevent
- other threads from waiting using the futex
- because it could interleave with the stores
- by subsequent writers. In turn, this means that
- we have to clean up when we do not hand over
- WRLOCKED.
- Release MO so that another writer that gets
- WRLOCKED from us can take over our view of
- __readers. */
- unsigned int wf
- = atomic_exchange_relaxed (&rwlock->__data.__writers_futex, 0);
- while (w != 0)
- {
- if (atomic_compare_exchange_weak_release
- (&rwlock->__data.__writers, &w,
- w | PTHREAD_RWLOCK_WRHANDOVER))
- {
- /* Wake other writers. */
- if ((wf & PTHREAD_RWLOCK_FUTEX_USED) != 0)
- futex_wake (&rwlock->__data.__writers_futex,
- 1, private);
- return err;
- }
- /* TODO Back-off. */
- }
- /* We still own WRLOCKED and someone else might set
- a write phase concurrently, so enable waiting
- again. Make sure we don't loose the flag that
- signals whether there are threads waiting on
- this futex. */
- atomic_store_relaxed (&rwlock->__data.__writers_futex, wf);
- }
- }
- /* If we timed out and we are not in a write phase, we can
- just stop being a primary writer. Otherwise, we just
- acquire the lock. */
- r = atomic_load_relaxed (&rwlock->__data.__readers);
- if ((r & PTHREAD_RWLOCK_WRPHASE) == 0)
- {
- /* We are about to release WRLOCKED, so we must release
- __writers_futex too; see the handling of
- writer--writer hand-over above. */
- unsigned int wf
- = atomic_exchange_relaxed (&rwlock->__data.__writers_futex, 0);
- while ((r & PTHREAD_RWLOCK_WRPHASE) == 0)
- {
- /* While we don't need to make anything from a
- caller's critical section visible to other
- threads, we need to ensure that our changes to
- __writers_futex are properly ordered.
- Therefore, use release MO to synchronize with
- subsequent primary writers. Also wake up any
- waiting readers as they are waiting because of
- us. */
- if (atomic_compare_exchange_weak_release
- (&rwlock->__data.__readers, &r,
- (r ^ PTHREAD_RWLOCK_WRLOCKED)
- & ~(unsigned int) PTHREAD_RWLOCK_RWAITING))
- {
- /* Wake other writers. */
- if ((wf & PTHREAD_RWLOCK_FUTEX_USED) != 0)
- futex_wake (&rwlock->__data.__writers_futex,
- 1, private);
- /* Wake waiting readers. */
- if ((r & PTHREAD_RWLOCK_RWAITING) != 0)
- futex_wake (&rwlock->__data.__readers,
- INT_MAX, private);
- return ETIMEDOUT;
- }
- }
- /* We still own WRLOCKED and someone else might set a
- write phase concurrently, so enable waiting again.
- Make sure we don't loose the flag that signals
- whether there are threads waiting on this futex. */
- atomic_store_relaxed (&rwlock->__data.__writers_futex, wf);
- }
- /* Use the acquire MO fence to mirror the steps taken in the
- non-timeout case. Note that the read can happen both
- in the atomic_load above as well as in the failure case
- of the CAS operation. */
- atomic_thread_fence_acquire ();
- /* We still need to wait for explicit hand-over, but we must
- not use futex_wait anymore. */
- while ((atomic_load_relaxed (&rwlock->__data.__wrphase_futex)
- | PTHREAD_RWLOCK_FUTEX_USED)
- == PTHREAD_RWLOCK_FUTEX_USED)
- {
- /* TODO Back-off. */
- }
- ready = true;
- break;
- }
- /* If we got interrupted (EINTR) or the futex word does not have
- the expected value (EAGAIN), retry. */
- }
+ | PTHREAD_RWLOCK_FUTEX_USED)
+ == PTHREAD_RWLOCK_FUTEX_USED)
+ {
+ int private = __pthread_rwlock_get_private (rwlock);
+ if ((wpf & PTHREAD_RWLOCK_FUTEX_USED) == 0
+ && (!atomic_compare_exchange_weak_relaxed (
+ &rwlock->__data.__wrphase_futex, &wpf,
+ PTHREAD_RWLOCK_FUTEX_USED)))
+ continue;
+ int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
+ PTHREAD_RWLOCK_FUTEX_USED,
+ clockid, abstime, private);
+ if (err == ETIMEDOUT || err == EOVERFLOW)
+ {
+ if (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP)
+ {
+ /* We try writer--writer hand-over. */
+ unsigned int w
+ = atomic_load_relaxed (&rwlock->__data.__writers);
+ if (w != 0)
+ {
+ /* We are about to hand over WRLOCKED, so we must
+ release __writers_futex too; otherwise, we'd have
+ a pending store, which could at least prevent
+ other threads from waiting using the futex
+ because it could interleave with the stores
+ by subsequent writers. In turn, this means that
+ we have to clean up when we do not hand over
+ WRLOCKED.
+ Release MO so that another writer that gets
+ WRLOCKED from us can take over our view of
+ __readers. */
+ unsigned int wf = atomic_exchange_relaxed (
+ &rwlock->__data.__writers_futex, 0);
+ while (w != 0)
+ {
+ if (atomic_compare_exchange_weak_release (
+ &rwlock->__data.__writers, &w,
+ w | PTHREAD_RWLOCK_WRHANDOVER))
+ {
+ /* Wake other writers. */
+ if ((wf & PTHREAD_RWLOCK_FUTEX_USED) != 0)
+ futex_wake (&rwlock->__data.__writers_futex, 1,
+ private);
+ return err;
+ }
+ /* TODO Back-off. */
+ }
+ /* We still own WRLOCKED and someone else might set
+ a write phase concurrently, so enable waiting
+ again. Make sure we don't loose the flag that
+ signals whether there are threads waiting on
+ this futex. */
+ atomic_store_relaxed (&rwlock->__data.__writers_futex,
+ wf);
+ }
+ }
+ /* If we timed out and we are not in a write phase, we can
+ just stop being a primary writer. Otherwise, we just
+ acquire the lock. */
+ r = atomic_load_relaxed (&rwlock->__data.__readers);
+ if ((r & PTHREAD_RWLOCK_WRPHASE) == 0)
+ {
+ /* We are about to release WRLOCKED, so we must release
+ __writers_futex too; see the handling of
+ writer--writer hand-over above. */
+ unsigned int wf = atomic_exchange_relaxed (
+ &rwlock->__data.__writers_futex, 0);
+ while ((r & PTHREAD_RWLOCK_WRPHASE) == 0)
+ {
+ /* While we don't need to make anything from a
+ caller's critical section visible to other
+ threads, we need to ensure that our changes to
+ __writers_futex are properly ordered.
+ Therefore, use release MO to synchronize with
+ subsequent primary writers. Also wake up any
+ waiting readers as they are waiting because of
+ us. */
+ if (atomic_compare_exchange_weak_release (
+ &rwlock->__data.__readers, &r,
+ (r ^ PTHREAD_RWLOCK_WRLOCKED)
+ & ~(unsigned int) PTHREAD_RWLOCK_RWAITING))
+ {
+ /* Wake other writers. */
+ if ((wf & PTHREAD_RWLOCK_FUTEX_USED) != 0)
+ futex_wake (&rwlock->__data.__writers_futex, 1,
+ private);
+ /* Wake waiting readers. */
+ if ((r & PTHREAD_RWLOCK_RWAITING) != 0)
+ futex_wake (&rwlock->__data.__readers, INT_MAX,
+ private);
+ return ETIMEDOUT;
+ }
+ }
+ /* We still own WRLOCKED and someone else might set a
+ write phase concurrently, so enable waiting again.
+ Make sure we don't loose the flag that signals
+ whether there are threads waiting on this futex. */
+ atomic_store_relaxed (&rwlock->__data.__writers_futex, wf);
+ }
+ /* Use the acquire MO fence to mirror the steps taken in the
+ non-timeout case. Note that the read can happen both
+ in the atomic_load above as well as in the failure case
+ of the CAS operation. */
+ atomic_thread_fence_acquire ();
+ /* We still need to wait for explicit hand-over, but we must
+ not use futex_wait anymore. */
+ while ((atomic_load_relaxed (&rwlock->__data.__wrphase_futex)
+ | PTHREAD_RWLOCK_FUTEX_USED)
+ == PTHREAD_RWLOCK_FUTEX_USED)
+ {
+ /* TODO Back-off. */
+ }
+ ready = true;
+ break;
+ }
+ /* If we got interrupted (EINTR) or the futex word does not have
+ the expected value (EAGAIN), retry. */
+ }
/* See pthread_rwlock_rdlock_full. */
if (ready)
- break;
+ break;
if ((atomic_load_acquire (&rwlock->__data.__readers)
- & PTHREAD_RWLOCK_WRPHASE) != 0)
- ready = true;
+ & PTHREAD_RWLOCK_WRPHASE)
+ != 0)
+ ready = true;
}
- done:
+done:
atomic_store_relaxed (&rwlock->__data.__cur_writer,
- THREAD_GETMEM (THREAD_SELF, tid));
+ THREAD_GETMEM (THREAD_SELF, tid));
return 0;
}
--
2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 19:30 ` Noah Goldstein
@ 2022-03-30 19:47 ` Joseph Myers
2022-03-30 20:11 ` Noah Goldstein
2022-03-30 20:16 ` Noah Goldstein
0 siblings, 2 replies; 24+ messages in thread
From: Joseph Myers @ 2022-03-30 19:47 UTC (permalink / raw)
To: Noah Goldstein; +Cc: Fangrui Song, GNU C Library
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.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] Add .clang-format style file
2022-03-30 19:27 ` [PATCH v3] " Noah Goldstein
@ 2022-03-30 19:59 ` Fangrui Song
2022-03-30 20:01 ` Florian Weimer
1 sibling, 0 replies; 24+ messages in thread
From: Fangrui Song @ 2022-03-30 19:59 UTC (permalink / raw)
To: Noah Goldstein; +Cc: libc-alpha
On 2022-03-30, Noah Goldstein via Libc-alpha wrote:
>Went with version >= 11.0 since it covers most of the major features
>and should be pretty universally accessibly.
>
>There are some issues:
>
>1. 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. i.e:
> Desired:
> ```
> #ifndef A
> # define B
> #endif
> ```
> Options:
> ```
> #ifndef A
> # define B /* Two spaces instead of one. */
> #endif
>
> #ifndef C
> #define D /* No spaces. */
> #endif
> ```
> 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.
>
>2. concatenation of lines missing semi-colons:
> Throughout glibc there are macros used to setup aliasing that are
> outside of functions and don't end in semi-colons i.e:
> ```
> libc_hidden_def (__pthread_self)
> weak_alias (__pthread_self, pthread_self)
> ```
>
> clang-format reformats lines like these to:
> ```
> libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> ```
>
> which is generally undesirable.
>
>Other than those two big concerns there are certainly some questions
>diffs but for the most part it creates a easy to read and consistent
>style.
Thanks. I have spot checked a few files and having the file will be
helpful.
Side note: clang-format works even without .clang-format and falls back to LLVM
style. Having the file makes the format rules actually reflect the glibc coding
standard.
Reviewed-by: Fangrui Song <maskray@google.com>
>---
> .clang-format | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 175 insertions(+)
> create mode 100644 .clang-format
>
>diff --git a/.clang-format b/.clang-format
>new file mode 100644
>index 0000000000..fcb344ef70
>--- /dev/null
>+++ b/.clang-format
>@@ -0,0 +1,175 @@
>+# 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.
>+#
>+# To format the current git diff inplace (-i) the follow command can
>+# be used:
>+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
>+#
>+# To just view the diff clang-format would generate:
>+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
>+#
>+# NB: clang-format-diff, along with other clang-format related tools,
>+# can be found at: /path/to/llvm-project/clang/tools/clang-format/
>+#
The ability to re-format blocks of code is especially useful for
contributors sending patches. Thanks for highlighting it.
>+#
>+---
>+# 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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] Add .clang-format style file
2022-03-30 19:27 ` [PATCH v3] " Noah Goldstein
2022-03-30 19:59 ` Fangrui Song
@ 2022-03-30 20:01 ` Florian Weimer
2022-03-30 20:09 ` Noah Goldstein
1 sibling, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2022-03-30 20:01 UTC (permalink / raw)
To: Noah Goldstein via Libc-alpha
* Noah Goldstein via Libc-alpha:
> Went with version >= 11.0 since it covers most of the major features
> and should be pretty universally accessibly.
>
> There are some issues:
>
> 1. 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. i.e:
> Desired:
> ```
> #ifndef A
> # define B
> #endif
> ```
> Options:
> ```
> #ifndef A
> # define B /* Two spaces instead of one. */
> #endif
>
> #ifndef C
> #define D /* No spaces. */
> #endif
> ```
> 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.
>
> 2. concatenation of lines missing semi-colons:
> Throughout glibc there are macros used to setup aliasing that are
> outside of functions and don't end in semi-colons i.e:
> ```
> libc_hidden_def (__pthread_self)
> weak_alias (__pthread_self, pthread_self)
> ```
>
> clang-format reformats lines like these to:
> ```
> libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> ```
>
> which is generally undesirable.
>
> Other than those two big concerns there are certainly some questions
> diffs but for the most part it creates a easy to read and consistent
> style.
> ---
> .clang-format | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 175 insertions(+)
> create mode 100644 .clang-format
>
> diff --git a/.clang-format b/.clang-format
> new file mode 100644
> index 0000000000..fcb344ef70
> --- /dev/null
> +++ b/.clang-format
> @@ -0,0 +1,175 @@
> +# 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
That's not a valid path in the glibc source tree.
> +# BasedOnStyle: GNU
Why is this commented out?
> +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
These regular expression do not seem to be relevant to glibc.
> +UseTab: Never
I prefer not using tabs, but others disagree, and most of the source
files use tabs instead of 8 spaces (and not just for indentation).
> +ForEachMacros:
> + - 'FOR_EACH_IMPL'
> + - 'list_for_each'
> + - 'list_for_each_prev'
> + - 'list_for_each_prev_safe'
If this is a YAML file, do these duplicate FoEachMacros keys have any
effect? Only the second one is relevant to glibc.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] Add .clang-format style file
2022-03-30 20:01 ` Florian Weimer
@ 2022-03-30 20:09 ` Noah Goldstein
2022-03-30 20:14 ` Florian Weimer
0 siblings, 1 reply; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 20:09 UTC (permalink / raw)
To: Florian Weimer; +Cc: Noah Goldstein via Libc-alpha
On Wed, Mar 30, 2022 at 3:01 PM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Noah Goldstein via Libc-alpha:
>
> > Went with version >= 11.0 since it covers most of the major features
> > and should be pretty universally accessibly.
> >
> > There are some issues:
> >
> > 1. 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. i.e:
> > Desired:
> > ```
> > #ifndef A
> > # define B
> > #endif
> > ```
> > Options:
> > ```
> > #ifndef A
> > # define B /* Two spaces instead of one. */
> > #endif
> >
> > #ifndef C
> > #define D /* No spaces. */
> > #endif
> > ```
> > 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.
> >
> > 2. concatenation of lines missing semi-colons:
> > Throughout glibc there are macros used to setup aliasing that are
> > outside of functions and don't end in semi-colons i.e:
> > ```
> > libc_hidden_def (__pthread_self)
> > weak_alias (__pthread_self, pthread_self)
> > ```
> >
> > clang-format reformats lines like these to:
> > ```
> > libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> > ```
> >
> > which is generally undesirable.
> >
> > Other than those two big concerns there are certainly some questions
> > diffs but for the most part it creates a easy to read and consistent
> > style.
> > ---
> > .clang-format | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 175 insertions(+)
> > create mode 100644 .clang-format
> >
> > diff --git a/.clang-format b/.clang-format
> > new file mode 100644
> > index 0000000000..fcb344ef70
> > --- /dev/null
> > +++ b/.clang-format
> > @@ -0,0 +1,175 @@
> > +# 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
>
> That's not a valid path in the glibc source tree.
>
> > +# BasedOnStyle: GNU
>
> Why is this commented out?
>
> > +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
>
> These regular expression do not seem to be relevant to glibc.
Yeah. Will fix in v4.
>
> > +UseTab: Never
>
> I prefer not using tabs, but others disagree, and most of the source
> files use tabs instead of 8 spaces (and not just for indentation).
+1 but yeah the consistent style seems to be tabs so will update in v4.
>
> > +ForEachMacros:
> > + - 'FOR_EACH_IMPL'
> > + - 'list_for_each'
> > + - 'list_for_each_prev'
> > + - 'list_for_each_prev_safe'
>
> If this is a YAML file, do these duplicate FoEachMacros keys have any
> effect? Only the second one is relevant to glibc.
They all seem to have an effect.
What do you mean only the second is relevant? I see all but `list_for_each_prev`
used and `list_for_each_prev` appears to be defined so it could be used
to make a loop.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4] Add .clang-format style file
2022-03-28 19:12 [PATCH v1] Add .clang-format style file Noah Goldstein
2022-03-30 0:26 ` Fangrui Song
2022-03-30 19:27 ` [PATCH v3] " Noah Goldstein
@ 2022-03-30 20:10 ` Noah Goldstein
2022-03-30 20:24 ` [PATCH v5] " Noah Goldstein
2022-04-04 16:56 ` [PATCH v6] " Noah Goldstein
4 siblings, 0 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 20:10 UTC (permalink / raw)
To: libc-alpha
Went with version >= 11.0 since it covers most of the major features
and should be pretty universally accessibly.
There are some issues:
1. 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. i.e:
Desired:
```
#ifndef A
# define B
#endif
```
Options:
```
#ifndef A
# define B /* Two spaces instead of one. */
#endif
#ifndef C
#define D /* No spaces. */
#endif
```
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.
2. concatenation of lines missing semi-colons:
Throughout glibc there are macros used to setup aliasing that are
outside of functions and don't end in semi-colons i.e:
```
libc_hidden_def (__pthread_self)
weak_alias (__pthread_self, pthread_self)
```
clang-format reformats lines like these to:
```
libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
```
which is generally undesirable.
Other than those two big concerns there are certainly some questions
diffs but for the most part it creates a easy to read and consistent
style.
---
.clang-format | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 171 insertions(+)
create mode 100644 .clang-format
diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..b1dc52aed8
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,171 @@
+# 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.
+#
+# To format the current git diff inplace (-i) the follow command can
+# be used:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
+#
+# To just view the diff clang-format would generate:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
+#
+# NB: clang-format-diff, along with other clang-format related tools,
+# can be found at: /path/to/llvm-project/clang/tools/clang-format/
+#
+#
+---
+# 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: '.*'
+ 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: Always
+ForEachMacros:
+ - 'FOR_EACH_IMPL'
+ - 'list_for_each'
+ - 'list_for_each_prev'
+ - 'list_for_each_prev_safe'
+...
--
2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 19:47 ` Joseph Myers
@ 2022-03-30 20:11 ` Noah Goldstein
2022-03-30 20:16 ` Noah Goldstein
1 sibling, 0 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 20:11 UTC (permalink / raw)
To: Joseph Myers; +Cc: Fangrui Song, GNU C Library
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] Add .clang-format style file
2022-03-30 20:09 ` Noah Goldstein
@ 2022-03-30 20:14 ` Florian Weimer
2022-03-30 20:21 ` Noah Goldstein
0 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2022-03-30 20:14 UTC (permalink / raw)
To: Noah Goldstein; +Cc: Noah Goldstein via Libc-alpha
* Noah Goldstein:
>> > +UseTab: Never
>>
>> I prefer not using tabs, but others disagree, and most of the source
>> files use tabs instead of 8 spaces (and not just for indentation).
>
> +1 but yeah the consistent style seems to be tabs so will update in v4.
Is there an option to use tabs only if the file contains tab?
>> > +ForEachMacros:
>> > + - 'FOR_EACH_IMPL'
>> > + - 'list_for_each'
>> > + - 'list_for_each_prev'
>> > + - 'list_for_each_prev_safe'
>>
>> If this is a YAML file, do these duplicate FoEachMacros keys have any
>> effect? Only the second one is relevant to glibc.
>
> They all seem to have an effect.
<https://yaml.org/spec/1.2.2/#mapping> says that keys have to be
unique, so I don't think to ForEachMacros is valid YAML syntax.
> What do you mean only the second is relevant?
glibc does not use the Boost and Qt macros.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 19:47 ` Joseph Myers
2022-03-30 20:11 ` Noah Goldstein
@ 2022-03-30 20:16 ` Noah Goldstein
2022-03-30 20:21 ` Joseph Myers
2022-03-30 20:59 ` Andreas Schwab
1 sibling, 2 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 20:16 UTC (permalink / raw)
To: Joseph Myers; +Cc: Fangrui Song, GNU C Library
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 20:16 ` Noah Goldstein
@ 2022-03-30 20:21 ` Joseph Myers
2022-03-30 20:59 ` Andreas Schwab
1 sibling, 0 replies; 24+ messages in thread
From: Joseph Myers @ 2022-03-30 20:21 UTC (permalink / raw)
To: Noah Goldstein; +Cc: GNU C Library
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.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] Add .clang-format style file
2022-03-30 20:14 ` Florian Weimer
@ 2022-03-30 20:21 ` Noah Goldstein
0 siblings, 0 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 20:21 UTC (permalink / raw)
To: Florian Weimer; +Cc: Noah Goldstein via Libc-alpha
On Wed, Mar 30, 2022 at 3:14 PM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Noah Goldstein:
>
> >> > +UseTab: Never
> >>
> >> I prefer not using tabs, but others disagree, and most of the source
> >> files use tabs instead of 8 spaces (and not just for indentation).
> >
> > +1 but yeah the consistent style seems to be tabs so will update in v4.
>
> Is there an option to use tabs only if the file contains tab?
There doesn't appear to be one. Only
https://clang.llvm.org/docs/ClangFormatStyleOptions.html
UT_Never (in configuration: Never) Never use tab.
UT_ForIndentation (in configuration: ForIndentation) Use tabs only for
indentation.
UT_ForContinuationAndIndentation (in configuration:
ForContinuationAndIndentation) Fill all leading whitespace with tabs,
and use spaces for alignment that appears within a line (e.g.
consecutive assignments and declarations).
UT_AlignWithSpaces (in configuration: AlignWithSpaces) Use tabs for
line continuation and indentation, and spaces for alignment.
UT_Always (in configuration: Always) Use tabs whenever we need to fill
whitespace that spans at least from one tab stop to the next one.
>
> >> > +ForEachMacros:
> >> > + - 'FOR_EACH_IMPL'
> >> > + - 'list_for_each'
> >> > + - 'list_for_each_prev'
> >> > + - 'list_for_each_prev_safe'
> >>
> >> If this is a YAML file, do these duplicate FoEachMacros keys have any
> >> effect? Only the second one is relevant to glibc.
> >
> > They all seem to have an effect.
>
> <https://yaml.org/spec/1.2.2/#mapping> says that keys have to be
> unique, so I don't think to ForEachMacros is valid YAML syntax.
>
> > What do you mean only the second is relevant?
>
> glibc does not use the Boost and Qt macros.
Oh I see, sorry started from the clang default GNU style.
clang-format will pick up latest configuration. Thought you
had meant the second one meaning 'list_for_each'.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5] Add .clang-format style file
2022-03-28 19:12 [PATCH v1] Add .clang-format style file Noah Goldstein
` (2 preceding siblings ...)
2022-03-30 20:10 ` [PATCH v4] " Noah Goldstein
@ 2022-03-30 20:24 ` Noah Goldstein
2022-04-04 14:13 ` Carlos O'Donell
2022-04-04 16:56 ` [PATCH v6] " Noah Goldstein
4 siblings, 1 reply; 24+ messages in thread
From: Noah Goldstein @ 2022-03-30 20:24 UTC (permalink / raw)
To: libc-alpha
Went with version >= 11.0 since it covers most of the major features
and should be pretty universally accessibly.
There are some issues:
1. 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. i.e:
Desired:
```
#ifndef A
# define B
#endif
```
Options:
```
#ifndef A
# define B /* Two spaces instead of one. */
#endif
#ifndef C
#define D /* No spaces. */
#endif
```
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.
2. concatenation of lines missing semi-colons:
Throughout glibc there are macros used to setup aliasing that are
outside of functions and don't end in semi-colons i.e:
```
libc_hidden_def (__pthread_self)
weak_alias (__pthread_self, pthread_self)
```
clang-format reformats lines like these to:
```
libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
```
which is generally undesirable.
Other than those two big concerns there are certainly some questions
diffs but for the most part it creates a easy to read and consistent
style.
---
.clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 156 insertions(+)
create mode 100644 .clang-format
diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..c0d993fd03
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,156 @@
+# 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.
+#
+# To format the current git diff inplace (-i) the follow command can
+# be used:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
+#
+# To just view the diff clang-format would generate:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
+#
+# NB: clang-format-diff, along with other clang-format related tools,
+# can be found at: /path/to/llvm-project/clang/tools/clang-format/
+#
+#
+---
+# BasedOnStyle: GNU
+AccessModifierOffset: -2
+AlignAfterOpenBracket: Align
+AlignConsecutiveMacros: false
+AlignConsecutiveAssignments: false
+AlignConsecutiveBitFields: false
+AlignConsecutiveDeclarations: false
+AlignEscapedNewlines: Right
+AlignOperands: true
+AlignTrailingComments: true
+AllowAllArgumentsOnNextLine: true
+AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortEnumsOnASingleLine: true
+AllowShortBlocksOnASingleLine: false
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: All
+AllowShortLambdasOnASingleLine: All
+AllowShortIfStatementsOnASingleLine: Never
+AllowShortLoopsOnASingleLine: false
+AlwaysBreakAfterDefinitionReturnType: All
+AlwaysBreakAfterReturnType: AllDefinitions
+AlwaysBreakBeforeMultilineStrings: false
+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
+BreakStringLiterals: true
+ColumnLimit: 79
+CommentPragmas: '^ IWYU pragma:'
+CompactNamespaces: false
+ContinuationIndentWidth: 4
+Cpp11BracedListStyle: false
+DeriveLineEnding: true
+DerivePointerAlignment: false
+DisableFormat: false
+ExperimentalAutoDetectBinPacking: false
+FixNamespaceComments: false
+IncludeBlocks: Preserve
+IncludeCategories:
+ - 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
+PenaltyExcessCharacter: 1000000
+PenaltyReturnTypeOnItsOwnLine: 60
+PointerAlignment: Right
+ReflowComments: true
+SortIncludes: false
+SortUsingDeclarations: true
+SpaceAfterCStyleCast: true
+SpaceAfterLogicalNot: false
+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
+TabWidth: 8
+UseTab: Always
+ForEachMacros:
+ - 'FOR_EACH_IMPL'
+ - 'list_for_each'
+ - 'list_for_each_prev'
+ - 'list_for_each_prev_safe'
+...
--
2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Add .clang-format style file
2022-03-30 20:16 ` Noah Goldstein
2022-03-30 20:21 ` Joseph Myers
@ 2022-03-30 20:59 ` Andreas Schwab
1 sibling, 0 replies; 24+ messages in thread
From: Andreas Schwab @ 2022-03-30 20:59 UTC (permalink / raw)
To: Noah Goldstein via Libc-alpha; +Cc: Joseph Myers, Noah Goldstein
On Mär 30 2022, Noah Goldstein via Libc-alpha wrote:
> What's the switch for that in emacs?
c-label-minimum-indentation
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5] Add .clang-format style file
2022-03-30 20:24 ` [PATCH v5] " Noah Goldstein
@ 2022-04-04 14:13 ` Carlos O'Donell
2022-04-04 16:57 ` Noah Goldstein
0 siblings, 1 reply; 24+ messages in thread
From: Carlos O'Donell @ 2022-04-04 14:13 UTC (permalink / raw)
To: Noah Goldstein, libc-alpha
On 3/30/22 16:24, Noah Goldstein via Libc-alpha wrote:
> Went with version >= 11.0 since it covers most of the major features
> and should be pretty universally accessibly.
Almost there. I think with v6 we'll have a version we can commit.
Thanks for iterating.
> There are some issues:
>
> 1. 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. i.e:
> Desired:
> ```
> #ifndef A
> # define B
> #endif
> ```
> Options:
> ```
> #ifndef A
> # define B /* Two spaces instead of one. */
> #endif
>
> #ifndef C
> #define D /* No spaces. */
> #endif
> ```
> 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.
>
> 2. concatenation of lines missing semi-colons:
> Throughout glibc there are macros used to setup aliasing that are
> outside of functions and don't end in semi-colons i.e:
> ```
> libc_hidden_def (__pthread_self)
> weak_alias (__pthread_self, pthread_self)
> ```
>
> clang-format reformats lines like these to:
> ```
> libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> ```
>
> which is generally undesirable.
>
> Other than those two big concerns there are certainly some questions
> diffs but for the most part it creates a easy to read and consistent
> style.
> ---
> .clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 156 insertions(+)
> create mode 100644 .clang-format
>
> diff --git a/.clang-format b/.clang-format
> new file mode 100644
> index 0000000000..c0d993fd03
> --- /dev/null
> +++ b/.clang-format
> @@ -0,0 +1,156 @@
> +# 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
This is not a valid path in the glibc source tree.
> +# 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.
> +#
> +# To format the current git diff inplace (-i) the follow command can
> +# be used:
> +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
> +#
> +# To just view the diff clang-format would generate:
> +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
> +#
> +# NB: clang-format-diff, along with other clang-format related tools,
> +# can be found at: /path/to/llvm-project/clang/tools/clang-format/
> +#
> +#
> +---
> +# BasedOnStyle: GNU
Suggest:
# This file is based on the existing GNU style e.g. BasedOnStyle: GNU.
Otherwise developers will be confused why the directive is commented out.
> +AccessModifierOffset: -2
> +AlignAfterOpenBracket: Align
> +AlignConsecutiveMacros: false
> +AlignConsecutiveAssignments: false
> +AlignConsecutiveBitFields: false
> +AlignConsecutiveDeclarations: false
> +AlignEscapedNewlines: Right
> +AlignOperands: true
> +AlignTrailingComments: true
> +AllowAllArgumentsOnNextLine: true
> +AllowAllParametersOfDeclarationOnNextLine: true
> +AllowShortEnumsOnASingleLine: true
> +AllowShortBlocksOnASingleLine: false
> +AllowShortCaseLabelsOnASingleLine: false
> +AllowShortFunctionsOnASingleLine: All
> +AllowShortLambdasOnASingleLine: All
> +AllowShortIfStatementsOnASingleLine: Never
> +AllowShortLoopsOnASingleLine: false
> +AlwaysBreakAfterDefinitionReturnType: All
> +AlwaysBreakAfterReturnType: AllDefinitions
> +AlwaysBreakBeforeMultilineStrings: false
> +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
> +BreakStringLiterals: true
> +ColumnLimit: 79
> +CommentPragmas: '^ IWYU pragma:'
> +CompactNamespaces: false
> +ContinuationIndentWidth: 4
> +Cpp11BracedListStyle: false
> +DeriveLineEnding: true
> +DerivePointerAlignment: false
> +DisableFormat: false
> +ExperimentalAutoDetectBinPacking: false
> +FixNamespaceComments: false
> +IncludeBlocks: Preserve
> +IncludeCategories:
> + - Regex: '.*'
> + Priority: 1
OK. Resolves Florian's comment regarding IncludeCategories.
> +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
> +PenaltyExcessCharacter: 1000000
> +PenaltyReturnTypeOnItsOwnLine: 60
> +PointerAlignment: Right
> +ReflowComments: true
> +SortIncludes: false
> +SortUsingDeclarations: true
> +SpaceAfterCStyleCast: true
> +SpaceAfterLogicalNot: false
> +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
OK. Removed StatementMacro use for QT. Resolves Florian's comments in v3.
> +TabWidth: 8
> +UseTab: Always
OK. Resolved Florian's comments in v3.
> +ForEachMacros:
> + - 'FOR_EACH_IMPL'
> + - 'list_for_each'
> + - 'list_for_each_prev'
> + - 'list_for_each_prev_safe'
> +...
OK.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v6] Add .clang-format style file
2022-03-28 19:12 [PATCH v1] Add .clang-format style file Noah Goldstein
` (3 preceding siblings ...)
2022-03-30 20:24 ` [PATCH v5] " Noah Goldstein
@ 2022-04-04 16:56 ` Noah Goldstein
2022-04-11 14:18 ` Carlos O'Donell
4 siblings, 1 reply; 24+ messages in thread
From: Noah Goldstein @ 2022-04-04 16:56 UTC (permalink / raw)
To: libc-alpha
Went with version >= 11.0 since it covers most of the major features
and should be pretty universally accessibly.
There are some issues:
1. 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. i.e:
Desired:
```
#ifndef A
# define B
#endif
```
Options:
```
#ifndef A
# define B /* Two spaces instead of one. */
#endif
#ifndef C
#define D /* No spaces. */
#endif
```
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.
2. concatenation of lines missing semi-colons:
Throughout glibc there are macros used to setup aliasing that are
outside of functions and don't end in semi-colons i.e:
```
libc_hidden_def (__pthread_self)
weak_alias (__pthread_self, pthread_self)
```
clang-format reformats lines like these to:
```
libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
```
which is generally undesirable.
Other than those two big concerns there are certainly some questions
diffs but for the most part it creates a easy to read and consistent
style.
---
.clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 156 insertions(+)
create mode 100644 .clang-format
diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..79530b305a
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,156 @@
+# 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:
+#
+# 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.
+#
+# To format the current git diff inplace (-i) the follow command can
+# be used:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
+#
+# To just view the diff clang-format would generate:
+# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
+#
+# NB: clang-format-diff, along with other clang-format related tools,
+# can be found at: /path/to/llvm-project/clang/tools/clang-format/
+#
+#
+# Based on autogenerated format from:
+# $> clang-format --style=GNU -dump-config
+---
+AccessModifierOffset: -2
+AlignAfterOpenBracket: Align
+AlignConsecutiveMacros: false
+AlignConsecutiveAssignments: false
+AlignConsecutiveBitFields: false
+AlignConsecutiveDeclarations: false
+AlignEscapedNewlines: Right
+AlignOperands: true
+AlignTrailingComments: true
+AllowAllArgumentsOnNextLine: true
+AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortEnumsOnASingleLine: true
+AllowShortBlocksOnASingleLine: false
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: All
+AllowShortLambdasOnASingleLine: All
+AllowShortIfStatementsOnASingleLine: Never
+AllowShortLoopsOnASingleLine: false
+AlwaysBreakAfterDefinitionReturnType: All
+AlwaysBreakAfterReturnType: AllDefinitions
+AlwaysBreakBeforeMultilineStrings: false
+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
+BreakStringLiterals: true
+ColumnLimit: 79
+CommentPragmas: '^ IWYU pragma:'
+CompactNamespaces: false
+ContinuationIndentWidth: 4
+Cpp11BracedListStyle: false
+DeriveLineEnding: true
+DerivePointerAlignment: false
+DisableFormat: false
+ExperimentalAutoDetectBinPacking: false
+FixNamespaceComments: false
+IncludeBlocks: Preserve
+IncludeCategories:
+ - 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
+PenaltyExcessCharacter: 1000000
+PenaltyReturnTypeOnItsOwnLine: 60
+PointerAlignment: Right
+ReflowComments: true
+SortIncludes: false
+SortUsingDeclarations: true
+SpaceAfterCStyleCast: true
+SpaceAfterLogicalNot: false
+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
+TabWidth: 8
+UseTab: Always
+ForEachMacros:
+ - 'FOR_EACH_IMPL'
+ - 'list_for_each'
+ - 'list_for_each_prev'
+ - 'list_for_each_prev_safe'
+...
--
2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5] Add .clang-format style file
2022-04-04 14:13 ` Carlos O'Donell
@ 2022-04-04 16:57 ` Noah Goldstein
0 siblings, 0 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-04-04 16:57 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: GNU C Library
On Mon, Apr 4, 2022 at 9:13 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 3/30/22 16:24, Noah Goldstein via Libc-alpha wrote:
> > Went with version >= 11.0 since it covers most of the major features
> > and should be pretty universally accessibly.
>
> Almost there. I think with v6 we'll have a version we can commit.
>
> Thanks for iterating.
>
> > There are some issues:
> >
> > 1. 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. i.e:
> > Desired:
> > ```
> > #ifndef A
> > # define B
> > #endif
> > ```
> > Options:
> > ```
> > #ifndef A
> > # define B /* Two spaces instead of one. */
> > #endif
> >
> > #ifndef C
> > #define D /* No spaces. */
> > #endif
> > ```
> > 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.
> >
> > 2. concatenation of lines missing semi-colons:
> > Throughout glibc there are macros used to setup aliasing that are
> > outside of functions and don't end in semi-colons i.e:
> > ```
> > libc_hidden_def (__pthread_self)
> > weak_alias (__pthread_self, pthread_self)
> > ```
> >
> > clang-format reformats lines like these to:
> > ```
> > libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> > ```
> >
> > which is generally undesirable.
> >
> > Other than those two big concerns there are certainly some questions
> > diffs but for the most part it creates a easy to read and consistent
> > style.
> > ---
> > .clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 156 insertions(+)
> > create mode 100644 .clang-format
> >
> > diff --git a/.clang-format b/.clang-format
> > new file mode 100644
> > index 0000000000..c0d993fd03
> > --- /dev/null
> > +++ b/.clang-format
> > @@ -0,0 +1,156 @@
> > +# 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
>
> This is not a valid path in the glibc source tree.
Removed that comment. Think the online documentation is enough.
>
> > +# 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.
> > +#
> > +# To format the current git diff inplace (-i) the follow command can
> > +# be used:
> > +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
> > +#
> > +# To just view the diff clang-format would generate:
> > +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
> > +#
> > +# NB: clang-format-diff, along with other clang-format related tools,
> > +# can be found at: /path/to/llvm-project/clang/tools/clang-format/
> > +#
> > +#
> > +---
> > +# BasedOnStyle: GNU
>
> Suggest:
>
> # This file is based on the existing GNU style e.g. BasedOnStyle: GNU.
>
> Otherwise developers will be confused why the directive is commented out.
Removed this and added comment in the header clarifying.
>
> > +AccessModifierOffset: -2
> > +AlignAfterOpenBracket: Align
> > +AlignConsecutiveMacros: false
> > +AlignConsecutiveAssignments: false
> > +AlignConsecutiveBitFields: false
> > +AlignConsecutiveDeclarations: false
> > +AlignEscapedNewlines: Right
> > +AlignOperands: true
> > +AlignTrailingComments: true
> > +AllowAllArgumentsOnNextLine: true
> > +AllowAllParametersOfDeclarationOnNextLine: true
> > +AllowShortEnumsOnASingleLine: true
> > +AllowShortBlocksOnASingleLine: false
> > +AllowShortCaseLabelsOnASingleLine: false
> > +AllowShortFunctionsOnASingleLine: All
> > +AllowShortLambdasOnASingleLine: All
> > +AllowShortIfStatementsOnASingleLine: Never
> > +AllowShortLoopsOnASingleLine: false
> > +AlwaysBreakAfterDefinitionReturnType: All
> > +AlwaysBreakAfterReturnType: AllDefinitions
> > +AlwaysBreakBeforeMultilineStrings: false
> > +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
> > +BreakStringLiterals: true
> > +ColumnLimit: 79
> > +CommentPragmas: '^ IWYU pragma:'
> > +CompactNamespaces: false
> > +ContinuationIndentWidth: 4
> > +Cpp11BracedListStyle: false
> > +DeriveLineEnding: true
> > +DerivePointerAlignment: false
> > +DisableFormat: false
> > +ExperimentalAutoDetectBinPacking: false
> > +FixNamespaceComments: false
> > +IncludeBlocks: Preserve
> > +IncludeCategories:
> > + - Regex: '.*'
> > + Priority: 1
>
> OK. Resolves Florian's comment regarding IncludeCategories.
>
> > +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
> > +PenaltyExcessCharacter: 1000000
> > +PenaltyReturnTypeOnItsOwnLine: 60
> > +PointerAlignment: Right
> > +ReflowComments: true
> > +SortIncludes: false
> > +SortUsingDeclarations: true
> > +SpaceAfterCStyleCast: true
> > +SpaceAfterLogicalNot: false
> > +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
>
> OK. Removed StatementMacro use for QT. Resolves Florian's comments in v3.
>
> > +TabWidth: 8
> > +UseTab: Always
>
> OK. Resolved Florian's comments in v3.
>
> > +ForEachMacros:
> > + - 'FOR_EACH_IMPL'
> > + - 'list_for_each'
> > + - 'list_for_each_prev'
> > + - 'list_for_each_prev_safe'
> > +...
>
> OK.
>
> --
> Cheers,
> Carlos.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v6] Add .clang-format style file
2022-04-04 16:56 ` [PATCH v6] " Noah Goldstein
@ 2022-04-11 14:18 ` Carlos O'Donell
2022-04-11 15:52 ` Noah Goldstein
0 siblings, 1 reply; 24+ messages in thread
From: Carlos O'Donell @ 2022-04-11 14:18 UTC (permalink / raw)
To: Noah Goldstein, libc-alpha
On 4/4/22 12:56, Noah Goldstein via Libc-alpha wrote:
> Went with version >= 11.0 since it covers most of the major features
> and should be pretty universally accessibly.
This version looks good to me and has addressed the concerns of the previous reviewers.
This kind of file can be evolved over time if we find that it doesn't exactly meet our
needs, but it should produce correctly formatted code in as many cases as possible.
LGTM.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> There are some issues:
>
> 1. 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. i.e:
> Desired:
> ```
> #ifndef A
> # define B
> #endif
> ```
> Options:
> ```
> #ifndef A
> # define B /* Two spaces instead of one. */
> #endif
>
> #ifndef C
> #define D /* No spaces. */
> #endif
> ```
> 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.
Agreed.
>
> 2. concatenation of lines missing semi-colons:
> Throughout glibc there are macros used to setup aliasing that are
> outside of functions and don't end in semi-colons i.e:
> ```
> libc_hidden_def (__pthread_self)
> weak_alias (__pthread_self, pthread_self)
> ```
>
> clang-format reformats lines like these to:
> ```
> libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> ```
>
> which is generally undesirable.
It is undesirable.
However, I'm open to having a consensus discussion if we should just put a ; after these macros
to avoid formatter issues. Please feel free to start that discussion in a new thread.
We cannot just hide behind established practice and expect all of our tooling to mold itself
to our whims, we need, in some cases to meet tooling half-way.
>
> Other than those two big concerns there are certainly some questions
> diffs but for the most part it creates a easy to read and consistent
> style.
Agreed.
> ---
> .clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 156 insertions(+)
> create mode 100644 .clang-format
>
> diff --git a/.clang-format b/.clang-format
> new file mode 100644
> index 0000000000..79530b305a
> --- /dev/null
> +++ b/.clang-format
> @@ -0,0 +1,156 @@
> +# 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:
> +#
> +# 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.
> +#
> +# To format the current git diff inplace (-i) the follow command can
> +# be used:
> +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
> +#
> +# To just view the diff clang-format would generate:
> +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
> +#
> +# NB: clang-format-diff, along with other clang-format related tools,
> +# can be found at: /path/to/llvm-project/clang/tools/clang-format/
> +#
> +#
> +# Based on autogenerated format from:
> +# $> clang-format --style=GNU -dump-config
> +---
> +AccessModifierOffset: -2
> +AlignAfterOpenBracket: Align
> +AlignConsecutiveMacros: false
> +AlignConsecutiveAssignments: false
> +AlignConsecutiveBitFields: false
> +AlignConsecutiveDeclarations: false
> +AlignEscapedNewlines: Right
> +AlignOperands: true
> +AlignTrailingComments: true
> +AllowAllArgumentsOnNextLine: true
> +AllowAllParametersOfDeclarationOnNextLine: true
> +AllowShortEnumsOnASingleLine: true
> +AllowShortBlocksOnASingleLine: false
> +AllowShortCaseLabelsOnASingleLine: false
> +AllowShortFunctionsOnASingleLine: All
> +AllowShortLambdasOnASingleLine: All
> +AllowShortIfStatementsOnASingleLine: Never
> +AllowShortLoopsOnASingleLine: false
> +AlwaysBreakAfterDefinitionReturnType: All
> +AlwaysBreakAfterReturnType: AllDefinitions
> +AlwaysBreakBeforeMultilineStrings: false
> +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
> +BreakStringLiterals: true
> +ColumnLimit: 79
> +CommentPragmas: '^ IWYU pragma:'
> +CompactNamespaces: false
> +ContinuationIndentWidth: 4
> +Cpp11BracedListStyle: false
> +DeriveLineEnding: true
> +DerivePointerAlignment: false
> +DisableFormat: false
> +ExperimentalAutoDetectBinPacking: false
> +FixNamespaceComments: false
> +IncludeBlocks: Preserve
> +IncludeCategories:
> + - 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
> +PenaltyExcessCharacter: 1000000
> +PenaltyReturnTypeOnItsOwnLine: 60
> +PointerAlignment: Right
> +ReflowComments: true
> +SortIncludes: false
> +SortUsingDeclarations: true
> +SpaceAfterCStyleCast: true
> +SpaceAfterLogicalNot: false
> +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
> +TabWidth: 8
> +UseTab: Always
> +ForEachMacros:
> + - 'FOR_EACH_IMPL'
> + - 'list_for_each'
> + - 'list_for_each_prev'
> + - 'list_for_each_prev_safe'
> +...
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v6] Add .clang-format style file
2022-04-11 14:18 ` Carlos O'Donell
@ 2022-04-11 15:52 ` Noah Goldstein
0 siblings, 0 replies; 24+ messages in thread
From: Noah Goldstein @ 2022-04-11 15:52 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: GNU C Library
On Mon, Apr 11, 2022 at 9:18 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 4/4/22 12:56, Noah Goldstein via Libc-alpha wrote:
> > Went with version >= 11.0 since it covers most of the major features
> > and should be pretty universally accessibly.
>
> This version looks good to me and has addressed the concerns of the previous reviewers.
> This kind of file can be evolved over time if we find that it doesn't exactly meet our
> needs, but it should produce correctly formatted code in as many cases as possible.
>
> LGTM.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Thanks pushed.
>
> > There are some issues:
> >
> > 1. 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. i.e:
> > Desired:
> > ```
> > #ifndef A
> > # define B
> > #endif
> > ```
> > Options:
> > ```
> > #ifndef A
> > # define B /* Two spaces instead of one. */
> > #endif
> >
> > #ifndef C
> > #define D /* No spaces. */
> > #endif
> > ```
> > 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.
>
> Agreed.
>
> >
> > 2. concatenation of lines missing semi-colons:
> > Throughout glibc there are macros used to setup aliasing that are
> > outside of functions and don't end in semi-colons i.e:
> > ```
> > libc_hidden_def (__pthread_self)
> > weak_alias (__pthread_self, pthread_self)
> > ```
> >
> > clang-format reformats lines like these to:
> > ```
> > libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
> > ```
> >
> > which is generally undesirable.
>
> It is undesirable.
>
> However, I'm open to having a consensus discussion if we should just put a ; after these macros
> to avoid formatter issues. Please feel free to start that discussion in a new thread.
>
> We cannot just hide behind established practice and expect all of our tooling to mold itself
> to our whims, we need, in some cases to meet tooling half-way.
>
> >
> > Other than those two big concerns there are certainly some questions
> > diffs but for the most part it creates a easy to read and consistent
> > style.
>
> Agreed.
>
> > ---
> > .clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 156 insertions(+)
> > create mode 100644 .clang-format
> >
> > diff --git a/.clang-format b/.clang-format
> > new file mode 100644
> > index 0000000000..79530b305a
> > --- /dev/null
> > +++ b/.clang-format
> > @@ -0,0 +1,156 @@
> > +# 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:
> > +#
> > +# 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.
> > +#
> > +# To format the current git diff inplace (-i) the follow command can
> > +# be used:
> > +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1
> > +#
> > +# To just view the diff clang-format would generate:
> > +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1
> > +#
> > +# NB: clang-format-diff, along with other clang-format related tools,
> > +# can be found at: /path/to/llvm-project/clang/tools/clang-format/
> > +#
> > +#
> > +# Based on autogenerated format from:
> > +# $> clang-format --style=GNU -dump-config
> > +---
> > +AccessModifierOffset: -2
> > +AlignAfterOpenBracket: Align
> > +AlignConsecutiveMacros: false
> > +AlignConsecutiveAssignments: false
> > +AlignConsecutiveBitFields: false
> > +AlignConsecutiveDeclarations: false
> > +AlignEscapedNewlines: Right
> > +AlignOperands: true
> > +AlignTrailingComments: true
> > +AllowAllArgumentsOnNextLine: true
> > +AllowAllParametersOfDeclarationOnNextLine: true
> > +AllowShortEnumsOnASingleLine: true
> > +AllowShortBlocksOnASingleLine: false
> > +AllowShortCaseLabelsOnASingleLine: false
> > +AllowShortFunctionsOnASingleLine: All
> > +AllowShortLambdasOnASingleLine: All
> > +AllowShortIfStatementsOnASingleLine: Never
> > +AllowShortLoopsOnASingleLine: false
> > +AlwaysBreakAfterDefinitionReturnType: All
> > +AlwaysBreakAfterReturnType: AllDefinitions
> > +AlwaysBreakBeforeMultilineStrings: false
> > +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
> > +BreakStringLiterals: true
> > +ColumnLimit: 79
> > +CommentPragmas: '^ IWYU pragma:'
> > +CompactNamespaces: false
> > +ContinuationIndentWidth: 4
> > +Cpp11BracedListStyle: false
> > +DeriveLineEnding: true
> > +DerivePointerAlignment: false
> > +DisableFormat: false
> > +ExperimentalAutoDetectBinPacking: false
> > +FixNamespaceComments: false
> > +IncludeBlocks: Preserve
> > +IncludeCategories:
> > + - 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
> > +PenaltyExcessCharacter: 1000000
> > +PenaltyReturnTypeOnItsOwnLine: 60
> > +PointerAlignment: Right
> > +ReflowComments: true
> > +SortIncludes: false
> > +SortUsingDeclarations: true
> > +SpaceAfterCStyleCast: true
> > +SpaceAfterLogicalNot: false
> > +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
> > +TabWidth: 8
> > +UseTab: Always
> > +ForEachMacros:
> > + - 'FOR_EACH_IMPL'
> > + - 'list_for_each'
> > + - 'list_for_each_prev'
> > + - 'list_for_each_prev_safe'
> > +...
>
>
>
> --
> Cheers,
> Carlos.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2022-04-11 15:52 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-28 19:12 [PATCH v1] Add .clang-format style file Noah Goldstein
2022-03-30 0:26 ` Fangrui Song
2022-03-30 1:33 ` Noah Goldstein
2022-03-30 17:02 ` [PATCH v2] " Noah Goldstein
2022-03-30 18:17 ` Fangrui Song
2022-03-30 19:30 ` Noah Goldstein
2022-03-30 19:47 ` Joseph Myers
2022-03-30 20:11 ` Noah Goldstein
2022-03-30 20:16 ` Noah Goldstein
2022-03-30 20:21 ` Joseph Myers
2022-03-30 20:59 ` Andreas Schwab
2022-03-30 19:27 ` [PATCH v3] " Noah Goldstein
2022-03-30 19:59 ` Fangrui Song
2022-03-30 20:01 ` Florian Weimer
2022-03-30 20:09 ` Noah Goldstein
2022-03-30 20:14 ` Florian Weimer
2022-03-30 20:21 ` Noah Goldstein
2022-03-30 20:10 ` [PATCH v4] " Noah Goldstein
2022-03-30 20:24 ` [PATCH v5] " Noah Goldstein
2022-04-04 14:13 ` Carlos O'Donell
2022-04-04 16:57 ` Noah Goldstein
2022-04-04 16:56 ` [PATCH v6] " Noah Goldstein
2022-04-11 14:18 ` Carlos O'Donell
2022-04-11 15:52 ` Noah Goldstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).