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

[Carlos O'Donell <carlos@redhat.com>]

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.